fix: retry CompleteMultipartUpload on transient backend errors#138
Open
ServerSideHannes wants to merge 1 commit into
Open
fix: retry CompleteMultipartUpload on transient backend errors#138ServerSideHannes wants to merge 1 commit into
ServerSideHannes wants to merge 1 commit into
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.
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.