HBASE-24055 Make AsyncFSWAL can run on EC cluster #1437
Merged
Conversation
This file contains 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
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
OK, good. All green. Can I get a review? @saintstack @ndimiduk I think this should be included in 2.3. @infraio Do you think this should also go into 2.2.x? |
ndimiduk
approved these changes
Apr 7, 2020
There was a problem hiding this comment.
Changes look good. wal package tests in general all pass. This one specifically passes without triggering the assumption on JDK11+Hadoop3 BUT it does trigger the assumption and skip on JDK8+Hadoop2. I believe this is to be expected as the EC feature added in HDFS-7285 has a fixVersion of 3.0.0-alpha1.
| try { | ||
| return CreateFlag.valueOf("SHOULD_REPLICATE"); | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.debug("can not find SHOULD_REPLICATE flag, should be hadoop 2.x", e); |
There was a problem hiding this comment.
Should this be logged at warn instead?
There was a problem hiding this comment.
I used to make all the adaptor code to log as WARN but it was proven to be annoying for most users and we changed them all to debug...
There was a problem hiding this comment.
This happens every time we open a file in hadoop2? Or just once on class loading? (Looks like the latter).
There was a problem hiding this comment.
This is how we test if the the SHOULD_REPLICATE flag is available? It is not available if hadoop2? If so, can we have a comment on the method to this effect? The log is a little confusing. Suggest: "SHOULD_REPLICATE is not available; this is a problem if we are running on hadoop3 (Its expected if hadoop2)".
There was a problem hiding this comment.
Only once when loading.
There was a problem hiding this comment.
All the other log messages are just like this, so do not want to only change this one. Let me add some comments on the field.
| @@ -502,8 +524,8 @@ private static FanOutOneBlockAsyncDFSOutput createOutput(DistributedFileSystem d | |||
| try { | |||
| stat = FILE_CREATOR.create(namenode, src, | |||
| FsPermission.getFileDefault().applyUMask(FsPermission.getUMask(conf)), clientName, | |||
| new EnumSetWritable<>(overwrite ? EnumSet.of(CREATE, OVERWRITE) : EnumSet.of(CREATE)), | |||
| createParent, replication, blockSize, CryptoProtocolVersion.supported()); | |||
| getCreateFlags(overwrite), createParent, replication, blockSize, | |||
|
And +1 for branch-2.3 |
Yes, EC is only available in hadoop 3.x. |
|
Any other concerns? @saintstack |
saintstack
approved these changes
Apr 7, 2020
| try { | ||
| return CreateFlag.valueOf("SHOULD_REPLICATE"); | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.debug("can not find SHOULD_REPLICATE flag, should be hadoop 2.x", e); |
There was a problem hiding this comment.
This happens every time we open a file in hadoop2? Or just once on class loading? (Looks like the latter).
| try { | ||
| return CreateFlag.valueOf("SHOULD_REPLICATE"); | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.debug("can not find SHOULD_REPLICATE flag, should be hadoop 2.x", e); |
There was a problem hiding this comment.
This is how we test if the the SHOULD_REPLICATE flag is available? It is not available if hadoop2? If so, can we have a comment on the method to this effect? The log is a little confusing. Suggest: "SHOULD_REPLICATE is not available; this is a problem if we are running on hadoop3 (Its expected if hadoop2)".
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
saintstack
approved these changes
Apr 7, 2020
asfgit
pushed a commit
that referenced
this pull request
Apr 7, 2020
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org>
asfgit
pushed a commit
that referenced
this pull request
Apr 7, 2020
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org>
thangTang
pushed a commit
to thangTang/hbase
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org>
thangTang
pushed a commit
to thangTang/hbase
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org>
thangTang
pushed a commit
to thangTang/hbase
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: stack <stack@apache.org>
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.
No description provided.