fix: retry CompleteMultipartUpload on transient backend errors - #138
Merged
Conversation
Hetzner streams a 200 for CompleteMultipartUpload then can fail mid-response
with an embedded InternalError ("The server did not respond in time.") - a
documented S3-family quirk for this operation. Nothing retried it: botocore's
own retry checker only treats HTTP 5xx status and a small error-code allowlist
as transient (neither covers a 200 status with an error body), and the
handler bare-raised everything except EntityTooSmall. Scylla backups saw
these as failed multi-GB SSTable uploads and had to re-upload the whole file.
Add a bounded retry (reusing base.is_retryable_source_error) around
complete_multipart_upload. A plain retry isn't safe by itself: if the backend
actually finished assembling the object before the response died, the
upload_id is already invalidated, so the retry surfaces NoSuchUpload even
though the object is fine. Guard that case with a head_object check before
treating NoSuchUpload-after-retry as a real failure.
ServerSideHannes
added a commit
that referenced
this pull request
Jul 28, 2026
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
added a commit
that referenced
this pull request
Jul 28, 2026
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.
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
1 participant
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
Prod incident 2026-07-22: a Scylla backup run showed non-zero "Failed" bytes for
main.companieson 7/13 nodes. Traced through scylla-manager-agent -> rclone -> this proxy -> Hetzner backend:Hetzner streams a
200 OKforCompleteMultipartUploadthen can fail mid-response, embedding an<Error>InternalError</Error>in the body ("The server did not respond in time.") - a documented S3-family quirk for this specific operation. Nothing in the stack retried it:max_attempts=3, mode=adaptive) only treats HTTP 5xx status codes and a small error-code allowlist (RequestTimeout,RequestTimeoutException,PriorRequestNotComplete) as transient - neither a 200 status norInternalErrorqualifies._handle_complete_erroronly special-casedEntityTooSmall; every other code (includingInternalError) was a bareraise.rclone saw this as a failed upload and scylla-manager had to re-upload the entire multi-GB SSTable instead of just retrying one API call.
Fix
Wrap
complete_multipart_uploadin a bounded retry, reusing the existingis_retryable_source_errorclassifier frombase.py(already used for the analogousUploadPartCopy/GetObjectretry path).A plain retry isn't safe on its own: if the backend actually finished assembling the object before the response died, the
upload_idis already invalidated (S3 semantics: it dies once completion succeeds), so the retry surfacesNoSuchUploadeven though the object is fine. Added ahead_object-based check so aNoSuchUploadon retry is only treated as a real failure if the object doesn't actually exist with the expected size - otherwise it's treated as the success it actually was.Configurable via
S3PROXY_COMPLETE_RETRY_ATTEMPTS(default 4) /S3PROXY_COMPLETE_RETRY_BACKOFF(default 0.5s), matching the existingS3PROXY_SOURCE_READ_*env vars.Tests
tests/unit/test_complete_multipart_retry.py- 5 cases:AccessDenied) is not retried at allNoSuchUploadbut the object was actually already assembled -> recovered as success (the exact prod failure shape)NoSuchUploadand the object genuinely doesn't exist -> still fails (the safety net doesn't mask real failures)Confirmed these fail without the fix (AttributeError, method/constants don't exist pre-patch). Full
tests/unitsuite (596 tests) passes.