fix: retry UploadPart on transient backend errors - #142
Closed
ServerSideHannes wants to merge 1 commit into
Closed
Conversation
ServerSideHannes
force-pushed
the
fix/upload-part-retry-transient
branch
from
July 28, 2026 20:01
36e1aea to
ac80c99
Compare
RGW can accept a part, commit to a 200, stream it, and only then put the
failure in the body ("InternalError: The server did not respond in time.",
status code: 200). Three retry layers all miss that shape because each decides
from the HTTP status line, which already read 200:
- rclone's LowLevelRetries gates on status 429/500/503 (backend/s3/s3.go)
- rclone's string fallback matches only transport phrases ("use of closed
network connection", "unexpected EOF reading trailer"), not InternalError
- botocore's max_attempts in S3Client sees the same 200
#133 added this retry to UploadPartCopy and #138 to CompleteMultipartUpload,
but plain UploadPart was left bare -- and it is the only one of the three a
Scylla backup uses (267 PUT, 0 UploadPartCopy measured against the backup
bucket). At 50MB rclone chunks a 6.2GB SSTable is ~762 internal PUTs, so one
unretried transient failure loses the whole multi-GB file.
2026-07-28 prod: 10 of 13 racks failed, ~2.4TiB never uploaded, every failure
on main.companies (47 of 48 large files).
Reuses the existing base.SOURCE_READ_ATTEMPTS machinery from #133, so no new
tunables. Safe at both call sites: the full ciphertext is still in memory when
upload_part is called (the del happens after), so a retry re-PUTs identical
bytes to the same internal part number.
ServerSideHannes
force-pushed
the
fix/upload-part-retry-transient
branch
from
July 28, 2026 20:11
ac80c99 to
2683672
Compare
Owner
Author
|
Superseded by #143 (same change, signed with the correct key so the commit verifies). |
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.
Problem
RGW can accept a part, commit to a 200, stream it, and only then put the failure in the body:
Three retry layers all miss that shape, because each decides from the HTTP status line — which already read 200:
LowLevelRetries = 20backend/s3/s3.go:1393)InternalErrormax_attempts: 3s3proxy is the only layer that parses the body and can see the real error.
#133 added this retry to
UploadPartCopyand #138 toCompleteMultipartUpload, but plainUploadPartwas left bare — and it's the only one of the three a Scylla backup uses (267 PUT, 0 UploadPartCopy measured against the backup bucket). At 50MB rclone chunks a 6.2GB SSTable is ~762 internal PUTs, so one unretried transient failure loses the whole multi-GB file.2026-07-28 prod: 10 of 13 racks failed, ~2.4TiB never uploaded, every failure on
main.companies(47 of 48 large files).Fix
_upload_part_with_retry, mirroring_complete_multipart_upload_with_retryfrom #138, wired into both call sites inupload_part.py:_stream_and_upload_framed_upload_internal_part_with_semaphoreReuses the existing
base.SOURCE_READ_ATTEMPTS/SOURCE_READ_BACKOFF_SEC/is_retryable_source_errormachinery from #133, so there are no new tunables andInternalErroris already in the retryable set.Why a retry is safe here: at both sites the full ciphertext is still in memory when
upload_partis called — thedel ciphertexthappens after. So a retry re-PUTs identical bytes to the same internal part number, no body re-read needed. Same idempotency argument #133 used for the copy path.Verification
ruff check/format --checkThat last row matters — the retry loop was neutered to confirm the tests actually catch the bug rather than passing vacuously.
Scope
This makes an individual PUT survive a transient upstream failure. It does not protect a PUT in flight when its pod is terminated — that trigger is KEDA scale-in cliffs (30→21→10 in two minutes on 07-28), addressed separately by capping the HPA
scaleDownrate. With this retry a cut part is re-attempted and lands on a surviving pod, so the file completes instead of dying.