Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature flags for each protocol in shotover #1441

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Jan 29, 2024

This improves compile times of windsock without compromising on performance like #1438 ended up doing.

Previously cargo windsock took 2m 45s to compile after a change to our windsock benches.
Now it takes:

  • cargo windsock-redis - 1 min 32s
  • cargo windsock-kafka - 1 min 21s
  • cargo windsock-cassandra - 2 min 23s
  • cargo windsock - 2m 45s

We would be getting similar improvements to our integration tests but we rarely need to run them in release so I didnt bother adding cargo aliases for them or measuring them.

Additionally, this PR improves compile times of shotover when developing custom transforms that only need to support a single protocol by allowing developers to disable unneeded dependencies within shotover, this will be quite helpful for our internal kafka project where we are also struggling with build times and have no need for any protocol other than kafka.
As a result of compiling less, the binary size with just kafka enabled is now 13.9MB, while the standard build with everything enabled is 19MB.

Additionally, additionally, this will be quite helpful for investigating the cause of build time issues in the future as we can easily eliminate certain protocols from the build.

Updates:

Reduced cfgs

With the help of #1442 #1443 #1444 and #1445 I have reduced the number of cfgs from 283 down to 181

Improvements for custom transform devs

Testing on our internal kafka-specific project:

  • full build times of shotover have been reduced from 8m 53s to 4m 49s
  • iterative build times have been reduced from 4m 26s to 1m 57s

As a result with this PR we will cut 4 minutes off the time to run CI as we have no caching setup for our CI on that project.
We will also cut 2 mins 30s from our iterative benchmark workflow which is super important to the project as its very performance sensitive.

@rukai rukai force-pushed the featurize_shotover branch 14 times, most recently from 2ce3fe7 to 312cfcd Compare January 30, 2024 04:30
@rukai rukai marked this pull request as ready for review January 30, 2024 04:51
@rukai rukai requested a review from conorbros January 30, 2024 04:58
Copy link
Collaborator

@cjrolo cjrolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good learning to understand how to set feature flags!

@rukai rukai enabled auto-merge (squash) January 30, 2024 10:27
@rukai rukai force-pushed the featurize_shotover branch 4 times, most recently from a6eda87 to 4a223bf Compare January 31, 2024 03:30
@rukai rukai force-pushed the featurize_shotover branch 2 times, most recently from 0180540 to 6dc067a Compare January 31, 2024 05:04
@rukai
Copy link
Member Author

rukai commented Jan 31, 2024

From a discussion in DMs with @conorbros about this PR:

conor:
My concern is just a future shotover developer really fighting with the project trying to get it to compile with all these feature flags around.
Feature issues are especially hard to debug, IME
At the moment, I think we should not merge the changes in 1441 and instead figure out how to split out the protocol specific logic, modules and transforms

So I'll make a bunch of prereq PRs to better split out protocol specific logic such that this PR results in far less cfgs.
Additionally I'll measure the build time improvements on my internal kafka project so we have more measurements we can take into consideration.

@rukai rukai force-pushed the featurize_shotover branch 7 times, most recently from 43db390 to 541973a Compare February 1, 2024 01:40
@rukai rukai force-pushed the featurize_shotover branch from 541973a to 65d3640 Compare February 1, 2024 10:16
@rukai rukai force-pushed the featurize_shotover branch from 65d3640 to 3fd9968 Compare February 1, 2024 10:21
@rukai rukai merged commit e3c3bfe into shotover:main Feb 1, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants