bbeaudreault commented on code in PR #4285:
URL: https://github.com/apache/hbase/pull/4285#discussion_r841107787
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java:
##########
@@ -132,7 +133,8 @@ public AsyncConnectionImpl(Configuration conf,
ConnectionRegistry registry, Stri
this.connConf = new AsyncConnectionConfiguration(conf);
this.registry = registry;
if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) {
- this.metrics = Optional.of(new MetricsConnection(this.toString(), () ->
null, () -> null));
+ String scope = conf.get(METRICS_SCOPE_KEY, clusterId + "-" + hashCode());
Review Comment:
This scope is added to the JMX bean name by JmxReporter. A good default
value here can make it easier for find the JMX metrics for a specific
connection.
I personally think `this.toString()` is a relatively bad default because it
doesn't help identify at all. If you have multiple connections going, you'll
just see 2 beans with: `scope=AsyncConnectionImpl@23fd322` and
`scope=AsyncConnectionImpl@abcd123`. What do those signify? Impossible to
really know.
The one benefit of that default is that the hashCode ensures that it will
indeed be 2 distinct beans in jmx.
The change I propose still may not help if you have 2 connections to the
same cluster -- for clusterId `foo` you might then see `foo-23fd322` and
`foo-abcd123`. In that case, if you care, you should customize the scope with
my new setting.
But if you have two connections to different clusters, it will be really
helpful i think -- for clusterIds `foo` and `bar` you'd then see `foo-23fd322`
and `bar-abcd123`.
The only problem i see with this change is im not sure if it's a
compatibility issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
