HADOOP-19464. S3A: Restore Compatibility with EMRFS FileSystem #7410
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
steveloughran
requested changes
Feb 20, 2025
| * This test verifies that the EMRFS or legacy S3N filesystem compatibility with | ||
| * S3A works as expected. | ||
| */ | ||
| public class ITestEMRFSCompatibility extends AbstractS3ATestBase { |
There was a problem hiding this comment.
are there other tests here? I think I'd like
- list parent path reports an empty dir
- delete parent dir results in a getFileStatus(parent) => 404, and same for marker.
What does dir rename do? I know for normal / markers we only create a dir marker if there's nothing underneath, but that's just an optimisation. Here we'd want:
touch parent/src/subdir/$folder$
mv parent/src parent/dest
isDir(parent/dest/subdir)
isNotFound(parent/src)
There was a problem hiding this comment.
@steveloughran - It is a good call out.
list parent path reports an empty dir
This is already covered
delete parent dir results in a getFileStatus(parent) => 404, and same for marker.
This is not happening right now (even with hadoop-3.4.1)
There was a problem hiding this comment.
Added additional coverage
|
🎊 +1 overall
This message was automatically generated. |
|
@steveloughran - Thanks a lot for the review. |
|
🎊 +1 overall
This message was automatically generated. |
|
@steveloughran Gentle reminder for the review |
steveloughran
approved these changes
Feb 24, 2025
|
+1 pending you saying what your test run parameters were for the full test suite, +what kind of bucket. Really someone needs to make an s3 express bucket their default bucket, shouldn't they... |
|
Thanks a lot @steveloughran I will setup a S3 express bucket for future testing. Please do let me know if you need any testing for that bucket as well. |
adideshpande
pushed a commit
to adideshpande/hadoop
that referenced
this pull request
Feb 27, 2025
…e#7410) After HADOOP-19278, The S3N folder marker _$folder$ is not skipped during listing of S3 directories. This can lead to the S3A filesystem failing to read data written by the legacy Hadoop S3N filesystem and AWS EMR's EMRFS ("S3" filesystem) Contributed by Syed Shameerur Rahman
YanivKunda
pushed a commit
to YanivKunda/hadoop
that referenced
this pull request
Mar 23, 2025
…e#7410) After HADOOP-19278, The S3N folder marker _$folder$ is not skipped during listing of S3 directories. This can lead to the S3A filesystem failing to read data written by the legacy Hadoop S3N filesystem and AWS EMR's EMRFS ("S3" filesystem) Contributed by Syed Shameerur Rahman
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of PR
After HADOOP-19278 , The S3N folder marker _$folder$ is not skipped during listing of S3 directories leading to S3A filesystem not able to read data written by legacy Hadoop S3N filesystem and AWS EMR's EMRFS (S3 filesystem) leading to compatibility issues and possible migration risks to S3A filesystem.
How was this patch tested?
Added integration test
ITestEMRFSCompatibilityand ran other UT/IT inus-east-1regionFor code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?