Skip to content

HBASE-27890 Expose a getter on Connection/AsyncConnection for getting… - #5257

Closed
frostruan wants to merge 1 commit into
apache:masterfrom
frostruan:HBASE-27890
Closed

HBASE-27890 Expose a getter on Connection/AsyncConnection for getting…#5257
frostruan wants to merge 1 commit into
apache:masterfrom
frostruan:HBASE-27890

Conversation

@frostruan

Copy link
Copy Markdown
Contributor

… public access to connection metrics

@frostruan

Copy link
Copy Markdown
ContributorAuthor

hi @bbeaudreault

I have completed a simple version. I believe there must be a lot of improvement. If you have time, would you mind taking a look and giving me some comments and suggestions?

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 0sDocker mode activated.
-1 ❌patch0m 2s#5257 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.
SubsystemReport/Notes
GITHUB PR#5257
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/1/console
versionsgit=2.25.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

1 similar comment
@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 0sDocker mode activated.
-1 ❌patch0m 2s#5257 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.
SubsystemReport/Notes
GITHUB PR#5257
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/1/console
versionsgit=2.25.1
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 0sDocker mode activated.
-1 ❌patch0m 3s#5257 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.
SubsystemReport/Notes
GITHUB PR#5257
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/1/console
versionsgit=2.17.1
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 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 14sMaven dependency ordering for branch
+1 💚mvninstall2m 58smaster passed
+1 💚compile3m 8smaster passed
+1 💚checkstyle0m 48smaster passed
+1 💚spotless0m 44sbranch has no errors when running spotless:check.
+1 💚spotbugs2m 15smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall2m 41sthe patch passed
+1 💚compile3m 5sthe patch passed
+1 💚javac3m 5sthe patch passed
+1 💚checkstyle0m 47sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck10m 13sPatch does not cause any errors with Hadoop 3.2.4 3.3.5.
+1 💚spotless0m 42spatch has no errors when running spotless:check.
+1 💚spotbugs2m 24sthe patch passed
_ Other Tests _
+1 💚asflicense0m 15sThe patch does not generate ASF License warnings.
37m 29s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#5257
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
unameLinux 8fc00b2a1d35 5.4.0-1097-aws #105~18.04.1-Ubuntu SMP Mon Feb 13 17:50:57 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 79c985f
Default JavaEclipse Adoptium-11.0.17+8
Max. process+thread count79 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/console
versionsgit=2.34.1 maven=3.8.6 spotbugs=4.7.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 52sDocker 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 11sMaven dependency ordering for branch
+1 💚mvninstall2m 47smaster passed
+1 💚compile1m 6smaster passed
+1 💚shadedjars4m 42sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 44smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
+1 💚mvninstall2m 40sthe patch passed
+1 💚compile1m 7sthe patch passed
+1 💚javac1m 7sthe patch passed
+1 💚shadedjars4m 36spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 42sthe patch passed
_ Other Tests _
+1 💚unit1m 27shbase-client in the patch passed.
-1 ❌unit223m 23shbase-server in the patch failed.
249m 22s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#5257
Optional Testsjavac javadoc unit shadedjars compile
unameLinux d6d6efbfc16f 5.4.0-144-generic #161-Ubuntu SMP Fri Feb 3 14:49:04 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 79c985f
Default JavaEclipse Adoptium-11.0.17+8
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/testReport/
Max. process+thread count4616 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/console
versionsgit=2.34.1 maven=3.8.6
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 16sDocker 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 11sMaven dependency ordering for branch
+1 💚mvninstall2m 23smaster passed
+1 💚compile0m 58smaster passed
+1 💚shadedjars4m 32sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 39smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
+1 💚mvninstall2m 21sthe patch passed
+1 💚compile0m 56sthe patch passed
+1 💚javac0m 56sthe patch passed
+1 💚shadedjars5m 21spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 53sthe patch passed
_ Other Tests _
+1 💚unit1m 51shbase-client in the patch passed.
-1 ❌unit245m 38shbase-server in the patch failed.
270m 49s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#5257
Optional Testsjavac javadoc unit shadedjars compile
unameLinux c5ba642bab99 5.4.0-144-generic #161-Ubuntu SMP Fri Feb 3 14:49:04 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 79c985f
Default JavaTemurin-1.8.0_352-b08
unithttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/testReport/
Max. process+thread count4206 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5257/2/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

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.

Better make an interface and let the MetricsConnection implements it, where we only expose read only methods. Like OnlineRegions and MutableOnlineRegions

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'm also wondering if we want to try to avoid exposing the codahale metrics Snapshot directly? It might make compatibility tricky in the future, if the upstream api there changes? Just a thought

@Apache9

Copy link
Copy Markdown
Contributor

Any updates here?

Thanks.

@frostruan

Copy link
Copy Markdown
ContributorAuthor

Sorry, got a little stuck here. I haven't thought of a good way to expose the metric information in a read-only mode without introducing too many new data structures. I will continue to try.

Thanks Duo.

@bbeaudreault

Copy link
Copy Markdown
Contributor

How many do you think we need to introduce?

@Apache9

Copy link
Copy Markdown
Contributor

I do not think we need to introduce new 'data structures', instead, we need to introduce new 'interfaces', where the current data structures will implement these interfaces, and provide read only access to the metrics. Anyway, maybe we need to add lots of new methods.

Thanks.

@frostruan

Copy link
Copy Markdown
ContributorAuthor

Thanks all for the comments. @Apache9@bbeaudreault

Many of our metrics are Histogram or Timer types. In order to avoid user code updating metrics, we can only provide interfaces similar to snapshots. As we can imagine, our metric interfaces need to define getMin, getMax, getMean, getMedian, get99thPercentile, get999thPercentile .... , as Duo said, a lot of new methods need to be added, and I am still thinking about how to add them. Thanks.

@Apache9

Copy link
Copy Markdown
Contributor

Any updates here?

@frostruan

Thanks.

@frostruan

Copy link
Copy Markdown
ContributorAuthor

After our internal discussion, it is not a good practice to expose metrics directly in connection and AsyncConnection. Maybe we should avoid doing so. So maybe we can close this issue.

Thanks Duo.@Apache9

@Apache9

Copy link
Copy Markdown
Contributor

OK, so do we still need HBASE-27885?

@Apache9Apache9 closed this Aug 31, 2025
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

@frostruan@Apache-HBase@Apache9@bbeaudreault