HADOOP-18840. Add enQueue time to RpcMetrics #5926

2005hithlj · 2023-08-05T06:54:36Z

Description of PR

JIRA: HADOOP-18840. Add enQueue time to RpcMetrics.

How was this patch tested?

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?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

hadoop-yetus · 2023-08-05T10:42:27Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 38s		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 _
-1 ❌	mvninstall	45m 1s	/branch-mvninstall-root.txt	root in trunk failed.
+1 💚	compile	19m 4s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	compile	16m 0s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	checkstyle	1m 26s		trunk passed
+1 💚	mvnsite	1m 46s		trunk passed
+1 💚	javadoc	1m 23s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 54s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	2m 41s		trunk passed
+1 💚	shadedclient	35m 44s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 57s		the patch passed
+1 💚	compile	16m 27s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javac	16m 27s		the patch passed
+1 💚	compile	15m 56s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	javac	15m 56s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	1m 21s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 3 new + 195 unchanged - 0 fixed = 198 total (was 195)
+1 💚	mvnsite	1m 46s		the patch passed
+1 💚	javadoc	1m 17s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	1m 0s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	2m 46s		the patch passed
+1 💚	shadedclient	35m 59s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	19m 9s		hadoop-common in the patch passed.
+1 💚	asflicense	1m 10s		The patch does not generate ASF License warnings.
		226m 17s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/1/artifact/out/Dockerfile
GITHUB PR	#5926
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 89b46bc70557 4.15.0-212-generic #223-Ubuntu SMP Tue May 23 13:09:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `a40b534`
Default Java	Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/1/testReport/
Max. process+thread count	2356 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/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.

Hexiaoqiao · 2023-08-07T06:08:54Z

LGTM. Would you mind to add unit test to cover it. Add check report from Yetus first please. Thanks.

2005hithlj · 2023-08-07T09:00:37Z

LGTM. Would you mind to add unit test to cover it. Add check report from Yetus first please. Thanks.
@Hexiaoqiao sir, Thanks for your review, I will add unit test to cover it.

slfan1989 · 2023-08-07T09:16:38Z

@2005hithlj Thank you for your contribution! But from my personal point of view, we have too many metrics. Hadoop rpc has very good performance. What is this metric used for? Can this metric be of any specific use?

slfan1989 · 2023-08-07T09:20:41Z

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

@@ -81,6 +83,9 @@ public class RpcMetrics {
          new MutableQuantiles[intervals.length];
      for (int i = 0; i < intervals.length; i++) {
        int interval = intervals[i];
+        rpcEnQueueTimeQuantiles[i] = registry.newQuantiles("rpcEnQueueTime"
+                + interval + "s", "rpc enqueue time in " + metricsTimeUnit, "ops",


indentation 5 chars.

slfan1989 · 2023-08-07T09:21:15Z

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

@@ -257,6 +264,19 @@ public void incrReceivedBytes(int count) {
    receivedBytes.incr(count);
  }

+  /**
+   * Add an RPC enqueue time sample
+   * @param enQTime the queue time


Comments should describe in more detail.

xinglin

change lgtm.

Also FYI, in HDFS-17042, we added OverallProcessingTime for each rpc method. It measures the latency from when an rpc request arrives at the NN to when a response is sent back. You might be interested to check out these metrics too.

2005hithlj · 2023-08-08T02:29:28Z

change lgtm.

Also FYI, in HDFS-17042, we added OverallProcessingTime for each rpc method. It measures the latency from when an rpc request arrives at the NN to when a response is sent back. You might be interested to check out these metrics too.

@xinglin sir.
Thanks for your review, We also need to know the exact enQeue time in order to troubleshoot performance bottlenecks.

slfan1989 · 2023-08-08T03:48:00Z

Change lgtm to wait for Yetus report.

hadoop-yetus · 2023-08-08T06:01:47Z

💔 -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 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 appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	45m 6s		trunk passed
+1 💚	compile	17m 30s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	compile	17m 10s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	checkstyle	1m 25s		trunk passed
+1 💚	mvnsite	1m 41s		trunk passed
+1 💚	javadoc	1m 17s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 59s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	2m 51s		trunk passed
+1 💚	shadedclient	39m 40s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 57s		the patch passed
+1 💚	compile	16m 58s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javac	16m 58s		the patch passed
+1 💚	compile	17m 45s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	javac	17m 45s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	1m 18s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 3 new + 213 unchanged - 0 fixed = 216 total (was 213)
+1 💚	mvnsite	1m 42s		the patch passed
+1 💚	javadoc	1m 9s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 56s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	3m 0s		the patch passed
+1 💚	shadedclient	40m 52s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	20m 27s	/patch-unit-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch passed.
+1 💚	asflicense	1m 3s		The patch does not generate ASF License warnings.
		237m 39s

Reason	Tests
Failed junit tests	hadoop.ipc.TestRPCCallBenchmark

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/2/artifact/out/Dockerfile
GITHUB PR	#5926
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 2fbc6a5dc938 4.15.0-212-generic #223-Ubuntu SMP Tue May 23 13:09:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `91b5409`
Default Java	Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/2/testReport/
Max. process+thread count	3159 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/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.

hadoop-yetus · 2023-08-08T06:23:49Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 47s		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 appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	47m 13s		trunk passed
+1 💚	compile	19m 24s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	compile	16m 30s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	checkstyle	1m 21s		trunk passed
+1 💚	mvnsite	1m 43s		trunk passed
+1 💚	javadoc	1m 16s		trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 54s		trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	2m 40s		trunk passed
+1 💚	shadedclient	38m 20s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 10s		the patch passed
+1 💚	compile	18m 26s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javac	18m 26s		the patch passed
+1 💚	compile	17m 8s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	javac	17m 8s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	1m 16s	/results-checkstyle-hadoop-common-project_hadoop-common.txt	hadoop-common-project/hadoop-common: The patch generated 2 new + 213 unchanged - 0 fixed = 215 total (was 213)
+1 💚	mvnsite	1m 41s		the patch passed
+1 💚	javadoc	1m 13s		the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04
+1 💚	javadoc	0m 51s		the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
+1 💚	spotbugs	2m 45s		the patch passed
+1 💚	shadedclient	39m 4s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	20m 31s	/patch-unit-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch passed.
+1 💚	asflicense	1m 12s		The patch does not generate ASF License warnings.
		239m 13s

Reason	Tests
Failed junit tests	hadoop.ipc.TestRPCCallBenchmark

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/3/artifact/out/Dockerfile
GITHUB PR	#5926
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 5ea36bf704e6 4.15.0-212-generic #223-Ubuntu SMP Tue May 23 13:09:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `66f3ee5`
Default Java	Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/3/testReport/
Max. process+thread count	1302 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5926/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.

Hexiaoqiao · 2023-08-08T07:36:54Z

@2005hithlj Please check the failed unit test and checkstyle.

2005hithlj · 2023-08-09T02:06:34Z

@2005hithlj Please check the failed unit test and checkstyle.

@Hexiaoqiao sir.
The failed UT 'TestRPCCallBenchmark is not related to my modification, and it will success in my modification branch in my local env.

And Checkstyle Warn:
RpcMetrics.java:122: @Metric("EQueue time") MutableRate rpcEnQueueTime;:38: Variable 'rpcEnQueueTime' must be private and have accessor methods. [VisibilityModifier]
RpcMetrics.java:123: MutableQuantiles[] rpcEnQueueTimeQuantiles;:22: Variable 'rpcEnQueueTimeQuantiles' must be private and have accessor methods. [VisibilityModifier]

If I follow the above prompts to modify, it will break the style of the RpcMetrics class.

xinglin · 2023-08-09T06:34:57Z

@2005hithlj Please check the failed unit test and checkstyle.

@Hexiaoqiao sir. The failed UT 'TestRPCCallBenchmark is not related to my modification, and it will success in my modification branch in my local env.

And Checkstyle Warn: RpcMetrics.java:122: @Metric("EQueue time") MutableRate rpcEnQueueTime;:38: Variable 'rpcEnQueueTime' must be private and have accessor methods. [VisibilityModifier] RpcMetrics.java:123: MutableQuantiles[] rpcEnQueueTimeQuantiles;:22: Variable 'rpcEnQueueTimeQuantiles' must be private and have accessor methods. [VisibilityModifier]

If I follow the above prompts to modify, it will break the style of the RpcMetrics class.

I don't know the unit test but yeah, for the checkstyle, we can ignore these warnings: for metrics, we don't add visibility modifier. None of the existing metrics has the visibility modifier.

slfan1989 · 2023-08-09T11:40:40Z

Change to LGTM.

Hexiaoqiao

LGTM. +1

Hexiaoqiao · 2023-08-10T02:39:18Z

Committed to trunk. Thanks @2005hithlj for your contribution and @slfan1989 @xinglin for your reviews!

Hexiaoqiao · 2023-08-10T02:41:36Z

Hi @2005hithlj Would you mind to check why TestRPCCallBenchmark failed and try to fix it? Thanks.

2005hithlj · 2023-08-10T03:06:53Z

Hi @2005hithlj Would you mind to check why TestRPCCallBenchmark failed and try to fix it? Thanks.

OK Sir, I have created a issue HADOOP-18846 to track it.

…ed by Liangjun He. Reviewed-by: Shilun Fan <slfan1989@apache.org> Reviewed-by: Xing Lin <linxingnku@gmail.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>

github-actions bot added trunk Common labels Aug 5, 2023

slfan1989 reviewed Aug 7, 2023

View reviewed changes

xinglin approved these changes Aug 7, 2023

View reviewed changes

2005hithlj force-pushed the HADOOP-18840 branch from a40b534 to 91b5409 Compare August 8, 2023 02:02

HADOOP-18840. Add enQueue time to RpcMetrics

66f3ee5

2005hithlj force-pushed the HADOOP-18840 branch from 91b5409 to 66f3ee5 Compare August 8, 2023 02:23

slfan1989 approved these changes Aug 9, 2023

View reviewed changes

Hexiaoqiao approved these changes Aug 10, 2023

View reviewed changes

Hexiaoqiao merged commit b6edcb9 into apache:trunk Aug 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-18840. Add enQueue time to RpcMetrics #5926

HADOOP-18840. Add enQueue time to RpcMetrics #5926

2005hithlj commented Aug 5, 2023 •

edited by slfan1989

Loading

hadoop-yetus commented Aug 5, 2023

Hexiaoqiao commented Aug 7, 2023

2005hithlj commented Aug 7, 2023

slfan1989 commented Aug 7, 2023 •

edited

Loading

slfan1989 Aug 7, 2023

slfan1989 Aug 7, 2023

xinglin left a comment

2005hithlj commented Aug 8, 2023

slfan1989 commented Aug 8, 2023

hadoop-yetus commented Aug 8, 2023

hadoop-yetus commented Aug 8, 2023

Hexiaoqiao commented Aug 8, 2023

2005hithlj commented Aug 9, 2023

xinglin commented Aug 9, 2023

slfan1989 commented Aug 9, 2023

Hexiaoqiao left a comment

Hexiaoqiao commented Aug 10, 2023

Hexiaoqiao commented Aug 10, 2023

2005hithlj commented Aug 10, 2023

HADOOP-18840. Add enQueue time to RpcMetrics #5926

HADOOP-18840. Add enQueue time to RpcMetrics #5926

Conversation

2005hithlj commented Aug 5, 2023 • edited by slfan1989 Loading

Description of PR

How was this patch tested?

For code changes:

hadoop-yetus commented Aug 5, 2023

Hexiaoqiao commented Aug 7, 2023

2005hithlj commented Aug 7, 2023

slfan1989 commented Aug 7, 2023 • edited Loading

slfan1989 Aug 7, 2023

Choose a reason for hiding this comment

slfan1989 Aug 7, 2023

Choose a reason for hiding this comment

xinglin left a comment

Choose a reason for hiding this comment

2005hithlj commented Aug 8, 2023

slfan1989 commented Aug 8, 2023

hadoop-yetus commented Aug 8, 2023

hadoop-yetus commented Aug 8, 2023

Hexiaoqiao commented Aug 8, 2023

2005hithlj commented Aug 9, 2023

xinglin commented Aug 9, 2023

slfan1989 commented Aug 9, 2023

Hexiaoqiao left a comment

Choose a reason for hiding this comment

Hexiaoqiao commented Aug 10, 2023

Hexiaoqiao commented Aug 10, 2023

2005hithlj commented Aug 10, 2023

2005hithlj commented Aug 5, 2023 •

edited by slfan1989

Loading

slfan1989 commented Aug 7, 2023 •

edited

Loading