HDFS-16540 Data locality is lost when DataNode pod restarts in kubern… #4170
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. |
tomscut
reviewed
Apr 15, 2022
...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
Outdated
Show resolved
Hide resolved
huaxiangsun
force-pushed
the
trunk.HDFS-16540
branch
from
1316ff0 to
43bd835
Compare
tomscut
reviewed
Apr 16, 2022
...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
Show resolved
Hide resolved
huaxiangsun
force-pushed
the
trunk.HDFS-16540
branch
from
43bd835 to
ccce180
Compare
ndimiduk
approved these changes
Apr 21, 2022
...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
Show resolved
Hide resolved
| NameNode.stateChangeLog.info("BLOCK* registerDatanode: " + nodeS | ||
| + " is replaced by " + nodeReg + " with the same storageID " | ||
| + nodeReg.getDatanodeUuid()); | ||
| + nodeReg.getDatanodeUuid() + ", updateHost2DatanodeMap: " + updateHost2DatanodeMap); |
There was a problem hiding this comment.
Is this extra information needed at the INFO level log? I understand that having the value printed is helpful during development, but I don't think it's meaningful to an operator.
Also, if you're here to change a log message, maybe also change it to use the format string version instead of string concatenation?
There was a problem hiding this comment.
Yeah, agree. Let me undo this change.
There was a problem hiding this comment.
I am going to upload the patch which does not log updateHost2DatanodeMap.
...op-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
huaxiangsun
force-pushed
the
trunk.HDFS-16540
branch
from
ccce180 to
f73d13a
Compare
|
💔 -1 overall
This message was automatically generated. |
|
I run the failed test [.TestReplaceDatanodeFailureReplication.testWithOnlyLastDatanodeIsAlive] locally multiple times, it passed. |
|
Any more comments? Thanks. |
|
I'll merge later today (unless someone else beats me to it). |
saintstack
added a commit
that referenced
this pull request
Apr 28, 2022
…n kubernetes (#4170)" Revert to add the '.' after HDFS-16540 so commit message format matches precedent This reverts commit bda0881.
saintstack
added a commit
that referenced
this pull request
Apr 28, 2022
…netes (#4170) This reverts the previous commit 4e47eb6 undone so I could reapply with the '.' after the HDFS-16540 as is done in all other commits.
saintstack
added a commit
to saintstack/hadoop
that referenced
this pull request
Apr 28, 2022
…netes (apache#4170) Cherry-pick of 9ed8d60
saintstack
added a commit
to saintstack/hadoop
that referenced
this pull request
May 15, 2022
…netes (apache#4170) Cherry-pick of 9ed8d60
|
Sorry for late response. Just found that this PR involved unrelated changes '.BUILDING.txt.swp' under root path of project. If no other concerns I would like to remove it for a while. |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…etes (apache#4170) When DN with the same UUID is registered with a different IP, host2DatanodeMap needs to be updated accordingly.
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…n kubernetes (apache#4170)" Revert to add the '.' after HDFS-16540 so commit message format matches precedent This reverts commit bda0881.
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…netes (apache#4170) This reverts the previous commit 4e47eb6 undone so I could reapply with the '.' after the HDFS-16540 as is done in all other commits.
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.
…etes
Description of PR
When Dn with the same uuid is registered with a different ip, host2DatanodeMap needs to be updated accordingly.
How was this patch tested?
Tested 3.3.2 with the patch on a eks cluster, restarted the pod hosting DataNode and HBase region server. After that, doing a major compaction of Hbase region, made sure that locality is kept.
There is also a new unittest case added.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?