Ethanlm commented on a change in pull request #3255:
URL: https://github.com/apache/storm/pull/3255#discussion_r412218182
##########
File path:
storm-client/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java
##########
@@ -52,16 +64,19 @@ public JcMetrics jcMetrics(String name, String topologyId,
String componentId, I
public Meter meter(String name, WorkerTopologyContext context, String
componentId, Integer taskId, String streamId) {
String metricName = metricName(name, context.getStormId(),
componentId, streamId, taskId, context.getThisWorkerPort());
+ LOG.warn("Adding meter: " + metricName);
return registry.meter(metricName);
}
public Counter counter(String name, WorkerTopologyContext context, String
componentId, Integer taskId, String streamId) {
String metricName = metricName(name, context.getStormId(),
componentId, streamId, taskId, context.getThisWorkerPort());
+ LOG.warn("Adding counter: " + metricName);
return registry.counter(metricName);
Review comment:
Since the registry is shared among tasks inside the same worker, all the
metrics should have different name otherwise the same metric object will be
returned:
https://github.com/dropwizard/metrics/blob/4.1-development/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java#L172-L181
So if `registery.counter("a-counter", xxxx)` is called more than once,
starting from the second time, it returns the same metric object.
Which means all tasks will have the same metric objects in the worker. For
example, all tasks have `acked`, `failed`, `emitted`, `transferred` metrics
https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/metrics2/TaskMetrics.java#L21-L24
With this code change, all the tasks will have the same metric values inside
the same worker.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
