HDFS-16776 Erasure Coding: The length of targets should be checked when DN gets a reconstruction task #4901
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
tasanuma
reviewed
Sep 21, 2022
Comment on lines
295
to
298
| int getIndicesNum(){ | ||
| return liveIndiceNum; | ||
| } | ||
|
|
There was a problem hiding this comment.
@kidd53685368 Thanks for your PR.
How about using BitSet#cardinality instead of calculating liveIndiceNum?
Suggested change
| int getIndicesNum(){ | |
| return liveIndiceNum; | |
| } | |
| int getNumLiveBlocks(){ | |
| return liveBitSet.cardinality(); | |
| } | |
There was a problem hiding this comment.
Thanks, this is absolutely a better substitution.
tasanuma
reviewed
Sep 21, 2022
| @@ -84,6 +84,8 @@ class StripedWriter { | |||
| writers = new StripedBlockWriter[targets.length]; | |||
|
|
|||
| targetIndices = new short[targets.length]; | |||
| Preconditions.checkArgument(targetIndices.length <= dataBlkNum + parityBlkNum - reconstructor.getIndicesNum(), | |||
There was a problem hiding this comment.
Could you fix the checkstyle error?
Line is longer than 100 characters (found 114)
|
💔 -1 overall
This message was automatically generated. |
tasanuma
reviewed
Sep 22, 2022
| targetIndices = new short[targets.length]; | ||
| Preconditions.checkArgument( | ||
| targetIndices.length <= dataBlkNum + parityBlkNum - reconstructor.getNumLiveBlocks(), | ||
| "Reconstrutcion work gets too much targets."); |
There was a problem hiding this comment.
@kidd53685368 Sorry, I have one more request. Reconstrutcion is a typo of Reconstruction.
|
💔 -1 overall
This message was automatically generated. |
tasanuma
approved these changes
Sep 23, 2022
|
Merged. Thanks for your contribution, @kidd53685368. |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…en DN gets a reconstruction task (apache#4901)
|
As mentioned in HDFS-16739, in order to satisfy the storage policy, the length of This change may cause the reconstruction task to never succeed to satisfy the storage policy (unless the mover is manually triggered to make the block satisfy the storage policy). |
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
JIRA: HDFS-16776
The length of targets should be checked when DN gets a EC reconstruction task.For some reason (HDFS-14768, HDFS-16739) , the length of targets will be larger than additionalReplRequired which causes some elements in targets get the default value 0. It may trigger the bug which leads to the data corrupttion just like HDFS-14768.