Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

object-store: remove S3ConditionalPut::ETagPutIfNotExists #6802

Merged
merged 4 commits into from
Nov 30, 2024

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Nov 26, 2024

Now that real S3 supports If-Match, we no longer need this special conditional put mode for real S3.

Considerations:

Which issue does this PR close?

Cleanup related to #6799.

Are there any user-facing changes?

The removal of S3ConditionalPut::ETagPutIfNotExists would be user facing, but this variant has not yet made it into a release.

cc @tustvold

@github-actions github-actions bot added the object-store Object Store Interface label Nov 26, 2024
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/
Now that real S3 supports `If-Match`, we no longer need this special
conditional put mode for real S3.
@benesch benesch force-pushed the object-store-s3-put-if-match-cleanup branch from 6a69ad1 to ac2b05c Compare November 26, 2024 16:14
@bentsku
Copy link

bentsku commented Nov 26, 2024

Quick update on the LocalStack side: I've got the green light to release this feature during this week, and will work on it either tonight or tomorrow. The feature will be available in the latest docker image then (I'll ping you once it is so you can also maybe test it locally?), and on Friday we will release localstack/localstack:4.0.3, so you could use that in CI. Sorry to hold you off!

@tustvold
Copy link
Contributor

That's amazing, thank you for working on this!

@benesch
Copy link
Contributor Author

benesch commented Nov 26, 2024

Sorry to hold you off!

Quite the opposite—thanks for turning the implementation around so quickly!

@bentsku
Copy link

bentsku commented Nov 27, 2024

As written in #6799 (comment), the feature is now available in the latest image tag, and will be available in a tagged release (4.0.3) on Friday 29th (time still be decided, depending on what needs to get in).

I've given a try to this branch with the flag enabled and it worked for me locally, might be worth trying it out. Thanks a lot for the tests, they're always great to run locally! 🙏

@benesch
Copy link
Contributor Author

benesch commented Nov 28, 2024

Yes, just bumped to the latest localstack image in this PR and CI passed with flying colors! 🎉 Thanks very much, @bentsku.

Give me a holler when 4.0.3 is released—I'll try to keep an eye on it myself—so that I can update this PR to use it. And then we should be ready to merge.

@bentsku
Copy link

bentsku commented Nov 29, 2024

Hello @benesch! The 4.0.3 image is now available! A bit later than initially planned, we were ironing some quirks in other AWS services, but things are all good now.

Can't wait to see this feature merged! 🚀 thanks a lot for the collaboration on this. Have a great week-end 😄

@tustvold tustvold merged commit c60ce14 into apache:main Nov 30, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants