HBASE-26366 Provide meaningful parent spans to ZK interactions #4459

ndimiduk · 2022-05-23T14:27:36Z

As of this patch, now every span that's emitted in my unit test has a meaningful parent span, tied back to either a server activity or the ZKWatcher handler pool. PTAL.

apurtell · 2022-05-26T22:25:11Z

...ver/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerInternalsTracing.java

@@ -0,0 +1,313 @@
+/*


Skimmed the rest of the changes. Spent more time here. LGTM

Apache9

In general I think the approach is fine, my only concern is about passing the span to another thread, as the span may end before its sub spans? I'm not sure if this is a big problem...

hbase-server/src/main/java/org/apache/hadoop/hbase/MetaRegionLocationCache.java

ndimiduk · 2022-06-02T14:01:36Z

FYI I'm working through the branch-2 back-port and it's finding some other places I missed in this patch for master. Will be back.

ndimiduk · 2022-06-03T14:49:56Z

rebased
decouple spawned thread spans from parent thread spans, just in case
ensure all manually-managed spans have a status set
rename/move test to better match its scope

ndimiduk · 2022-06-09T10:52:20Z

I tracked down the broken test to 313a37f.

Signed-off-by: Andrew Purtell <[email protected]>

- decouple spawned thread spans from parent thread spans, just in case - ensure all manually-managed spans have a status set - rename/move test to better match its scope

Apache-HBase · 2022-06-09T16:11:42Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 53s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+0 🆗	mvndep	0m 15s	Maven dependency ordering for branch
+1 💚	mvninstall	4m 10s	master passed
+1 💚	compile	5m 39s	master passed
+1 💚	checkstyle	1m 24s	master passed
+1 💚	spotless	1m 1s	branch has no errors when running spotless:check.
+1 💚	spotbugs	3m 59s	master passed
-0 ⚠️	patch	0m 51s	Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s	Maven dependency ordering for patch
+1 💚	mvninstall	3m 41s	the patch passed
+1 💚	compile	4m 30s	the patch passed
+1 💚	javac	4m 30s	the patch passed
-0 ⚠️	checkstyle	0m 30s	hbase-server: The patch generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	16m 15s	Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚	spotless	0m 41s	patch has no errors when running spotless:check.
+1 💚	spotbugs	3m 9s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 37s	The patch does not generate ASF License warnings.
		54m 49s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#4459
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux f75666d32a19 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `414cfb3`
Default Java	AdoptOpenJDK-1.8.0_282-b08
checkstyle	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
Max. process+thread count	73 (vs. ulimit of 30000)
modules	C: hbase-common hbase-client hbase-zookeeper hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/console
versions	git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2022-06-09T19:23:20Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 15s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 13s	Maven dependency ordering for branch
+1 💚	mvninstall	3m 24s	master passed
+1 💚	compile	1m 38s	master passed
+1 💚	shadedjars	4m 15s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	1m 15s	master passed
-0 ⚠️	patch	6m 4s	Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 13s	Maven dependency ordering for patch
+1 💚	mvninstall	3m 7s	the patch passed
+1 💚	compile	1m 37s	the patch passed
+1 💚	javac	1m 37s	the patch passed
+1 💚	shadedjars	4m 12s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	1m 13s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 3s	hbase-common in the patch passed.
+1 💚	unit	1m 14s	hbase-client in the patch passed.
+1 💚	unit	0m 38s	hbase-zookeeper in the patch passed.
+1 💚	unit	201m 54s	hbase-server in the patch passed.
		230m 5s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#4459
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 5239da22b171 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `414cfb3`
Default Java	AdoptOpenJDK-11.0.10+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/testReport/
Max. process+thread count	2487 (vs. ulimit of 30000)
modules	C: hbase-common hbase-client hbase-zookeeper hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2022-06-09T19:42:51Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 27s	Docker mode activated.
-0 ⚠️	yetus	0m 2s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+0 🆗	mvndep	0m 17s	Maven dependency ordering for branch
+1 💚	mvninstall	3m 17s	master passed
+1 💚	compile	1m 38s	master passed
+1 💚	shadedjars	5m 15s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	1m 17s	master passed
-0 ⚠️	patch	7m 3s	Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s	Maven dependency ordering for patch
+1 💚	mvninstall	3m 3s	the patch passed
+1 💚	compile	1m 37s	the patch passed
+1 💚	javac	1m 37s	the patch passed
+1 💚	shadedjars	4m 57s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	1m 9s	the patch passed
		_ Other Tests _
+1 💚	unit	1m 39s	hbase-common in the patch passed.
+1 💚	unit	1m 16s	hbase-client in the patch passed.
+1 💚	unit	0m 39s	hbase-zookeeper in the patch passed.
+1 💚	unit	235m 53s	hbase-server in the patch passed.
		265m 34s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#4459
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 7b6103d087f2 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `414cfb3`
Default Java	AdoptOpenJDK-1.8.0_282-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/testReport/
Max. process+thread count	2407 (vs. ulimit of 30000)
modules	C: hbase-common hbase-client hbase-zookeeper hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4459/5/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ndimiduk requested review from Apache9, apurtell, joshelser and taklwu May 23, 2022 14:27

This comment was marked as outdated.

Sign in to view

apurtell approved these changes May 26, 2022

View reviewed changes

ndimiduk force-pushed the 26366-meaningful-parents-zk-spans branch from 249a944 to c17b447 Compare May 31, 2022 13:55

This comment was marked as outdated.

Sign in to view

Apache9 reviewed Jun 2, 2022

View reviewed changes

hbase-server/src/main/java/org/apache/hadoop/hbase/MetaRegionLocationCache.java Outdated Show resolved Hide resolved

ndimiduk force-pushed the 26366-meaningful-parents-zk-spans branch from c17b447 to a8a8c9d Compare June 3, 2022 14:47

ndimiduk mentioned this pull request Jun 3, 2022

Backport "HBASE-26366 Provide meaningful parent spans to ZK interactions" to branch-2 #4487

Merged

This comment was marked as outdated.

Sign in to view

ndimiduk force-pushed the 26366-meaningful-parents-zk-spans branch from a8a8c9d to 3c2f637 Compare June 6, 2022 09:14

This comment was marked as outdated.

Sign in to view

ndimiduk added 2 commits June 9, 2022 14:35

HBASE-26366 Provide meaningful parent spans to ZK interactions

51eef0d

Signed-off-by: Andrew Purtell <[email protected]>

PR feedback, also...

b933fca

- decouple spawned thread spans from parent thread spans, just in case - ensure all manually-managed spans have a status set - rename/move test to better match its scope

ndimiduk force-pushed the 26366-meaningful-parents-zk-spans branch from 3c2f637 to b933fca Compare June 9, 2022 12:35

ndimiduk merged commit ed7e15d into apache:master Jun 10, 2022

ndimiduk deleted the 26366-meaningful-parents-zk-spans branch June 10, 2022 14:26

HBASE-26366 Provide meaningful parent spans to ZK interactions #4459

HBASE-26366 Provide meaningful parent spans to ZK interactions #4459

Uh oh!

Conversation

ndimiduk commented May 23, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

apurtell May 26, 2022

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Apache9 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ndimiduk commented Jun 2, 2022

Uh oh!

ndimiduk commented Jun 3, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Jun 9, 2022

Uh oh!

Apache-HBase commented Jun 9, 2022

Uh oh!

Apache-HBase commented Jun 9, 2022

Uh oh!

Apache-HBase commented Jun 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants