object-store: support real S3's If-Match semantics #6801
Conversation
As of today [0] S3 now supports the If-Match for in-place conditional
writes. This commit adjusts the existing support for
S3ConditionalPut::Etag mode for compatibility with real S3's particular
semantics, which vary slightly from MinIO and R2. Specifically:
* Real S3 can occasionally return 409 Conflict when concurrent
If-Match requests are in progress. These requests need to be
retried.
* Real S3 returns 404 Not Found instead of 412 Precondition Failed
when issuing an If-Match request against an object that does not
exist.
Fix apache#6799.
[0]: https://aws.amazon.com/about-aws/whats-new/2024/11/amazon-s3-functionality-conditional-writes/
benesch
force-pushed
the
object-store-s3-put-if-match
branch
from
f2ba56a
to
5535851
Compare
benesch
changed the title
| { | ||
| // Real S3 reports NotFound rather than PreconditionFailed when the | ||
| // object doesn't exist. Convert to PreconditionFailed for | ||
| // consistency with R2. This also matches what the HTTP spec |
You bet! Anything else you need from me to merge this? |
|
No, I think this looks good, but given the localstack update may land shortly I was going to hold this until then just to ensure there aren't any gremlins lurking |
Makes sense! In that case would you prefer if I closed this PR in favor of #6802, so there's just one PR to review and merge after the new localstack release comes out? Or is it still useful to have both PRs? |
|
Let's keep it for now just in case there's some unforeseen hickup with the localstack release, but yes presuming localstack lands we'll probably just merge the other PR |
|
Sounds good. |
As of today (0) S3 now supports the If-Match for in-place conditional writes. This commit adjusts the existing support for S3ConditionalPut::Etag mode for compatibility with real S3's particular semantics, which vary slightly from MinIO and R2. Specifically:
Real S3 can occasionally return 409 Conflict when concurrent If-Match requests are in progress. These requests need to be retried.
Real S3 returns 404 Not Found instead of 412 Precondition Failed when issuing an If-Match request against an object that does not exist.
I tested this against real S3, since localstack doesn't yet support
If-Match. #6802 is a follow-up PR that will update CI to test this new support once we have a version of localstack to test against.Fix #6799.
Which issue does this PR close?
#6799
Are there any user-facing changes?
None beyond what's described in the PR description.
cc @tustvold