fix(objectstore): Add retry logic and structured error handling to S3 readObject()#59198
Draft
joshtrichards wants to merge 3 commits intomasterfrom
Draft
fix(objectstore): Add retry logic and structured error handling to S3 readObject()#59198joshtrichards wants to merge 3 commits intomasterfrom
readObject()#59198joshtrichards wants to merge 3 commits intomasterfrom
Conversation
Since we bypass the SDK to support #20033, we need to emulate more of its behavior to keep readObject consistent with other transactions. This adds basic retry behavior for the main scenarios. Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
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.
Summary
readObject()bypasses the AWS SDK's HTTP transport to enable seekable reads via HTTP range requests (SeekableHttpStream). A side effect is that the SDK's retry middleware and error parsing -- configured viaretriesMaxAttempts-- have no effect on read operations. This means transient S3 failures (503, 429,connection timeouts) cause immediate failure, and all errors surface as the generic
"Failed to read object"with no S3 error context.Changes:
retriesMaxAttemptsconfigignore_errorsstream context option sofopen()returns a stream on 4xx/5xx instead of barefalse, allowing inspection of the actual responsex-amz-request-id, andx-amz-id-2from both the XML body and response headerswarningfor retryable failures (visible even when retries succeed) anderrorfor terminal failures"Failed to read object $urn"wrapper_datain reverse to handle 100 Continue / redirect / proxy response chainsContext:
See PR #20033 for the original motivation behind the
SeekableHttpStreamapproach. The SDK bypass remains necessary for seekable range-request streams; this change brings the error handling closer to parity with what the SDK provides for all other S3 operations.Not in scope:
S3ConnectionTrait/S3ObjectTraitinto a shared abstract method (flagged with a TODO for a follow-up)TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)