HADOOP-19044: AWS SDK V2 - Update S3A region logic #6482
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If both fs.s3a.endpoint & fs.s3a.endpoint.region are empty, Spark will set fs.s3a.endpoint to s3.amazonaws.com here:https://github.com/apache/spark/blob/9a2f39318e3af8b3817dc5e4baf52e548d82063c/core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala#L540 HADOOP-18908, updated the region logic such that if fs.s3a.endpoint.region is set, or if a region can be parsed from fs.s3a.endpoint (which will happen in this case, region will be US_EAST_1), cross region access is not enabled. This will cause 400 errors if the bucket is not in US_EAST_1. Proposed: Updated the logic so that if the endpoint is the global s3.amazonaws.com , cross region access is enabled.
|
🎊 +1 overall
This message was automatically generated. |
ahmarsuhail
reviewed
Jan 22, 2024
There was a problem hiding this comment.
Thanks @HarshitGupta11 , I think we should go with @virajjasani's implementation #6479 .. let me know what you think.
| region = getS3RegionFromEndpoint(parameters.getEndpoint()); | ||
| if (region != null) { | ||
| origin = "endpoint"; | ||
| if(parameters.getEndpoint().equals(CENTRAL_ENDPOINT)){ |
There was a problem hiding this comment.
if someone configures fs.s3a.endpoint to s3.amazonaws.com and sets region in fs.s3a.endpoint.region to eu-west-1, this code will start ignoring what we have in fs.s3a.endpoint.region and just enable cross region for everything. I think this could be risky as people could just have s.s3a.endpoint to s3.amazonaws.com in their core-site.xml as it doesn't make a difference if you've set your region right.
Cool, I will close this pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If both fs.s3a.endpoint & fs.s3a.endpoint.region are empty, Spark will set fs.s3a.endpoint to
s3.amazonaws.com here:https://github.com/apache/spark/blob/9a2f39318e3af8b3817dc5e4baf52e548d82063c/core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala#L540
HADOOP-18908, updated the region logic such that if fs.s3a.endpoint.region is set, or if a region can be parsed from fs.s3a.endpoint (which will happen in this case, region will be US_EAST_1), cross region access is not enabled. This will cause 400 errors if the bucket is not in US_EAST_1.
Proposed: Updated the logic so that if the endpoint is the global s3.amazonaws.com , cross region access is enabled.
Description of PR
How was this patch tested?
Its being currently tested against us-west-2 by explicitly setting the endpoint as s3.amazonaws.com
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?