HDFS-16456. EC: Decommission a rack with only on dn will fail when the rack number is equal with replication#4126
Merged
tasanuma merged 4 commits intoapache:trunkfrom Apr 14, 2022
Merged
Conversation
Contributor
Author
|
@tasanuma Please review, thank you. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
@tasanuma The failed UT don't seem to relate to this patch, please help to check. |
tasanuma
reviewed
Apr 7, 2022
Member
tasanuma
left a comment
tasanuma
left a comment
There was a problem hiding this comment.
@lfxy Thanks for creating the PR. I did some tests with this PR in my test cluster, and it worked well. I left some review comments about typo words.
And I have one more question. There are some clusterMap.getNumOfRacks() in BlockPlacementStatusDefault. Do we need to update them as well?
...t/java/org/apache/hadoop/hdfs/server/namenode/TestBlockPlacementPolicyRackFaultTolerant.java
Show resolved
Hide resolved
...t/java/org/apache/hadoop/hdfs/server/namenode/TestBlockPlacementPolicyRackFaultTolerant.java
Outdated
Show resolved
Hide resolved
...t/java/org/apache/hadoop/hdfs/server/namenode/TestBlockPlacementPolicyRackFaultTolerant.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@tasanuma Yes, I think clusterMap.getNumOfRacks() in BlockPlacementPolicyDefault should also be updated because only non empty rack makes sense. |
|
🎊 +1 overall
This message was automatically generated. |
tasanuma
approved these changes
Apr 8, 2022
Member
tasanuma
left a comment
tasanuma
left a comment
There was a problem hiding this comment.
Thanks for updating the PR. +1.
I will merge this PR next week if there are no other reviews.
Member
|
@surendralilhore Please comment if you have any concerns. |
|
💔 -1 overall
This message was automatically generated. |
Member
|
Merged. Thanks for your contribution, @lfxy! |
Contributor
Author
|
@tasanuma Thank you for your review and giving a lot of useful suggestions. |
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 20, 2022
…e rack number is equal with replication (apache#4126) (cherry picked from commit cee8c62) Conflicts: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java Change-Id: Id5f937c25d87ae48f3ccabecf8b0c5feac7ca496 (cherry picked from commit dd79aee635fdc61648e0c87bea1560dc35aee053)
jojochuang
added a commit
that referenced
this pull request
May 26, 2022
…e rack number is equal with replication (#4126) (#4304) (cherry picked from commit cee8c62) Conflicts: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java (cherry picked from commit dd79aee635fdc61648e0c87bea1560dc35aee053) Co-authored-by: caozhiqiang <lfxy@163.com> Reviewed-by: Takanobu Asanuma <tasanuma@apache.org>
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…e rack number is equal with replication (apache#4126)
LiuGuH
pushed a commit
to LiuGuH/hadoop
that referenced
this pull request
Mar 26, 2024
…e rack number is equal with replication (apache#4126)
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
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.
HDFS-16456
In below scenario, decommission will fail by TOO_MANY_NODES_ON_RACK reason:
The root cause is in BlockPlacementPolicyRackFaultTolerant::getMaxNodesPerRack() function, it will give a limit parameter maxNodesPerRack for choose targets. In this scenario, the maxNodesPerRack is 1, which means each rack can only be chosen one datanode.
int maxNodesPerRack = (totalNumOfReplicas - 1) / numOfRacks + 1;
here will be called, where totalNumOfReplicas=9 and numOfRacks=9
When we decommission one dn which is only one node in its rack, the chooseOnce() in BlockPlacementPolicyRackFaultTolerant::chooseTargetInOrder() will throw NotEnoughReplicasException, but the exception will not be caught and fail to fallback to chooseEvenlyFromRemainingRacks() function.
When decommission, after choose targets, verifyBlockPlacement() function will return the total rack number contains the invalid rack, and BlockPlacementStatusDefault::isPlacementPolicySatisfied() will return false and it will also cause decommission fail.
public boolean isPlacementPolicySatisfied() { return requiredRacks <= currentRacks || currentRacks >= totalRacks; }According to the above description, we should make the below modify to fix it: