Ethanlm commented on a change in pull request #3371:
URL: https://github.com/apache/storm/pull/3371#discussion_r556932098
##########
File path: storm-client/src/jvm/org/apache/storm/metrics2/TaskMetricRepo.java
##########
@@ -34,23 +34,33 @@
private SortedMap<String, Timer> timers = new TreeMap<>();
public void addCounter(String name, Counter counter) {
- counters.put(name, counter);
+ synchronized (this) {
+ counters.put(name, counter);
+ }
}
public void addGauge(String name, Gauge gauge) {
- gauges.put(name, gauge);
+ synchronized (this) {
+ gauges.put(name, gauge);
+ }
}
public void addMeter(String name, Meter meter) {
- meters.put(name, meter);
+ synchronized (this) {
Review comment:
In the reporter method, a list of new SortedMap is created if filter is
not null. Maybe we can create a dummy filter that returns true and use it when
the filter parameter is null. In this way, `report` method will always report
sortedMap.
And we use ConcurrentHashMap outside to reduce performance impact from
synchronization.
----------------------------------------------------------------
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:
us...@infra.apache.org
