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

Support S3 Express One Zone #642

Merged
merged 5 commits into from
Nov 28, 2023
Merged

Conversation

monthonk
Copy link
Contributor

Description of change

This change configure mountpoint-s3-client to use the new SigV4-Express signing algorithm when we detected that the bucket is an S3 Express One Zone bucket.

The tests are modified to support testing against S3 Express One Zone buckets, we can enable them by providing environment variables S3_BUCKET_NAME, S3_EXPRESS_ONE_ZONE_ENDPOINT and run the test with s3express_tests feature flag. For example. if the S3 Express One Zone bucket name is doc-example-bucket--use1-az5--x-s3, we can set environment variables with the following values

  • S3_BUCKET_NAME=doc-example-bucket--use1-az5--x-s3
  • S3_EXPRESS_ONE_ZONE_ENDPOINT=https://doc-example-bucket--use1-az5--x-s3.s3express-use1-az5.us-east-1.amazonaws.com

This PR also includes @jamesbornholt's change to fix a problem where ListObjectsV2 is unordered on S3 Express. This is an issue for our shadowing semantics in readdir, which does a merge-sort of the local and remote keys, assuming they're both ordered.

We are going to give up on readdir being ordered on Express -- it's not required by VFS, and the alternative would be to buffer all directory entries in memory before returning any, which would be too expensive on large directories. So this change introduces a new unordered variant of the readdir implementation. It just returns directory entries verbatim from ListObjectsV2, and then returns all otherwise-unreturned local entries at the end of the iterator.

Doing this requires Mountpoint to know whether the bucket is a directory bucket or not, and so I added both a command-line flag and auto-detection for that.

Does this change impact existing behavior?

No breaking changes.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

monthonk and others added 5 commits November 28, 2023 15:47
Signed-off-by: Monthon Klongklaew <[email protected]>
ListObjectsV2 is unordered on S3 Express. This is an issue for our
shadowing semantics in readdir, which does a merge-sort of the local and
remote keys, assuming they're both ordered.

We are going to give up on readdir being ordered on Express -- it's not
required by VFS, and the alternative would be to buffer all directory
entries in memory before returning any, which would be too expensive on
large directories. So this change introduces a new unordered variant of
the readdir implementation. It just returns directory entries verbatim
from ListObjectsV2, and then returns all otherwise-unreturned local
entries at the end of the iterator.

Doing this requires Mountpoint to know whether the bucket is a directory
bucket or not, and so I added both a command-line flag and
auto-detection for that.

Signed-off-by: James Bornholt <[email protected]>
Signed-off-by: Monthon Klongklaew <[email protected]>
Signed-off-by: Monthon Klongklaew <[email protected]>
@monthonk monthonk temporarily deployed to PR integration tests November 28, 2023 15:49 — with GitHub Actions Inactive
@monthonk monthonk temporarily deployed to PR integration tests November 28, 2023 15:49 — with GitHub Actions Inactive
@monthonk monthonk temporarily deployed to PR integration tests November 28, 2023 15:49 — with GitHub Actions Inactive
@monthonk monthonk temporarily deployed to PR integration tests November 28, 2023 15:49 — with GitHub Actions Inactive
@monthonk monthonk changed the title S3 express one zone Support S3 Express One Zone Nov 28, 2023
@monthonk monthonk requested a review from dannycjones November 28, 2023 15:56
Copy link
Contributor

@dannycjones dannycjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@dannycjones dannycjones enabled auto-merge (squash) November 28, 2023 15:59
@dannycjones dannycjones merged commit f4146fa into awslabs:main Nov 28, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants