HDDS-6168: Guard S3G metrics JMX registration#9771
HDDS-6168: Guard S3G metrics JMX registration#9771vyalamar wants to merge 1 commit intoapache:masterfrom
Conversation
|
@adoroszlai, @ivanzlenko, and @Russole pls help review. |
| MetricsSystem ms = DefaultMetricsSystem.instance(); | ||
| instance = ms.register(SOURCE_NAME, "S3 Gateway Metrics", | ||
| new S3GatewayMetrics(conf)); | ||
| Runtime.getRuntime().addShutdownHook(new Thread(S3GatewayMetrics::unRegister)); |
There was a problem hiding this comment.
is there a wait loop that lets threads spawned as shutdown hooks to complete?
There was a problem hiding this comment.
What is the point of this Shutdown hook anyway? We will unregister metrics on server stop.
There was a problem hiding this comment.
If you decide that a shutdown hook is needed, please use org.apache.hadoop.ozone.util.ShutdownHookManager.
| MetricsSystem ms = DefaultMetricsSystem.instance(); | ||
| instance = ms.register(SOURCE_NAME, "S3 Gateway Metrics", | ||
| new S3GatewayMetrics(conf)); | ||
| Runtime.getRuntime().addShutdownHook(new Thread(S3GatewayMetrics::unRegister)); |
There was a problem hiding this comment.
What is the point of this Shutdown hook anyway? We will unregister metrics on server stop.
| } | ||
|
|
||
| @Test | ||
| public void testMetricsCreateIsIdempotent() { |
There was a problem hiding this comment.
Would've be lovely to have tests which verify the configuration as well.
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @vyalamar for the patch.
| S3GatewayMetrics m1 = S3GatewayMetrics.create(new OzoneConfiguration()); | ||
| S3GatewayMetrics m2 = S3GatewayMetrics.create(new OzoneConfiguration()); |
There was a problem hiding this comment.
Error: hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:[122,55] cannot find symbol
Error: symbol: class OzoneConfiguration
Error: location: class org.apache.hadoop.ozone.s3.metrics.TestS3GatewayMetrics
https://github.com/vyalamar/ozone/actions/runs/22034226313/job/63664346554#step:16:22
| if (!conf.getBoolean(S3GatewayConfigKeys.OZONE_S3G_METRICS_ENABLED, | ||
| S3GatewayConfigKeys.OZONE_S3G_METRICS_ENABLED_DEFAULT)) { |
There was a problem hiding this comment.
Let's cache the result in a static Boolean variable.
if (metricsEnabled == null) {
metricsEnabled = conf.getBoolean(...);
}
if (!metricsEnabled) {
return null;
}
Summary
Testing