joshelser commented on code in PR #1431:
URL: https://github.com/apache/phoenix/pull/1431#discussion_r861966333
##########
phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricUtil.java:
##########
@@ -38,4 +46,22 @@ public static MetricsStopWatch getMetricsStopWatch(boolean
isRequestMetricsEnabl
return new MetricsStopWatch(true);
}
+ // We need to cover the case when JmxCacheBuster has just stopped the
HBase metrics
+ // system, and not accidentally overwrite the DefaultMetricsSystem
singleton.
+ // See PHOENIX-6699
+ public static boolean isDefaultMetricsInitialized() {
+ try {
+ MetricsSystemImpl metrics = (MetricsSystemImpl)
DefaultMetricsSystem.instance();
+ Field prefixField =
MetricsSystemImpl.class.getDeclaredField("prefix");
Review Comment:
`prefix` isn't `volatile` and is set from within the `synchronized`
`init(String)` method. This would mean that we may be initialized and not yet
be able to observe it from this separate thread.
That said, I don't know how likely this race condition would be to hit.
We could also check the `monitoring` boolean instead of `prefix`. This would
mean that we already have atomic access to that variable. I am assuming that
the synchronization on the methods in MetricsSystemImpl would not affect this
reflection, meaning we would still have a (small) window where `stop()` is
being called but not yet complete.
Maybe you have already thought about this :)
--
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]
