Retry 409 Conflict for PutMode::Create conditional puts in the S3 backend - #852
adriancaruana wants to merge 1 commit into
Conversation
The If-Match (Update) branch already retries on 409. The If-None-Match (Create) branch did not. AWS documents a 409 for If-None-Match when a concurrent delete of the key completes before the write does, and states that PutObject uploads may be retried after a 409. Concurrent conditional creates on the same key instead return 412, which this branch already maps to AlreadyExists. A 409 means the request reached S3 and nothing was created, so the retry cannot turn a successful create into a reported conflict. This does not set `.idempotent(true)`, so transport-level retry behavior is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Cna you explain what the end user visible issue / problem this PR addresses? Like what operation are you doing that fails? |
|
@alamb I am trying to use PutMode::Create to say whether we (the caller) created the object, and a transient 409 coms back as AlreadyExists just like a real conflict does, which is ambiguous. Retrying the 409 (the way the If-Match branch already) makes AlreadyExists unambiguity.
fyi, I haven't actually encountered this kind off failure. But I don't prefer if I didn't. 😄 |
Make sense. What I am trying to figure out is if this error is actually possible. Otherwise we are just making out code more complex for no reason |
|
Fair criticism. Sorry, I should have provided this in the PR description. S3 docs say:
Here's a similar issue on opendal apache/opendal#6741 |
2 participants
Which issue does this PR close?
Closes #.
Rationale for this change
The
If-Matchbranch ofAmazonS3::put_optsalready sets.retry_on_conflict(true). TheIf-None-Matchbranch does not, although AWS documents a 409 for it too: a conditional write returns409 Conflictwhen a concurrent delete to the key succeeds before the write completes, and the docs state that "uploads may be retried after receiving a409 Conflicterror". Concurrent conditional creates on the same key return 412, which this branch already maps toAlreadyExists.Without the retry, that 409 reaches the caller as
Error::AlreadyExists, via theCONFLICTmapping inclient/retry.rs. That is the same error the Create branch deliberately returns for the settled 412/304 case, so a caller cannot distinguish "the object exists" from "a competing create was in flight and nothing landed". Callers usingPutMode::Createas a mutual-exclusion primitive are all told a holder exists when no create landed, so nothing claims the work.What changes are included in this PR?
One line:
.retry_on_conflict(true)on the(PutMode::Create, S3ConditionalPut::ETagMatch)branch insrc/aws/mod.rs, with a comment matching the sibling branch.Are there any user-facing changes?
No API change. A conditional create that previously failed with
AlreadyExistson a transient conflict is now retried under the existing retry policy.Testing
No unit test added. The sibling
If-Matchretry has none either, and the 409 path is only exercised by the credentials-gatedtest_conditional_put. I did not want to add mock-server infrastructure for a one-line change, but happy to if you would prefer it.Verified with
cargo check,clippyandfmt --checkunder--no-default-features --features aws-base. I could not build--all-featureslocally (ring/aws-lc-rsneed a C compiler I do not have on this machine).I have not reproduced the 409 against real S3. The behaviour is from AWS documentation and from the sibling branch's comment, not from an observed failure.
AI disclosure
Written with AI assistance (Claude).