Skip to content

HBASE-27153 Improvements to read-path tracing - #4572

Merged
ndimiduk merged 1 commit into
apache:masterfrom
ndimiduk:27153-trace-read-path
Aug 2, 2022
Merged

HBASE-27153 Improvements to read-path tracing#4572
ndimiduk merged 1 commit into
apache:masterfrom
ndimiduk:27153-trace-read-path

Conversation

@ndimiduk

Copy link
Copy Markdown
Member

Take another pass through tracing of the read path, make adjustments accordingly. One of the major concerns raised previously is that we create a span for every block access. Start by simplifying this to trace events and see what else comes up.

Block-level and stream-level interactions are represented as span events with attached attributes (that's otel terminology for "structured logs", attached to the spans). I made an effort to surface information that an operator might be interested in while diagnosing slow read performance, and details that the previous generation of dev had already seen fit to surface via trace-level logging. We no longer emit spans for HFileReaderImpl.readBlock. I hope this is a good compromise between visibility and performance.

Also added a span context for block-cache pre-fetching.

@ndimiduk

Copy link
Copy Markdown
MemberAuthor

These are some screenshots of traces emitted by a build of branch-2 plus this patch, running in pseudo-distributed mode, sent to a Jaeger backed. Notice that the span events are marked as lines on spans. See the attributes on those event.s

Screen Shot 2022-06-23 at 2 15 57 PM

Screen Shot 2022-06-23 at 2 16 25 PM

@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@apurtellapurtell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we didn't have to pass the scan attribute builder through as an extra parameter to methods that don't care about tracing per se, they simply exist on the traced path. Is it possible to use threadlocals or some otel notion of thread context for these? Or is that an anti-pattern?

@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@ndimiduk

Copy link
Copy Markdown
MemberAuthor

@apurtell

I wish we didn't have to pass the scan attribute builder through as an extra parameter to methods that don't care about tracing per se, they simply exist on the traced path.

The reason that I have to pass this attributes object around is that I'm providing context that is eventually annotated to a span "event" -- essentially a log line. Otel API lets me stuff arbitrary attributes into a thread local for span attributes, not for span event attributes.

Is it possible to use threadlocals or some otel notion of thread context for these? Or is that an anti-pattern?

I am also not a fan of the current approach. Introducing a ThreadLocal for this purpose that is inspected at arbitrary code points seems ham-handed to me. At the very least, it should somehow be limited in scope to the current request, not the current thread. Let me see if I can find a better solution.

@ndimiduk
ndimidukforce-pushed the 27153-trace-read-path branch from 74de7bb to 3e73a7eCompareJune 28, 2022 12:20
@ndimiduk

Copy link
Copy Markdown
MemberAuthor

@apurtell I changed the implementation to use the otel Context for propagating the additional information I wanted, without changing the method signatures. I'm not sure if this is intended use of the otel api or not...

@ndimiduk
ndimiduk requested a review from apurtellJune 28, 2022 12:24
@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@Apache-HBase

This comment was marked as outdated.

@apurtell

apurtell commented Jul 1, 2022

Copy link
Copy Markdown
Contributor

Approved. I like the

 final AttributesBuilder attributesBuilder = builderFromContext(Context.current());
attributesBuilder...

style way of building attributes along a method call chain without adding the builder as a method parameter.

@ndimiduk
ndimidukforce-pushed the 27153-trace-read-path branch from 3e73a7e to 25aa61aCompareJuly 7, 2022 14:35
@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 32sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 23sMaven dependency ordering for branch
+1 💚mvninstall3m 32smaster passed
+1 💚compile4m 35smaster passed
+1 💚checkstyle1m 10smaster passed
+1 💚spotless0m 49sbranch has no errors when running spotless:check.
+1 💚spotbugs3m 5smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for patch
+1 💚mvninstall2m 48sthe patch passed
+1 💚compile3m 56sthe patch passed
+1 💚javac3m 56sthe patch passed
+1 💚checkstyle1m 17sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck17m 31sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotless1m 1spatch has no errors when running spotless:check.
+1 💚spotbugs4m 23sthe patch passed
_ Other Tests _
+1 💚asflicense0m 35sThe patch does not generate ASF License warnings.
54m 45s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#4572
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux dfb41767a851 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 6886d02
Default JavaAdoptOpenJDK-1.8.0_282-b08
Max. process+thread count64 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 6sDocker mode activated.
-0 ⚠️yetus0m 2sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 14sMaven dependency ordering for branch
+1 💚mvninstall2m 52smaster passed
+1 💚compile1m 20smaster passed
+1 💚shadedjars4m 38sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 1smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall2m 40sthe patch passed
+1 💚compile1m 13sthe patch passed
+1 💚javac1m 13sthe patch passed
+1 💚shadedjars4m 19spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 56sthe patch passed
_ Other Tests _
+1 💚unit1m 36shbase-common in the patch passed.
+1 💚unit1m 27shbase-client in the patch passed.
-1 ❌unit231m 56shbase-server in the patch failed.
257m 52s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#4572
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 3a2f1d160cdd 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 6886d02
Default JavaAdoptOpenJDK-1.8.0_282-b08
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/testReport/
Max. process+thread count3026 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 46sDocker mode activated.
-0 ⚠️yetus0m 2sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 6sMaven dependency ordering for branch
+1 💚mvninstall4m 17smaster passed
+1 💚compile2m 9smaster passed
+1 💚shadedjars4m 55sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 31smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for patch
+1 💚mvninstall4m 9sthe patch passed
+1 💚compile2m 10sthe patch passed
+1 💚javac2m 10sthe patch passed
+1 💚shadedjars5m 50spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 19sthe patch passed
_ Other Tests _
+1 💚unit2m 51shbase-common in the patch passed.
+1 💚unit2m 4shbase-client in the patch passed.
-1 ❌unit272m 30shbase-server in the patch failed.
308m 17s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#4572
Optional Testsjavac javadoc unit shadedjars compile
unameLinux b934934803c7 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 6886d02
Default JavaAdoptOpenJDK-11.0.10+9
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/testReport/
Max. process+thread count2567 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/4/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@apurtell

Copy link
Copy Markdown
Contributor

@ndimiduk
ndimidukforce-pushed the 27153-trace-read-path branch from 25aa61a to 38ed920CompareAugust 1, 2022 11:49
@ndimiduk

Copy link
Copy Markdown
MemberAuthor

TestAsyncTableScan.testReverseScanNoStopKey[1: scan=batch] failure seems relevant.

This failure is baffling to me. The assertion failed because a span with the name TracedScanResultConsumer#onComplete was not found in the collection. However, you can see that a span of that name is printed in the DEBUG logging!

Signed-off-by: Andrew Purtell <apurtell@apache.org>
@ndimiduk
ndimidukforce-pushed the 27153-trace-read-path branch from 38ed920 to 0bf363eCompareAugust 1, 2022 11:57
@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec3m 28sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 51sMaven dependency ordering for branch
+1 💚mvninstall2m 22smaster passed
+1 💚compile3m 24smaster passed
+1 💚checkstyle0m 51smaster passed
+1 💚spotless0m 42sbranch has no errors when running spotless:check.
+1 💚spotbugs2m 14smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall2m 29sthe patch passed
+1 💚compile4m 6sthe patch passed
+1 💚javac4m 6sthe patch passed
+1 💚checkstyle1m 4sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck13m 38sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotless0m 54spatch has no errors when running spotless:check.
+1 💚spotbugs4m 17sthe patch passed
_ Other Tests _
+1 💚asflicense0m 39sThe patch does not generate ASF License warnings.
48m 3s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#4572
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux a9ae449a9f06 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e8c14ee
Default JavaAdoptOpenJDK-1.8.0_282-b08
Max. process+thread count60 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 4sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 17sMaven dependency ordering for branch
+1 💚mvninstall2m 42smaster passed
+1 💚compile1m 26smaster passed
+1 💚shadedjars3m 44sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 58smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 14sMaven dependency ordering for patch
+1 💚mvninstall2m 41sthe patch passed
+1 💚compile1m 23sthe patch passed
+1 💚javac1m 23sthe patch passed
+1 💚shadedjars3m 44spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 59sthe patch passed
_ Other Tests _
+1 💚unit1m 54shbase-common in the patch passed.
+1 💚unit1m 23shbase-client in the patch passed.
-1 ❌unit208m 39shbase-server in the patch failed.
233m 22s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#4572
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 868547b198ce 5.4.0-122-generic #138-Ubuntu SMP Wed Jun 22 15:00:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e8c14ee
Default JavaAdoptOpenJDK-11.0.10+9
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/testReport/
Max. process+thread count2442 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 35sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for branch
+1 💚mvninstall2m 30smaster passed
+1 💚compile1m 3smaster passed
+1 💚shadedjars4m 1sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 47smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 11sMaven dependency ordering for patch
+1 💚mvninstall2m 12sthe patch passed
+1 💚compile1m 3sthe patch passed
+1 💚javac1m 3sthe patch passed
+1 💚shadedjars4m 0spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 46sthe patch passed
_ Other Tests _
+1 💚unit1m 23shbase-common in the patch passed.
+1 💚unit1m 7shbase-client in the patch passed.
-1 ❌unit228m 2shbase-server in the patch failed.
250m 4s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#4572
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 24056846fce9 5.4.0-1081-aws #88~18.04.1-Ubuntu SMP Thu Jun 23 16:29:17 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e8c14ee
Default JavaAdoptOpenJDK-1.8.0_282-b08
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/testReport/
Max. process+thread count2629 (vs. ulimit of 30000)
modulesC: hbase-common hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4572/5/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ndimiduk
ndimiduk merged commit d734acc into apache:masterAug 2, 2022
@ndimiduk
ndimiduk deleted the 27153-trace-read-path branch August 2, 2022 12:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ndimiduk@Apache-HBase@apurtell@Apache9