Make block uploads replayable and retry storage 5xx responses - #9
Open
jomplox wants to merge 1 commit into
Open
Conversation
UploadBlock handed an io.Reader straight to resty's multipart helper. A request consumes that reader, so when the shared client retried after a dropped connection the second attempt could not contain the original encrypted block bytes. An HTTP 5xx from the storage endpoint was also not a retry condition, although a transient 502 from /storage/blocks is what started the production stall this was found in. Serialize the multipart form once into a byte buffer, keep the generated Content-Type (with its boundary) and give resty the byte slice as the body, so every retry resends the identical complete body. Add a request scoped retry condition for 500-599. The client's bounded retry count is unchanged. Cost: one extra in-memory copy of each in-flight block (4 MiB each, bounded by the caller's upload semaphore). Add regression tests against a local HTTP test server: one drops the connection on the first attempt and asserts the replayed body is byte-identical; one returns 502 first and asserts the retry carries the same body. Both fail before this change.
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.
What this fixes
UploadBlockpasses anio.Readerstraight to resty's multipart helper. A request consumes that reader, so when the shared client retries after a dropped connection (RetryConditionsalready cover dial errors and connection drops) the second attempt cannot contain the original encrypted block bytes. An HTTP 5xx from the storage endpoint is also not a retry condition today.In production (rclone v1.74.4,
rclone syncof ~1.1M files to Proton Drive) the sequence was: a transient502 POST .../storage/blocks, no replayable retry, the file failed, and the surrounding Proton-API-Bridge upload path then leaked semaphore permits until rclone sat idle forever with no open sockets (that second half is rclone/Proton-API-Bridge#8).Change
bytes.Buffer, keep the generatedContent-Type(with boundary) and give resty the byte slice as the body, so every retry resends the identical complete body.AddRetryConditionfor HTTP 500–599 on block uploads only. The client's bounded retry count and backoff are unchanged.Cost: one extra in-memory copy of each in-flight block (4 MiB each, bounded by the caller's upload semaphore, ~80 MiB at the bridge's default of 20).
Tests
Two regression tests against
httptestservers, no Proton account needed:TestUploadBlockReplaysMultipartBodyAfterConnectionDrop: the server hijacks and closes the first connection, then asserts the replayed body is byte-identical to the original block.TestUploadBlockRetriesBadGatewayWithSameBody: the server returns 502 first, then asserts the retry carries the same body.Both fail on master and pass with this change.
go vetclean.Production evidence
Running as
rclone v1.74.4-DEV+protonfix.1since 2026-09-01 on a 1.1M-file sync. The log recorded a real connection reset while writing a block at 2026-09-01 23:07 UTC and the sync continued; before the patch the same sync had stalled permanently after a 502.