HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN #3317

wzhallright · 2021-08-20T10:51:53Z

Description of PR

I found the JN turn on edit cache, but the metric of rpcRequestCacheMissAmount can not display.

JIRA: https://issues.apache.org/jira/browse/HDFS-16181

…ay when tailEditLog form JN

hadoop-yetus · 2021-08-20T16:08:03Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 48s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		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	31m 3s		trunk passed
+1 💚	compile	1m 23s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	1m 18s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	checkstyle	1m 0s		trunk passed
+1 💚	mvnsite	1m 25s		trunk passed
+1 💚	javadoc	0m 56s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 26s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 7s		trunk passed
+1 💚	shadedclient	16m 14s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 15s		the patch passed
+1 💚	compile	1m 14s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	1m 14s		the patch passed
+1 💚	compile	1m 10s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	javac	1m 10s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 50s	/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 11 unchanged - 1 fixed = 12 total (was 12)
+1 💚	mvnsite	1m 13s		the patch passed
+1 💚	javadoc	0m 48s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 21s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 10s		the patch passed
+1 💚	shadedclient	16m 3s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	230m 4s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 45s		The patch does not generate ASF License warnings.
		314m 37s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3317/1/artifact/out/Dockerfile
GITHUB PR	#3317
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 89ce4a142518 4.15.0-151-generic #157-Ubuntu SMP Fri Jul 9 23:07:57 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `8a5092e`
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-3317/1/testReport/
Max. process+thread count	3879 (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-3317/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.

wzhallright · 2021-08-27T08:19:58Z

@xkrogen could you help review the PR?

xkrogen

Thanks for filing this PR! Sorry for the delay in my review, I was on vacation for some time and have been trying to catch up on a backlog since then.

The changes LGTM except one minor comment.

...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java

hadoop-yetus · 2021-09-14T11:36:20Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 38s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		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	30m 49s		trunk passed
+1 💚	compile	1m 23s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	1m 16s		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 23s		trunk passed
+1 💚	javadoc	0m 58s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 30s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 6s		trunk passed
+1 💚	shadedclient	16m 23s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 10s		the patch passed
+1 💚	compile	1m 15s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	1m 15s		the patch passed
+1 💚	compile	1m 9s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	javac	1m 9s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 51s	/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 10 unchanged - 1 fixed = 11 total (was 11)
+1 💚	mvnsite	1m 15s		the patch passed
+1 💚	javadoc	0m 48s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 20s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	3m 10s		the patch passed
+1 💚	shadedclient	16m 1s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	232m 12s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 46s		The patch does not generate ASF License warnings.
		316m 28s

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

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3317/2/artifact/out/Dockerfile
GITHUB PR	#3317
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux b8880ba9ae5a 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 / `17ff74e`
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-3317/2/testReport/
Max. process+thread count	3154 (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-3317/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.

xkrogen · 2021-09-14T16:51:41Z

Can you address the checkstyle issue?

./hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java:54:  MutableStat rpcRequestCacheMissAmount;:15: Variable 'rpcRequestCacheMissAmount' must be private and have accessor methods. [VisibilityModifier]

Previously with the @Metric annotation it had to be package-private to be accessible to metrics2, but now it's no longer necessary and we can resolve this by making it private.

LGTM other than that, the only unit test failing is TestBalancer which I have seen be flaky in the past and doesn't look related.

wzhallright · 2021-09-15T02:51:38Z

Can you address the checkstyle issue?
./hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java:54:  MutableStat rpcRequestCacheMissAmount;:15: Variable 'rpcRequestCacheMissAmount' must be private and have accessor methods. [VisibilityModifier]
Previously with the @Metric annotation it had to be package-private to be accessible to metrics2, but now it's no longer necessary and we can resolve this by making it private.

LGTM other than that, the only unit test failing is TestBalancer which I have seen be flaky in the past and doesn't look related.

OK, I have fixed the checkstyle, PTAL,Thanks. @xkrogen

hadoop-yetus · 2021-09-15T08:48:52Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 59s		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	42m 6s		trunk passed
+1 💚	compile	1m 42s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	1m 24s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	checkstyle	1m 13s		trunk passed
+1 💚	mvnsite	1m 39s		trunk passed
+1 💚	javadoc	1m 9s		trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	1m 34s		trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	spotbugs	4m 1s		trunk passed
+1 💚	shadedclient	35m 11s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 18s		the patch passed
+1 💚	compile	1m 19s		the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	1m 19s		the patch passed
+1 💚	compile	1m 15s		the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚	javac	1m 15s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 51s		hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 14 unchanged - 1 fixed = 14 total (was 15)
+1 💚	mvnsite	1m 16s		the patch passed
+1 💚	javadoc	0m 46s		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 19s		the patch passed
+1 💚	shadedclient	31m 35s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	246m 12s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 47s		The patch does not generate ASF License warnings.
		378m 14s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3317/3/artifact/out/Dockerfile
GITHUB PR	#3317
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 6e0bc9734654 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `7767807`
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-3317/3/testReport/
Max. process+thread count	3545 (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-3317/3/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.

xkrogen

LGTM

…cheMissAmount (#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com> (cherry picked from commit 232fd7cae170de8c6b52c14841a47dca8735c6d2)

…cheMissAmount (#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com> (cherry picked from commit 232fd7cae170de8c6b52c14841a47dca8735c6d2) (cherry picked from commit 2f73ac1)

…cheMissAmount (#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com> (cherry picked from commit 232fd7cae170de8c6b52c14841a47dca8735c6d2) (cherry picked from commit 2f73ac1) (cherry picked from commit 928dbac)

…cheMissAmount (#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com> (cherry picked from commit 232fd7cae170de8c6b52c14841a47dca8735c6d2) (cherry picked from commit 2f73ac1) (cherry picked from commit 928dbac) (cherry picked from commit 9b63f60)

xkrogen · 2021-09-15T17:11:54Z

Merged to trunk and backported to branch-3.3, branch-3.2, branch-3.1, branch-2.10.

Thanks @wzhallright !

…cheMissAmount (apache#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com>

HDFS-16181. [SBN] Fix metric of RpcRequestCacheMissAmount can't displ…

8a5092e

…ay when tailEditLog form JN

wzhallright changed the title ~~HDFS-16181. [SBN] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN~~ HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN Aug 23, 2021

xkrogen requested changes Sep 13, 2021

View reviewed changes

...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalMetrics.java Outdated Show resolved Hide resolved

HDFS-16181. address comment

17ff74e

HDFS-16181. fix checkstyle

7767807

xkrogen self-requested a review September 15, 2021 16:52

xkrogen approved these changes Sep 15, 2021

View reviewed changes

xkrogen merged commit 3ecaa39 into apache:trunk Sep 15, 2021

kiran-maturi pushed a commit to kiran-maturi/hadoop that referenced this pull request Nov 24, 2021

HDFS-16181. [SBN Read] Fix display of JournalNode metric RpcRequestCa…

f03e77c

…cheMissAmount (apache#3317) Co-authored-by: wangzhaohui8 <wangzhaohui8@jd.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN #3317

HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN #3317

Uh oh!

wzhallright commented Aug 20, 2021

Uh oh!

hadoop-yetus commented Aug 20, 2021

Uh oh!

wzhallright commented Aug 27, 2021

Uh oh!

xkrogen left a comment

Uh oh!

Uh oh!

hadoop-yetus commented Sep 14, 2021

Uh oh!

xkrogen commented Sep 14, 2021 •

edited

Loading

Uh oh!

wzhallright commented Sep 15, 2021 •

edited

Loading

Uh oh!

hadoop-yetus commented Sep 15, 2021

Uh oh!

xkrogen left a comment

Uh oh!

xkrogen commented Sep 15, 2021

Uh oh!

Uh oh!

HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN #3317

HDFS-16181. [SBN Read] Fix metric of RpcRequestCacheMissAmount can't display when tailEditLog form JN #3317

Uh oh!

Conversation

wzhallright commented Aug 20, 2021

Description of PR

Uh oh!

hadoop-yetus commented Aug 20, 2021

Uh oh!

wzhallright commented Aug 27, 2021

Uh oh!

xkrogen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hadoop-yetus commented Sep 14, 2021

Uh oh!

xkrogen commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wzhallright commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hadoop-yetus commented Sep 15, 2021

Uh oh!

xkrogen left a comment

Choose a reason for hiding this comment

Uh oh!

xkrogen commented Sep 15, 2021

Uh oh!

Uh oh!

xkrogen commented Sep 14, 2021 •

edited

Loading

wzhallright commented Sep 15, 2021 •

edited

Loading