-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-26141 Add tracing support for HTable and sync connection on branch-2 #3696
Conversation
taklwu
commented
Sep 23, 2021
- RPC and IPC tracing supported by HBASE-26125
- WAL tracing supported by HBASE-26131
…nch-2 * RPC and IPC supported by HBASE-26125 * WAL tracing supported by HBASE-26131
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM.
We have another issue for tracing the Admin interface?
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have another issue for tracing the Admin interface?
Hey Duo, Thanks for pointing out the missing part, and I would like to confirm if the Admin and HBaseAdmin are the interface you're referring?
basically, I didn't find tracing implemented for RawAsyncHBaseAdmin and AsyncHBaseAdmin in HBASE-22120, did I miss some JIRAs ?
anyhow, looks like the scope of support Admin interface is huge as well, so if it's needed, I would like to have a followup JIRA for the sync client if possible, what do you think?
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
Outdated
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
OK, checked the release note of HBASE-22120, it does not mention the tracing about the Admin interface too. I think the rpc level tracing can basically work for Admin operations. We could file another issue if there are real requirements in the future. Thanks. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
the javac warning of |
if (rpcClient != null) { | ||
rpcClient.close(); | ||
} | ||
synchronized (this) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fix the error of spotbugs report during one of the precommit checks
Bug type IS2_INCONSISTENT_SYNC (click for details)
In class org.apache.hadoop.hbase.client.ConnectionImplementation
Field org.apache.hadoop.hbase.client.ConnectionImplementation.choreService
Synchronized 50% of the time
Unsynchronized access at ConnectionImplementation.java:[line 2131]
Unsynchronized access at ConnectionImplementation.java:[line 2132]
Synchronized access at ConnectionImplementation.java:[line 617]
Synchronized access at ConnectionImplementation.java:[line 620]
Synchronized access at ConnectionImplementation.java:[line 618]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try it.