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

fix: parallelize deployments #1042

Merged
merged 1 commit into from
Mar 7, 2024
Merged

fix: parallelize deployments #1042

merged 1 commit into from
Mar 7, 2024

Conversation

wesbillman
Copy link
Collaborator

Fixes #1031

Introduces non-blocking deploys so that they can while other modules are build.

BEFORE (20s cold, 11s warm):
Screenshot 2024-03-07 at 1 28 09 PM

AFTER (16s cold, 8s warm):
Screenshot 2024-03-07 at 1 29 23 PM

@alecthomas alecthomas mentioned this pull request Mar 7, 2024
Comment on lines 229 to 230
wg, ctx := errgroup.WithContext(ctx)
wg.SetLimit(runtime.NumCPU())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to follow the pattern in the build command to limit the concurrency. Not 100% sure if this is the best solution of if there's something else I should try here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You probably don't need the same limit for deploys, it will be mostly network bound I think. You can probably remove the limit for now.

Comment on lines 229 to 230
wg, ctx := errgroup.WithContext(ctx)
wg.SetLimit(runtime.NumCPU())
Copy link
Collaborator

Choose a reason for hiding this comment

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

You probably don't need the same limit for deploys, it will be mostly network bound I think. You can probably remove the limit for now.


for i := 0; i < len(modules); i++ {
wg.Go(func() error {
for module := range deployQueue {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a for loop with a select inside, that checks <-ctx.Done() as the other Go call does.

@wesbillman wesbillman force-pushed the parallelize-deployments branch from 723b6ba to 2a577ad Compare March 7, 2024 22:30
@wesbillman wesbillman force-pushed the parallelize-deployments branch from 2a577ad to a2d1400 Compare March 7, 2024 22:39
@wesbillman wesbillman merged commit 2bfa633 into main Mar 7, 2024
11 checks passed
@wesbillman wesbillman deleted the parallelize-deployments branch March 7, 2024 23:28
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.

Add queuing for deployments to speed up builds
2 participants