You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/04/13 19:51:23 UTC
[GitHub] [hadoop] huaxiangsun opened a new pull request, #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
huaxiangsun opened a new pull request, #4170:
URL: https://github.com/apache/hadoop/pull/4170
…etes
<!--
Thanks for sending a pull request!
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
2. Make sure your PR title starts with JIRA issue id, e.g., 'HADOOP-17799. Your PR title ...'.
-->
### 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:
- [ ] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r851432390
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getAddress()) ||
Review Comment:
Good catch! Let me update it to getIpAddr(), will update shortly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r852277677
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getIpAddr()) ||
+ (nodeS.getXferPort() != nodeReg.getXferPort());
+
Review Comment:
Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r855427229
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -138,6 +138,35 @@ public void testNumVersionsCorrectAfterReregister()
mapToCheck.get("version1").intValue(), 1);
}
+ /**
+ * This test checks that if a node is re-registered with a different ip, its
+ * host2DatanodeMap is correctly updated with the new ip.
+ */
+ @Test
Review Comment:
When I checked the code, I think there is another one Network map, its code is what I followed for this hostToDatanodeMap change. Let me check, would rather avoid adding more testing cases for code which is already there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r856493693
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
+
NameNode.stateChangeLog.info("BLOCK* registerDatanode: " + nodeS
+ " is replaced by " + nodeReg + " with the same storageID "
- + nodeReg.getDatanodeUuid());
+ + nodeReg.getDatanodeUuid() + ", updateHost2DatanodeMap: " + updateHost2DatanodeMap);
Review Comment:
I am going to upload the patch which does not log updateHost2DatanodeMap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] saintstack merged pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
saintstack merged PR #4170:
URL: https://github.com/apache/hadoop/pull/4170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1105400155
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 58s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 45s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 6s | | trunk passed |
| +1 :green_heart: | compile | 24m 43s | | trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 20m 54s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 3m 55s | | trunk passed |
| +1 :green_heart: | mvnsite | 25m 47s | | trunk passed |
| -1 :x: | javadoc | 1m 30s | [/branch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/artifact/out/branch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) | root in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04. |
| +1 :green_heart: | javadoc | 8m 21s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 39m 0s | | trunk passed |
| +1 :green_heart: | shadedclient | 57m 59s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 25m 40s | | the patch passed |
| +1 :green_heart: | compile | 24m 26s | | the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 24m 26s | | the patch passed |
| +1 :green_heart: | compile | 21m 29s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 29s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 20m 28s | | the patch passed |
| -1 :x: | javadoc | 1m 30s | [/patch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/artifact/out/patch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) | root in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04. |
| +1 :green_heart: | javadoc | 8m 27s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 39m 20s | | the patch passed |
| +1 :green_heart: | shadedclient | 58m 8s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 1064m 10s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| +1 :green_heart: | asflicense | 2m 12s | | The patch does not generate ASF License warnings. |
| | | 1431m 51s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.yarn.server.resourcemanager.TestRMEmbeddedElector |
| | hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4170 |
| Optional Tests | dupname asflicense codespell compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux c73e326f6bae 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / ccce180d3ac535495ab8937cf6165542c3a89b46 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/testReport/ |
| Max. process+thread count | 2214 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r855422776
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
Review Comment:
Thanks Nick for the comment. I think nodeS.getXferAddr() cannot be null, let me investigate more and get back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1107606815
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 56s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 1s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 2s | | trunk passed |
| +1 :green_heart: | compile | 24m 51s | | trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 21m 32s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 29s | | trunk passed |
| +1 :green_heart: | mvnsite | 19m 53s | | trunk passed |
| -1 :x: | javadoc | 1m 37s | [/branch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/artifact/out/branch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) | root in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04. |
| +1 :green_heart: | javadoc | 8m 28s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 38m 52s | | trunk passed |
| +1 :green_heart: | shadedclient | 57m 54s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 27s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 26m 0s | | the patch passed |
| +1 :green_heart: | compile | 24m 27s | | the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 24m 27s | | the patch passed |
| +1 :green_heart: | compile | 21m 38s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 38s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 20s | | the patch passed |
| +1 :green_heart: | mvnsite | 19m 30s | | the patch passed |
| -1 :x: | javadoc | 1m 26s | [/patch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/artifact/out/patch-javadoc-root-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt) | root in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04. |
| +1 :green_heart: | javadoc | 8m 58s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 39m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 57m 59s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 1054m 18s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| +1 :green_heart: | asflicense | 2m 16s | | The patch does not generate ASF License warnings. |
| | | 1417m 34s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.TestReplaceDatanodeFailureReplication |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4170 |
| Optional Tests | dupname asflicense codespell compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle |
| uname | Linux 95b69934c683 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f73d13a7c16369edfdca2aa7bbebf205b6e7aedb |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/testReport/ |
| Max. process+thread count | 2519 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
tomscut commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r850967827
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getAddress()) ||
Review Comment:
`nodeReg.getAddress()` contains port, but `nodeS.getIpAddr()` doesn't, so `updateHost2DatanodeMap` is always `true`, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
tomscut commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r851582741
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getIpAddr()) ||
+ (nodeS.getXferPort() != nodeReg.getXferPort());
+
Review Comment:
Can we update
`updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getIpAddr()) ||
(nodeS.getXferPort() != nodeReg.getXferPort());`
to
`updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ndimiduk commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r855124598
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
+
NameNode.stateChangeLog.info("BLOCK* registerDatanode: " + nodeS
+ " is replaced by " + nodeReg + " with the same storageID "
- + nodeReg.getDatanodeUuid());
+ + nodeReg.getDatanodeUuid() + ", updateHost2DatanodeMap: " + updateHost2DatanodeMap);
Review 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?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
Review Comment:
Can `nodeS.getXferAddr()` ever be null? Use of `Objects.equals()` would be safer.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -138,6 +138,35 @@ public void testNumVersionsCorrectAfterReregister()
mapToCheck.get("version1").intValue(), 1);
}
+ /**
+ * This test checks that if a node is re-registered with a different ip, its
+ * host2DatanodeMap is correctly updated with the new ip.
+ */
+ @Test
Review Comment:
It looks like there's a couple caches like this one. Is there equivalent test coverage for all the registered DN state that is updated on registration? Can you add coverage for any of the other states that are missing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1109046711
I run the failed test [.TestReplaceDatanodeFailureReplication.testWithOnlyLastDatanodeIsAlive] locally multiple times, it passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] Hexiaoqiao commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1129990002
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1098680455
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 17m 3s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 35s | | trunk passed |
| +1 :green_heart: | compile | 1m 31s | | trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 1m 20s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 5s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 31s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 7s | | trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 31s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 3m 40s | | trunk passed |
| +1 :green_heart: | shadedclient | 26m 14s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 18s | | the patch passed |
| +1 :green_heart: | compile | 1m 25s | | the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 1m 25s | | the patch passed |
| +1 :green_heart: | compile | 1m 15s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 1m 15s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 53s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 20s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 54s | | the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 26s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 3m 32s | | the patch passed |
| +1 :green_heart: | shadedclient | 26m 6s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 341m 40s | | hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 41s | | The patch does not generate ASF License warnings. |
| | | 474m 5s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4170 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 98cf76256475 4.15.0-162-generic #170-Ubuntu SMP Mon Oct 18 11:38:05 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1316ff0eada1e29dec8ca56ab266c9bcbe60051c |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/1/testReport/ |
| Max. process+thread count | 2175 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4170/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r855423782
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
+
NameNode.stateChangeLog.info("BLOCK* registerDatanode: " + nodeS
+ " is replaced by " + nodeReg + " with the same storageID "
- + nodeReg.getDatanodeUuid());
+ + nodeReg.getDatanodeUuid() + ", updateHost2DatanodeMap: " + updateHost2DatanodeMap);
Review Comment:
Yeah, agree. Let me undo this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r856498429
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -138,6 +138,35 @@ public void testNumVersionsCorrectAfterReregister()
mapToCheck.get("version1").intValue(), 1);
}
+ /**
+ * This test checks that if a node is re-registered with a different ip, its
+ * host2DatanodeMap is correctly updated with the new ip.
+ */
+ @Test
Review Comment:
Yeah, the NetworkTopology map is another map. It is quite complicated to add a unittest for it. I am going to leave to hdfs experts to decide if a unittest is needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1110051006
Any more comments? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r851552322
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getAddress()) ||
Review Comment:
Updated and verified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r852249004
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,26 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getIpAddr().equals(nodeReg.getIpAddr()) ||
+ (nodeS.getXferPort() != nodeReg.getXferPort());
+
Review Comment:
Makes sense as getXferAddr() is "getIpAddr():getXferPort()".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] huaxiangsun commented on a diff in pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on code in PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#discussion_r856493317
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1189,16 +1190,25 @@ public void registerDatanode(DatanodeRegistration nodeReg)
nodes with its data cleared (or user can just remove the StorageID
value in "VERSION" file under the data directory of the datanode,
but this is might not work if VERSION file format has changed
- */
+ */
+ // Check if nodeS's host information is same as nodeReg's, if not,
+ // it needs to update host2DatanodeMap accordringly.
+ updateHost2DatanodeMap = !nodeS.getXferAddr().equals(nodeReg.getXferAddr());
Review Comment:
Yeah, since nodeS is not null, nodeS.getXferAddr() cannot be null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] saintstack commented on pull request #4170: HDFS-16540 Data locality is lost when DataNode pod restarts in kubern…
Posted by GitBox <gi...@apache.org>.
saintstack commented on PR #4170:
URL: https://github.com/apache/hadoop/pull/4170#issuecomment-1111106375
I'll merge later today (unless someone else beats me to it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org