HDFS-16378. Add datanode address to BlockReportLeaseManager logs #3786

tomscut · 2021-12-11T02:02:21Z

We should add datanode address to BlockReportLeaseManager logs. Because the datanodeuuid is not convenient for tracking.

Hexiaoqiao · 2021-12-11T04:44:10Z

Thanks @tomscut for your report. It is almost LGTM. Some nits,
a. There are also some other logs which only leaves DataNodeUUID, do you mind to complete them with address together?
b. Just suggest to invoke dn.getXferAddr() to keep the log style same. e.g.
200 LOG.info("Registered DN {} ({}).", dn.getDatanodeUuid(), dn.getXferAddr());

tomscut · 2021-12-11T06:18:12Z

Thanks @tomscut for your report. It is almost LGTM. Some nits, a. There are also some other logs which only leaves DataNodeUUID, do you mind to complete them with address together? b. Just suggest to invoke dn.getXferAddr() to keep the log style same. e.g. 200 LOG.info("Registered DN {} ({}).", dn.getDatanodeUuid(), dn.getXferAddr());

Thank you very much for your review and suggestion. Can we change getXferAddr to getXferAddrWithHostname, because hostname may be more intuitive than IP. What do you think? I will correct the omissions together.

Hexiaoqiao · 2021-12-11T06:45:15Z

I am prefer to keep getXferAddr here, IMO IP and Hostname is equal for maintainer. But when meet binding Multiple NICs env and multi DataNode instances deployed on one node, it is easy to differ them if use IP information. Another, getXferAddr saves to calculate sum of string than getXferAddrWithHostname (not key point).

hadoop-yetus · 2021-12-11T07:28:07Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 45s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 31s		trunk passed
+1 💚	compile	1m 25s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	1m 20s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	checkstyle	1m 1s		trunk passed
+1 💚	mvnsite	1m 27s		trunk passed
+1 💚	javadoc	1m 4s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 32s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 12s		trunk passed
+1 💚	shadedclient	22m 25s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 17s		the patch passed
+1 💚	compile	1m 17s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	1m 17s		the patch passed
+1 💚	compile	1m 8s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	javac	1m 8s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 50s		the patch passed
+1 💚	mvnsite	1m 17s		the patch passed
+1 💚	javadoc	0m 52s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 23s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 12s		the patch passed
+1 💚	shadedclient	22m 8s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	225m 28s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 45s		The patch does not generate ASF License warnings.
		324m 24s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3786/1/artifact/out/Dockerfile
GITHUB PR	#3786
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 38d895cb22a1 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `a2ab15b`
Default Java	Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3786/1/testReport/
Max. process+thread count	2993 (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-3786/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.

tomscut · 2021-12-11T12:34:01Z

I am prefer to keep getXferAddr here, IMO IP and Hostname is equal for maintainer. But when meet binding Multiple NICs env and multi DataNode instances deployed on one node, it is easy to differ them if use IP information. Another, getXferAddr saves to calculate sum of string than getXferAddrWithHostname (not key point).

Thank you for confirming and explaining this. I will fix it.

tomscut · 2021-12-11T12:50:02Z

We can not get the datanode address from NodeData in line 273:
LOG.info("Removing expired block report lease 0x{} for DN {}.", Long.toHexString(node.leaseId), node.datanodeUuid);

hadoop-yetus · 2021-12-11T18:15:29Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 45s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 31s		trunk passed
+1 💚	compile	1m 26s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	1m 15s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	checkstyle	1m 2s		trunk passed
+1 💚	mvnsite	1m 28s		trunk passed
+1 💚	javadoc	1m 4s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 31s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 15s		trunk passed
+1 💚	shadedclient	22m 29s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 16s		the patch passed
+1 💚	compile	1m 18s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	1m 18s		the patch passed
+1 💚	compile	1m 13s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	javac	1m 13s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 51s		the patch passed
+1 💚	mvnsite	1m 19s		the patch passed
+1 💚	javadoc	0m 52s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 24s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 13s		the patch passed
+1 💚	shadedclient	22m 7s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	227m 27s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 46s		The patch does not generate ASF License warnings.
		326m 15s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3786/2/artifact/out/Dockerfile
GITHUB PR	#3786
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 356cdab9c4bd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `7204e06`
Default Java	Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3786/2/testReport/
Max. process+thread count	3294 (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-3786/2/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.

tomscut · 2021-12-13T08:52:48Z

I am prefer to keep getXferAddr here, IMO IP and Hostname is equal for maintainer. But when meet binding Multiple NICs env and multi DataNode instances deployed on one node, it is easy to differ them if use IP information. Another, getXferAddr saves to calculate sum of string than getXferAddrWithHostname (not key point).

Hi @Hexiaoqiao , I fixed this. PTAL. Thanks a lot.

Hexiaoqiao

LGTM. +1. Will commit if no more other comments while 2 days later.

tomscut · 2021-12-13T11:40:07Z

LGTM. +1. Will commit if no more other comments while 2 days later.

Thanks for your reply.

tomscut · 2021-12-15T03:22:42Z

Thanks @Hexiaoqiao for the review and the merge.

…che#3786). Contributed by tomscut. Signed-off-by: He Xiaoqiao <[email protected]>

HDFS-16378. Add datanode address to BlockReportLeaseManager logs

a2ab15b

replace getXferAddrWithHostname with getXferAddr

7204e06

Hexiaoqiao approved these changes Dec 13, 2021

View reviewed changes

Hexiaoqiao merged commit 59c6508 into apache:trunk Dec 15, 2021

hotcodemacha pushed a commit to hotcodemacha/hadoop that referenced this pull request Dec 22, 2021

HDFS-16378. Add datanode address to BlockReportLeaseManager logs (apa…

84da38b

…che#3786). Contributed by tomscut. Signed-off-by: He Xiaoqiao <[email protected]>

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HDFS-16378. Add datanode address to BlockReportLeaseManager logs (apa…

dcac3d2

…che#3786). Contributed by tomscut. Signed-off-by: He Xiaoqiao <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDFS-16378. Add datanode address to BlockReportLeaseManager logs #3786

HDFS-16378. Add datanode address to BlockReportLeaseManager logs #3786

Uh oh!

tomscut commented Dec 11, 2021

Uh oh!

Hexiaoqiao commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021

Uh oh!

Hexiaoqiao commented Dec 11, 2021

Uh oh!

hadoop-yetus commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021 •

edited

Loading

Uh oh!

hadoop-yetus commented Dec 11, 2021

Uh oh!

tomscut commented Dec 13, 2021

Uh oh!

Hexiaoqiao left a comment

Uh oh!

tomscut commented Dec 13, 2021

Uh oh!

tomscut commented Dec 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HDFS-16378. Add datanode address to BlockReportLeaseManager logs #3786

HDFS-16378. Add datanode address to BlockReportLeaseManager logs #3786

Uh oh!

Conversation

tomscut commented Dec 11, 2021

Uh oh!

Hexiaoqiao commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021

Uh oh!

Hexiaoqiao commented Dec 11, 2021

Uh oh!

hadoop-yetus commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021

Uh oh!

tomscut commented Dec 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hadoop-yetus commented Dec 11, 2021

Uh oh!

tomscut commented Dec 13, 2021

Uh oh!

Hexiaoqiao left a comment

Choose a reason for hiding this comment

Uh oh!

tomscut commented Dec 13, 2021

Uh oh!

tomscut commented Dec 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomscut commented Dec 11, 2021 •

edited

Loading