HDFS-16648. Add isDebugEnabled check for debug logs in some classes #4529

ZanderXu · 2022-07-04T13:54:59Z

Refer to PR4480 to uniformly the debug logs in HDFS core classes.

And the detailed performance test, please refer to debugLogPerformanceTest.

hadoop-yetus · 2022-07-05T00:29:28Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	18m 30s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets 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 _
+0 🆗	mvndep	14m 47s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 7s		trunk passed
+1 💚	compile	24m 54s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	21m 27s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 32s		trunk passed
+1 💚	mvnsite	4m 0s		trunk passed
+1 💚	javadoc	3m 0s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 7s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	6m 54s		trunk passed
+1 💚	shadedclient	26m 42s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 26s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 30s		the patch passed
+1 💚	compile	24m 10s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 10s		the patch passed
+1 💚	compile	21m 35s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 35s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 23s		the patch passed
+1 💚	mvnsite	4m 3s		the patch passed
+1 💚	javadoc	2m 54s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 9s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 9s		the patch passed
+1 💚	shadedclient	26m 58s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 16s		hadoop-common in the patch passed.
+1 💚	unit	360m 12s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 35s		The patch does not generate ASF License warnings.
		633m 7s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/1/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 29ad789b159d 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 8218de367bad0d7616011465ae030a814b2c5901
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/1/testReport/
Max. process+thread count	2094 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

goiri · 2022-07-05T18:26:41Z

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

Are we saving anything by checking LOG.isDebugEnabled()? Using {} should avoid toString() and all.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

goiri · 2022-07-05T18:28:38Z

...ject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java

I don't think we need this check.
With the new style line limit this could fit in one line too.

ZanderXu · 2022-07-06T05:26:22Z

What are we saving by doing this?

In PR-4480, I do a performance stress test about it. And found that adding isDebugEnabled before strings concatenation is helpful for performance.
Please refer to the stress test code and results.
Test code:

    org.slf4j.Logger LOG = LoggerFactory.getLogger("Test");
    Block b = new Block();
    DatanodeInfo dn = new DatanodeInfo(EMPTY_DATANODE_ID);
    List<DatanodeInfo> staleNodes = new ArrayList<>();
    staleNodes.add(dn);
    long beginTime1 = Time.monotonicNowNanos();
    for (int i = 0; i < 100000000; i++) {
      LOG.debug("BLOCK* invalidateBlocks: postponing " +
              "invalidation of {} on {} because {} replica(s) are located on " +
              "nodes with potentially out-of-date block reports",
          b, dn, staleNodes);
    }
    long endTime1 = Time.monotonicNowNanos();
    LOG.info("Time1: {}", endTime1 - beginTime1);

    long beginTime2 = Time.monotonicNowNanos();
    for (int i = 0; i < 100000000; i++) {
      if (LOG.isDebugEnabled()) {
        LOG.debug("BLOCK* invalidateBlocks: postponing " +
                "invalidation of {} on {} because {} replica(s) are located on " +
                "nodes with potentially out-of-date block reports",
            b, dn, staleNodes);
      }
    }
    long endTime2 = Time.monotonicNowNanos();
    LOG.info("Time2: {}", endTime2 - beginTime2);

    long beginTime3 = Time.monotonicNowNanos();
    for (int i = 0; i < 100000000; i++) {
      LOG.debug("BLOCK* invalidateBlocks: postponing invalidation of {}", b);
    }
    long endTime3 = Time.monotonicNowNanos();
    LOG.info("Time3: {}", endTime3 - beginTime3);

And the result is:

-	Time1(ns)	Time2(ns)	Time3(ns)
First Run	353681417	287066916	270287253
Second Run	356797081	308013207	266873810
Third Run	366995010	287111940	259907404
Avg	359157836	294064021	265689489

goiri · 2022-07-06T18:56:23Z

Thanks @ZanderXu for the benchmark.
I always thought that the static + of string was taken care by the compiler.

goiri · 2022-07-06T18:58:28Z

...ct/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java

With the new checkstyle, the string fits in one line without splitting:

blockLog.debug("BLOCK* removeStoredBlock: {} has already been removed from node {}", storedBlock, node);

goiri · 2022-07-06T18:59:26Z

...src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java

The string fits in one line.

goiri · 2022-07-06T19:00:25Z

...src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockPlacementPolicyDefault.java

The string fits in one line if you do:

LOG.debug( "Failed to choose with favored nodes (={}), disregard favored nodes hint and retry.", favoredNodes, nr);

goiri · 2022-07-06T19:01:08Z

...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java

The string fits in one line.

hadoop-yetus · 2022-07-07T17:23:14Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 53s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets 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 _
+0 🆗	mvndep	14m 28s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 51s		trunk passed
+1 💚	compile	25m 37s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	21m 35s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 35s		trunk passed
+1 💚	mvnsite	4m 4s		trunk passed
+1 💚	javadoc	3m 4s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 7s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	6m 57s		trunk passed
+1 💚	shadedclient	26m 55s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 25s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 27s		the patch passed
+1 💚	compile	24m 14s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 14s		the patch passed
+1 💚	compile	21m 42s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 42s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 24s		root: The patch generated 0 new + 515 unchanged - 1 fixed = 515 total (was 516)
+1 💚	mvnsite	4m 0s		the patch passed
+1 💚	javadoc	2m 55s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 10s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 11s		the patch passed
+1 💚	shadedclient	26m 58s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 24s		hadoop-common in the patch passed.
+1 💚	unit	367m 47s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 34s		The patch does not generate ASF License warnings.
		625m 2s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/2/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 860faa4db824 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 52f6c737a92067e6fda82b77bff964d0ca518197
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/2/testReport/
Max. process+thread count	3137 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

goiri · 2022-07-08T04:43:16Z

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java

This particular one should stay within the isDebugEnabled() to avoid the mapToString

goiri · 2022-07-08T04:43:50Z

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java

This probably should stay.

goiri · 2022-07-08T04:44:18Z

...ct/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java

Don't we want to keep the check?

There is already a judgment, so we can remove the internal redundant judgment.

if (blockLog.isDebugEnabled()) { for (Block b : invalidatedBlocks) { blockLog.debug("BLOCK* processReport 0x{} with lease ID 0x{}: {} on node {} size {} " + "does not belong to any file.", strBlockReportId, fullBrLeaseId, b, node, b.getNumBytes()); } }

ZanderXu · 2022-07-08T06:13:19Z

@goiri thanks for your review, I learned a lot and have updated the patch. Please help me review it again.

hadoop-yetus · 2022-07-08T16:42:07Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 51s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets 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 _
+0 🆗	mvndep	14m 22s		Maven dependency ordering for branch
+1 💚	mvninstall	29m 3s		trunk passed
+1 💚	compile	25m 22s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	23m 53s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	5m 5s		trunk passed
+1 💚	mvnsite	4m 10s		trunk passed
+1 💚	javadoc	3m 5s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 3s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 16s		trunk passed
+1 💚	shadedclient	27m 52s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 27s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 32s		the patch passed
+1 💚	compile	25m 4s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	25m 4s		the patch passed
+1 💚	compile	21m 39s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 39s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	4m 27s	/results-checkstyle-root.txt	root: The patch generated 1 new + 515 unchanged - 1 fixed = 516 total (was 516)
+1 💚	mvnsite	3m 59s		the patch passed
+1 💚	javadoc	2m 56s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 9s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 13s		the patch passed
+1 💚	shadedclient	27m 2s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 23s		hadoop-common in the patch passed.
+1 💚	unit	366m 14s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 34s		The patch does not generate ASF License warnings.
		628m 11s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/3/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 26ea390b3adc 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 87d3bd16d6e4ffcce7b38b0112843d59cecb35e9
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/3/testReport/
Max. process+thread count	2880 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java

hadoop-yetus · 2022-07-10T12:41:58Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 4s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets 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 _
+0 🆗	mvndep	15m 18s		Maven dependency ordering for branch
+1 💚	mvninstall	32m 58s		trunk passed
+1 💚	compile	30m 12s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	25m 16s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 47s		trunk passed
+1 💚	mvnsite	4m 1s		trunk passed
+1 💚	javadoc	2m 57s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 58s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 5s		trunk passed
+1 💚	shadedclient	28m 12s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 28s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 44s		the patch passed
+1 💚	compile	29m 9s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	29m 9s		the patch passed
+1 💚	compile	23m 35s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 35s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 27s		root: The patch generated 0 new + 515 unchanged - 1 fixed = 515 total (was 516)
+1 💚	mvnsite	3m 56s		the patch passed
+1 💚	javadoc	2m 58s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 5s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 44s		the patch passed
+1 💚	shadedclient	27m 35s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 26s		hadoop-common in the patch passed.
+1 💚	unit	372m 4s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 36s		The patch does not generate ASF License warnings.
		651m 17s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/4/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux caf6b4701573 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 2dc51a698015fd4dfd872f9f87ebb498f4185350
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/4/testReport/
Max. process+thread count	2088 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/4/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ZanderXu · 2022-07-13T13:12:17Z

I have one question to ask, that is why some code should remove LOG.isDebugEnabled()?

@slfan1989 Thanks for your review. If you have any other questions or good ideas, please let me know. Thanks

ZanderXu · 2022-07-21T02:40:53Z

@slfan1989 ping. Can you push this PR forward?

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java

slfan1989 · 2022-07-21T07:00:15Z

@slfan1989 ping. Can you push this PR forward?

This pr looks good, let's wait for the opinions of other partners. thanks for your contribution!

hadoop-yetus · 2022-07-21T10:09:18Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 0s		Docker mode activated.
-1 ❌	patch	0m 19s		#4529 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.

Subsystem	Report/Notes
GITHUB PR	#4529
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/5/console
versions	git=2.17.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ferhui

can also remove isDebugEnabled from those. 2 lines

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java

slfan1989 · 2022-07-21T23:59:52Z

@ZanderXu After completing this pr, can you help summarize the precautions in the process of using logger, so that other partners can refer to it later?

ferhui · 2022-07-22T06:19:51Z

can remove isDebugEnabled from those clauses without '+'

ZanderXu · 2022-07-22T06:28:36Z

can remove isDebugEnabled from those clauses without '+'

Do you mean correct all classes in hadoop-hdfs module?

In this patch, I just modified the classes that are used frequently by the namenode.

ZanderXu · 2022-07-22T06:29:32Z

After completing this pr, can you help summarize the precautions in the process of using logger, so that other partners can refer to it later?

@slfan1989 Sure, I will summarize it.

ferhui · 2022-07-22T14:45:44Z

can remove isDebugEnabled from those clauses without '+'

Do you mean correct all classes in hadoop-hdfs module?

In this patch, I just modified the classes that are used frequently by the namenode.

can correct them in the classes you modified.

ZanderXu · 2022-07-22T14:46:51Z

Copy, I will correct them. Thanks~

ZanderXu · 2022-07-23T15:39:44Z

can correct them in the classes you modified.

@ferhui I have corrected them, please help me review it again. Thanks~

hadoop-yetus · 2022-07-23T15:41:47Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 0s		Docker mode activated.
-1 ❌	patch	0m 24s		#4529 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.

Subsystem	Report/Notes
GITHUB PR	#4529
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/6/console
versions	git=2.17.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ferhui · 2022-08-03T01:31:17Z

@ZanderXu could you rebase the patch?

ZanderXu · 2022-08-03T05:05:17Z

@ferhui Thanks for your review. I have updated this patch based on the latest trunk. Please help me review it again, thanks.

hadoop-yetus · 2022-08-03T15:49:06Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 52s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets 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 _
+0 🆗	mvndep	14m 44s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 21s		trunk passed
+1 💚	compile	25m 26s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	22m 3s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 34s		trunk passed
+1 💚	mvnsite	4m 2s		trunk passed
+1 💚	javadoc	3m 5s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 11s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 4s		trunk passed
+1 💚	shadedclient	27m 16s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 26s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 33s		the patch passed
+1 💚	compile	26m 45s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	26m 45s		the patch passed
+1 💚	compile	24m 21s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	24m 21s		the patch passed
+1 💚	blanks	0m 1s		The patch has no blanks issues.
+1 💚	checkstyle	4m 33s		root: The patch generated 0 new + 511 unchanged - 3 fixed = 511 total (was 514)
+1 💚	mvnsite	4m 1s		the patch passed
+1 💚	javadoc	2m 44s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 19s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 51s		the patch passed
+1 💚	shadedclient	27m 29s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 25s		hadoop-common in the patch passed.
+1 💚	unit	377m 36s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 39s		The patch does not generate ASF License warnings.
		643m 28s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/7/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux a9b43d8c429f 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 1ee3d9b91ca11d13e58513ea2a2077e79f31a416
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/7/testReport/
Max. process+thread count	3137 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/7/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ferhui · 2022-08-04T10:16:40Z

@ZanderXu CI reported that 'This branch has conflicts that must be resolved'

ZanderXu · 2022-08-04T11:12:35Z

@ferhui Sir, thanks for your review.

CI reported that 'This branch has conflicts that must be resolved'

I have fixed this conflict. This conflict is in EditLogTailer.java which has modified by HDFS-16642.

hadoop-yetus · 2022-08-04T22:47:17Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 0s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets 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 _
+0 🆗	mvndep	14m 57s		Maven dependency ordering for branch
+1 💚	mvninstall	29m 55s		trunk passed
+1 💚	compile	27m 2s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	26m 24s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 56s		trunk passed
+1 💚	mvnsite	4m 33s		trunk passed
+1 💚	javadoc	3m 51s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 15s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 28s		trunk passed
+1 💚	shadedclient	28m 4s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 31s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 46s		the patch passed
+1 💚	compile	26m 42s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	26m 42s		the patch passed
+1 💚	compile	23m 49s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 49s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 37s		root: The patch generated 0 new + 511 unchanged - 3 fixed = 511 total (was 514)
+1 💚	mvnsite	4m 14s		the patch passed
+1 💚	javadoc	3m 29s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 45s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	8m 10s		the patch passed
+1 💚	shadedclient	31m 28s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	19m 41s		hadoop-common in the patch passed.
-1 ❌	unit	413m 38s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 41s		The patch does not generate ASF License warnings.
		696m 24s

Reason	Tests
Failed junit tests	hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/8/artifact/out/Dockerfile
GITHUB PR	#4529
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux af653068323d 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `abad1fc`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /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-4529/8/testReport/
Max. process+thread count	1836 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4529/8/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ZanderXu · 2022-08-05T01:21:11Z

The failed UT org.apache.hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes.testBalancerWithObserverWithFailedNode works well locally.

ferhui · 2022-08-06T13:36:05Z

@ZanderXu Thanks for your contribution! @goiri @slfan1989 Thanks for your review! Merged

…ses (apache#4529)

ZanderXu changed the title ~~HDFS-16648. Add isDebugEnabled check for debug blockLogs in some classes~~ HDFS-16648. Add isDebugEnabled check for debug logs in some classes Jul 4, 2022

goiri reviewed Jul 5, 2022

View reviewed changes

goiri reviewed Jul 6, 2022

View reviewed changes

goiri reviewed Jul 8, 2022

View reviewed changes

goiri approved these changes Jul 8, 2022

View reviewed changes

slfan1989 reviewed Jul 10, 2022

View reviewed changes

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java Outdated Show resolved Hide resolved

ferhui reviewed Jul 21, 2022

View reviewed changes

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java Outdated Show resolved Hide resolved

ferhui reviewed Jul 21, 2022

View reviewed changes

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java Outdated Show resolved Hide resolved

...t/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java Outdated Show resolved Hide resolved

ZanderXu force-pushed the HDFS-16648 branch from f8750bc to 1ee3d9b Compare August 3, 2022 05:04

ferhui approved these changes Aug 4, 2022

View reviewed changes

HDFS-16648. Add isDebugEnabled check for debug blockLogs in some classes

abad1fc

ZanderXu force-pushed the HDFS-16648 branch from 1ee3d9b to abad1fc Compare August 4, 2022 11:09

ferhui merged commit 25ccdc7 into apache:trunk Aug 6, 2022

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

HDFS-16648. Add isDebugEnabled check for debug blockLogs in some clas…

0e6e407

…ses (apache#4529)

haiyang1987 mentioned this pull request Sep 28, 2023

HDFS-17194. Enhance the log message for striped block recovery #6094

Merged

HDFS-16648. Add isDebugEnabled check for debug logs in some classes #4529

HDFS-16648. Add isDebugEnabled check for debug logs in some classes #4529

Uh oh!

Conversation

ZanderXu commented Jul 4, 2022

Uh oh!

hadoop-yetus commented Jul 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZanderXu commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goiri commented Jul 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hadoop-yetus commented Jul 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZanderXu commented Jul 8, 2022

Uh oh!

hadoop-yetus commented Jul 8, 2022

Uh oh!

Uh oh!

hadoop-yetus commented Jul 10, 2022

Uh oh!

ZanderXu commented Jul 13, 2022

Uh oh!

ZanderXu commented Jul 21, 2022

Uh oh!

Uh oh!

slfan1989 commented Jul 21, 2022

Uh oh!

hadoop-yetus commented Jul 21, 2022

Uh oh!

ferhui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

slfan1989 commented Jul 21, 2022

Uh oh!

ferhui commented Jul 22, 2022

Uh oh!

ZanderXu commented Jul 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZanderXu commented Jul 22, 2022

Uh oh!

ferhui commented Jul 22, 2022

Uh oh!

ZanderXu commented Jul 22, 2022

Uh oh!

ZanderXu commented Jul 23, 2022

Uh oh!

hadoop-yetus commented Jul 23, 2022

Uh oh!

ferhui commented Aug 3, 2022

Uh oh!

ZanderXu commented Aug 3, 2022

Uh oh!

hadoop-yetus commented Aug 3, 2022

Uh oh!

ZanderXu commented Jul 6, 2022 •

edited

Loading

ZanderXu commented Jul 22, 2022 •

edited

Loading