feat: add command for optimizing existing builds#1709
Conversation
| return fmt.Errorf("failed to create sandbox proxy: %w", err) | ||
| } | ||
| go func() { | ||
| err := sandboxProxy.Start(parentCtx) |
There was a problem hiding this comment.
The goroutine at line 144 uses parentCtx instead of ctx for starting sandboxProxy. This means sandboxProxy.Start() ignores the 10-minute operation timeout and could run indefinitely even if the optimization fails or times out. Consider using ctx instead for proper timeout propagation.
| noop.NewMeterProvider(), | ||
| ) | ||
| go func() { | ||
| err := tcpFirewall.Start(ctx) |
There was a problem hiding this comment.
The tcpFirewall.Start() goroutine uses ctx (the timeout context) but tcpFirewall.Close() in the defer uses parentCtx. This creates inconsistency - if the timeout fires and cancels ctx, the Start goroutine will stop, but the Close() will use a different context. Consider using the same context for both operations.
| return fmt.Errorf("could not create device pool: %w", err) | ||
| } | ||
| go func() { | ||
| devicePool.Populate(ctx) |
There was a problem hiding this comment.
The devicePool.Populate() and networkPool.Populate() goroutines use ctx (timeout context). If the optimization completes or times out before these pools finish populating, the goroutines will be canceled but may still be running during cleanup. This could cause resource leaks or race conditions. Consider waiting for these goroutines to complete or using an errgroup to manage them properly.
| ) | ||
|
|
||
| // Create build context | ||
| uploadErrGroup := &errgroup.Group{} |
There was a problem hiding this comment.
The uploadErrGroup created at line 245 is never waited on with .Wait(). If any upload operations are scheduled via this errgroup during optimization, their errors will be silently ignored and goroutines may be leaked when the function returns. Either add a defer with uploadErrGroup.Wait() or document that no uploads are expected during optimization.
f342294 to
68db802
Compare
99db1f3 to
bde27e0
Compare
bde27e0 to
a01add7
Compare
No description provided.