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 CLI backend #41

Merged
merged 1 commit into from
Jan 28, 2024
Merged

Add CLI backend #41

merged 1 commit into from
Jan 28, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Jan 24, 2024

A refactor I've been thinking about to eliminate the aws-sdk dependencies to save us from our terrible build times.
Still a long way way to go so I think I'll just work around the issue for now.
Ended up finishing it off.

The code quality is poor in some places as I'm hoping to delete this if the aws-sdk team can fix the build times, which they have promised to try

So on one hand this PR cuts out 5 minutes from an uncached release build.
But it also only cuts out 20 seconds from a cached release build and I was expecting to cut out a lot more (currently our cached release builds take 3 minutes)

I think it still makes sense to land at this point, although maybe I should have invested my time elsewhere in the first place.

@rukai rukai force-pushed the add_cli_backend branch 5 times, most recently from d674c80 to c700ab5 Compare January 25, 2024 05:34
@rukai rukai marked this pull request as ready for review January 25, 2024 05:58
@rukai rukai requested a review from conorbros January 25, 2024 06:09
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.

Approving this.

Why is build time so important for an end-user? I feel like adding extra code to support a build time a bit detrimental.

@rukai
Copy link
Member Author

rukai commented Jan 28, 2024

Thanks for the feedback. It actually really pained me to make this PR but I felt like I couldnt justify putting off the issue any longer.

AFAIK the only end users of this project is us, and we use it for running benchmarks in EC2, it doesnt end up in any user facing project.
For testing changes in benchmarks fast iterations is key and there are 2 cases this PR improves:

  • Running the benchmark orchestrator from within a fresh EC2 instance.
    • we have no cache as its a fresh instance, it takes >10 minutes to compile just aws-sdk-ec2 on this instance while the rest of the project finishes in like ~5 mins. This PR cuts out the 10 minutes from aws-sdk-ec2, a huge time save.
    • this is currently a requirement when extracting shotover metrics from prometheus due to the way security groups are setup.
  • Running the benchmark orchestrator locally which then generates EC2 instances.
    • Locally we can make use of our build cache, so builds take only 3 mins.
    • with this PR we improve our build time in this case by 20s. I'm hoping to find wins in other areas but 20s is a good start.

If the promised aws-sdk-ec2 compile time improvements are good enough then we can delete this new CLI backend.
If they dont arrive after ~1 year then we should probably get rid of the SDK backend to reduce maintenance burden.

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