Support AWS compressed storage uploads#3113
Conversation
PR SummaryMedium Risk Overview Upload throttling is renamed to storage-neutral APIs and flags ( Shared contract tests exercise the Reviewed by Cursor Bugbot for commit 9f9b81a. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| bucketName: o.bucketName, | ||
| objectName: o.path, | ||
| metadata: putOpts.Metadata, | ||
| } |
There was a problem hiding this comment.
Missing uncompressed size metadata
Medium Severity
AWS compressed uploads pass only caller metadata into CreateMultipartUpload, unlike GCP which also sets uncompressed-size from the source file. awsObject.Size() uses S3 ContentLength, so compressed objects report compressed bytes instead of logical uncompressed size.
Reviewed by Cursor Bugbot for commit a1a8100. Configure here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The awsPartUploader is never closed in storeFileCompressed, which can cause unfinished multipart uploads to leak on AWS S3 if the compression or upload stream fails. Additionally, access to m.completed is not synchronized, leading to potential data races if Close is called concurrently with or shortly after Complete.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| uploader := &awsPartUploader{ | ||
| client: o.client, | ||
| bucketName: o.bucketName, | ||
| objectName: o.path, | ||
| metadata: putOpts.Metadata, | ||
| } |
There was a problem hiding this comment.
The awsPartUploader is never closed in storeFileCompressed, which causes unfinished multipart uploads to leak on AWS S3 if the compression or upload stream fails. Deferring uploader.Close() ensures that any incomplete multipart upload is properly aborted and its associated storage resources are cleaned up.
| uploader := &awsPartUploader{ | |
| client: o.client, | |
| bucketName: o.bucketName, | |
| objectName: o.path, | |
| metadata: putOpts.Metadata, | |
| } | |
| uploader := &awsPartUploader{ | |
| client: o.client, | |
| bucketName: o.bucketName, | |
| objectName: o.path, | |
| metadata: putOpts.Metadata, | |
| } | |
| defer uploader.Close() |
References
- Focus on resource leaks as specified in the repository style guide. (link)
|
|
||
| m.completed = true | ||
|
|
||
| return nil |
There was a problem hiding this comment.
Access to m.completed is not synchronized, which can lead to a data race if Close is called concurrently with or shortly after Complete (for example, during context cancellation or early termination of the upload pipeline). Protecting m.completed with m.mu in both Complete and Close prevents this race condition.
m.mu.Lock()
m.completed = true
m.mu.Unlock()
return nilReferences
- Focus on race conditions as specified in the repository style guide. (link)
| func (m *awsPartUploader) Close() error { | ||
| if m.completed || m.uploadID == "" { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Access to m.completed is not synchronized, which can lead to a data race if Close is called concurrently with or shortly after Complete (for example, during context cancellation or early termination of the upload pipeline). Protecting m.completed with m.mu in both Complete and Close prevents this race condition.
| func (m *awsPartUploader) Close() error { | |
| if m.completed || m.uploadID == "" { | |
| return nil | |
| } | |
| func (m *awsPartUploader) Close() error { | |
| m.mu.Lock() | |
| completed := m.completed | |
| m.mu.Unlock() | |
| if completed || m.uploadID == "" { | |
| return nil | |
| } |
References
- Focus on race conditions as specified in the repository style guide. (link)
| "github.com/e2b-dev/infra/packages/shared/pkg/utils" | ||
| ) | ||
|
|
||
| func (l *Limiter) StorageUploadLimiter() *utils.AdjustableSemaphore { |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0851835. Configure here.
| u.PartSize = 10 * 1024 * 1024 // 10 MB | ||
| u.Concurrency = 8 // eight parts in flight | ||
| u.PartSize = awsMultipartUploadPartSize | ||
| u.Concurrency = o.maxUploadConcurrency(ctx) |
There was a problem hiding this comment.
Non-compressed AWS skips upload semaphore
Medium Severity
Uncompressed AWS StoreFile uses StorageMaxUploadTasks for SDK concurrency but never acquires StorageUploadLimiter, while GCP StoreFile holds that semaphore for every upload when a limiter is configured.
Reviewed by Cursor Bugbot for commit 0851835. Configure here.
1 participant
Adds AWS support for compressed storage uploads using the shared compression pipeline and S3 multipart upload APIs. Renames the upload limiter API to storage-neutral names while preserving existing LaunchDarkly flag keys. Threads the existing limiter into AWS storage so AWS and GCP compressed uploads share the same concurrency controls. Adds shared part uploader contract tests for GCP and AWS.