Skip to content

Make block uploads replayable and retry storage 5xx responses - #9

Open
jomplox wants to merge 1 commit into
rclone:masterfrom
jomplox:fix/replayable-block-upload
Open

Make block uploads replayable and retry storage 5xx responses#9
jomplox wants to merge 1 commit into
rclone:masterfrom
jomplox:fix/replayable-block-upload

Conversation

@jomplox

@jomplox jomplox commented Sep 2, 2026

Copy link
Copy Markdown

What this fixes

UploadBlock passes an io.Reader straight to resty's multipart helper. A request consumes that reader, so when the shared client retries after a dropped connection (RetryConditions already 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 sync of ~1.1M files to Proton Drive) the sequence was: a transient 502 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

  • Serialize the multipart form once into a bytes.Buffer, keep the generated Content-Type (with boundary) and give resty the byte slice as the body, so every retry resends the identical complete body.
  • Add a request-scoped AddRetryCondition for 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 httptest servers, 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 vet clean.

Production evidence

Running as rclone v1.74.4-DEV+protonfix.1 since 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.

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.
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.

1 participant