Concurrent s3 parts upload - #205
Open
barnabehvrd wants to merge 10 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe upload response now reports the maximum concurrent upload count. S3 multipart uploads now run with bounded concurrency, independent file sections, context cancellation, dedicated HTTP transport settings, and expanded retry handling. ChangesBackup upload flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant BackupUploader
participant osFile as *os.File
participant errgroup
participant HTTPClient
participant S3Endpoint
BackupUploader->>osFile: Read file size
BackupUploader->>HTTPClient: Retrieve upload URLs with context
BackupUploader->>errgroup: Start bounded upload workers
errgroup->>osFile: Read independent io.SectionReader parts
errgroup->>HTTPClient: Send multipart part request
HTTPClient->>S3Endpoint: Upload part
S3Endpoint-->>HTTPClient: Return response
HTTPClient->>S3Endpoint: Retry HTTP 429 or 5xx response
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following PR pelican/panel#2510 on the Panel, this PR makes Wings upload S3 backup parts concurrently instead of one after another.
The number of parts in flight is bounded by the
max_concurrent_uploadsvalue returned by the Panel alongside the part URLs, and the upload stops as soon as any part fails permanently.Uploading parts in parallel was not possible before: every part read from the same shared
io.Reader, so concurrent reads would have interleaved and produced a corrupted archive.Each part now gets its own
io.SectionReaderover the backup file, which is safe for concurrent use and lets a failed part be retried from its correct offset. Parts are uploaded through anerrgroupwhose limit comes from themax_concurrent_uploadsvalue returned by the Panel alongside the part URLs. Results are written to a pre-allocated slice so the part list stays ordered regardless of completion order.If the Panel does not include
max_concurrent_uploadsin its response (older Panel version) or sends an invalid value, Wings falls back to1, preserving the previous sequential behavior. Upgrading Wings without upgrading the Panel is therefore safe.This also fixes the retry path, which could never work: the request was built once outside the backoff loop, so a retry replayed a body that had already been consumed and sent an empty payload with a mismatched
Content-Length.The request is now rebuilt on every attempt and the section is rewound first.
The backoff's elapsed-time bound has been removed as well, since a single part can take longer than the previous one-minute budget, which meant a second attempt never happened. The two-hour request timeout and the retry count are unchanged.
Idle connections are now kept in the pool between parts instead of being renegotiated, and response bodies are drained so connections can be reused.
The results are as follows:
Most of the gain is already there at the lower concurrency levels, and the highest setting only buys a little more while getting close to the link's capacity. The sequential baseline is also the median of several runs that varied widely: with a single connection the total time depends entirely on how that one connection happens to behave, and one stalled part goes straight into the total. Concurrent uploads absorb that, so the win is in predictability as much as in raw throughput.
Summary by CodeRabbit
New Features
Reliability