Support native S3 conditional writes by benesch · Pull Request #6682 · apache/arrow-rs

benesch · 2024-11-05T07:06:27Z

Add support for PutMode::Create and copy_if_not_exists on native AWS S3, which uses the underlying conditional write primitive that Amazon launched earlier this year 0.

The conditional write primitive is simpler than what's available in other S3-like products (e.g., R2), so new modes for s3_copy_if_not_exists and s3_conditional_put are added to select the native S3-specific behavior.

To maintain strict backwards compatibility (e.g. with older versions of LocalStack), the new behavior is not on by default. It must be explicitly requested by the end user.

The implementation for PutMode::Create is straightforward. The implementation of copy_if_not_exists is a bit more involved, as it requires managing a multipart upload that uses the UploadPartCopy operation, which was not previously supported by this crate's S3 client.

To ensure test coverage, the object store workflow now runs the AWS integration tests with conditional put both disabled and enabled.

Which issue does this PR close?

Fix apache/arrow-rs-object-store#54.

benesch · 2024-11-05T07:07:23Z

@tustvold are you the right person to review this? I saw you implemented quite a bit of the previous conditional put/get support.

benesch · 2024-11-05T07:15:17Z

To maintain strict backwards compatibility (e.g. with older versions of LocalStack), the new behavior is not on by default. It must be explicitly requested by the end user.

This isn't ideal, but it seemed best for the short term to avoid breaking backcompat for anyone who might be using a version of S3 that doesn't support CAS. In the long term, I think the ideal would definitely be to have the native S3 CAS support enabled by default (i.e. unless overridden explicitly by the user).

tustvold

Thank you, this looks good to me, mostly just some minor nits.

I agree with the decision to leave this off by default for a couple of reasons:

S3 compatible stores support these mechanisms with fewer drawbacks (e.g. full conditional update, native copy_if_not_exists, etc...) and users should prefer these
AWS may eventually bring S3 into feature parity with literally every other object store, at which point this will provide a coherent API to converge on

tustvold · 2024-11-05T15:45:55Z

tustvold · 2024-11-05T15:48:04Z

Suggested change

ETagCreateOnly,

ETagPutIfNotExists,

Maybe? I don't feel strongly though

Sure, works for me.

tustvold · 2024-11-05T15:51:21Z

👍

I had a brief scan and couldn't see a way around this

tustvold · 2024-11-05T15:52:15Z

Suggested change

Inline(PutPayload),

Part(PutPayload),

Maybe?

Sure, no strong feels from me on the name here!

tustvold · 2024-11-05T15:53:31Z

Do we need to use encode_path here?

Ah yeah, seems like it, thanks!

tustvold · 2024-11-05T16:00:31Z

Do we need to abort the multipart upload in this case? Does this happen automatically?

Oof, good call. It does not happen automatically. I added code to do a best effort multipart upload, and a docs note that this isn't guaranteed and that the user should configure automatic multipart expiration via a lifecycle rule.

tustvold · 2024-11-05T16:01:20Z

Could possibly also link to the module docs around lifecycle rules for cleaning up incomplete multipart uploads - https://docs.rs/object_store/latest/object_store/aws/index.html#multipart-uploads

Good call, done!

calavera · 2024-11-05T19:09:54Z

should the path also be in the displayed error?

Good call, done.

Add support for `PutMode::Create` and `copy_if_not_exists` on native AWS S3, which uses the underlying conditional write primitive that Amazon launched earlier this year [0]. The conditional write primitive is simpler than what's available in other S3-like products (e.g., R2), so new modes for `s3_copy_if_not_exists` and `s3_conditional_put` are added to select the native S3-specific behavior. To maintain strict backwards compatibility (e.g. with older versions of LocalStack), the new behavior is not on by default. It must be explicitly requested by the end user. The implementation for `PutMode::Create` is straightforward. The implementation of `copy_if_not_exists` is a bit more involved, as it requires managing a multipart upload that uses the UploadPartCopy operation, which was not previously supported by this crate's S3 client. To ensure test coverage, the object store workflow now runs the AWS integration tests with conditional put both disabled and enabled. Fix #6285. [0]: https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/

benesch

Thanks very much for the quick review, @tustvold! Addressed all your feedback, I believe.

benesch · 2024-11-05T20:44:54Z

Good call, done.

benesch · 2024-11-05T20:48:32Z

Sure, no strong feels from me on the name here!

benesch · 2024-11-05T21:16:49Z

Ah yeah, seems like it, thanks!

benesch · 2024-11-06T02:45:47Z

Good call, done!

benesch · 2024-11-06T02:48:36Z

Sure, works for me.

benesch · 2024-11-06T03:01:26Z

Oof, good call. It does not happen automatically. I added code to do a best effort multipart upload, and a docs note that this isn't guaranteed and that the user should configure automatic multipart expiration via a lifecycle rule.

To a version that supports conditional writes.

benesch · 2024-11-08T00:50:01Z

@tustvold if you can give me another re-run here I think we’ll be all green on CI. I fixed a minor clippy failure and upgraded to a version of localstack that supports conditional put.

benesch · 2024-11-08T14:55:09Z

Whew, all tests here are green! @tustvold let me know if there's anything else here you'd like to see, but from my perspective this is ready to (squash) merge.

tustvold

Just a small nit, then I think we're good to go. Thank you for this

tustvold · 2024-11-08T14:57:22Z

+                    let part_id = self
+                        .client
+                        .put_part(to, &upload_id, 0, PutPartPayload::Copy(from))
+                        .await?;


I think this error should also trigger cleanup, might be worth encapsulating the steps into a function

It does, doesn't it? If anything in this async block (L306-329) returns an error, the cleanup on L336 should fire.

Oh I missed that this is wrapped in an async block, cunning

The fact that async blocks can function like try blocks is one of my favorite Rust tricks!

criccomini · 2024-11-08T20:00:23Z

Amazing. Any idea when this might go out 🔥

tustvold · 2024-11-08T20:27:06Z

apache/arrow-rs-object-store#306 tracks the next release

benesch · 2024-11-09T16:16:00Z

Thanks very much for the review and merge, @tustvold!

tustvold · 2024-11-25T23:11:00Z

+    /// Encoded as `etag-put-if-not-exists` ignoring whitespace.
+    ///
+    /// [announcement]: https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/
+    ETagPutIfNotExists,


S3 now supports the full header so I wonder if we need this anymore?

https://github.com/apache/arrow-rs/issues/6799

Indeed! I put up #6802 to remove it.

* Support native S3 conditional writes Add support for `PutMode::Create` and `copy_if_not_exists` on native AWS S3, which uses the underlying conditional write primitive that Amazon launched earlier this year [0]. The conditional write primitive is simpler than what's available in other S3-like products (e.g., R2), so new modes for `s3_copy_if_not_exists` and `s3_conditional_put` are added to select the native S3-specific behavior. To maintain strict backwards compatibility (e.g. with older versions of LocalStack), the new behavior is not on by default. It must be explicitly requested by the end user. The implementation for `PutMode::Create` is straightforward. The implementation of `copy_if_not_exists` is a bit more involved, as it requires managing a multipart upload that uses the UploadPartCopy operation, which was not previously supported by this crate's S3 client. To ensure test coverage, the object store workflow now runs the AWS integration tests with conditional put both disabled and enabled. Fix #6285. [0]: https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/ * Address review feedback * Fix clippy failure * Upgrade localstack in GitHub Actions To a version that supports conditional writes.

github-actions Bot added the object-store label Nov 5, 2024

This was referenced Nov 5, 2024

object_store: Conditional put and rename_if_not_exist on S3 apache/arrow-rs-object-store#54

Closed

Support S3 CAS now that PUT If-none-match: * is supported slatedb/slatedb#164

Closed

tustvold approved these changes Nov 5, 2024

View reviewed changes

calavera reviewed Nov 5, 2024

View reviewed changes

benesch force-pushed the object-store-preconditions-native-s3 branch 2 times, most recently from dc2a634 to 075c524 Compare November 6, 2024 02:55

Address review feedback

9cf76bc

benesch force-pushed the object-store-preconditions-native-s3 branch from 075c524 to 9cf76bc Compare November 6, 2024 02:56

benesch commented Nov 6, 2024

View reviewed changes

kylebarron mentioned this pull request Nov 6, 2024

fsspec integration developmentseed/obstore#63

Merged

benesch added 2 commits November 7, 2024 17:35

Fix clippy failure

a974e76

Upgrade localstack in GitHub Actions

8bbf641

To a version that supports conditional writes.

tustvold reviewed Nov 8, 2024

View reviewed changes

tustvold merged commit c36ff79 into apache:master Nov 8, 2024

kylebarron mentioned this pull request Nov 14, 2024

Support native S3 conditional writes developmentseed/obstore#89

Closed

tustvold reviewed Nov 25, 2024

View reviewed changes

alamb mentioned this pull request Dec 17, 2024

Support S3 Put IfMatch apache/arrow-rs-object-store#33

Closed

criccomini mentioned this pull request Dec 24, 2024

Upgrade object_store to support put_if_not_exists on S3 without DynamoDB slatedb/slatedb#419

Merged

alamb mentioned this pull request Jun 17, 2025

refactor: remove AWS dynamo integration apache/arrow-rs-object-store#407

Merged

Uh oh!

Conversation

benesch commented Nov 5, 2024

Which issue does this PR close?

Uh oh!

benesch commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benesch commented Nov 5, 2024

Uh oh!

tustvold left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benesch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benesch commented Nov 8, 2024

Uh oh!

benesch commented Nov 8, 2024

Uh oh!

tustvold left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criccomini commented Nov 8, 2024

Uh oh!

tustvold commented Nov 8, 2024

Uh oh!

benesch commented Nov 9, 2024

Uh oh!

Choose a reason for hiding this comment

benesch commented Nov 5, 2024 •

edited

Loading