HBASE-19239 Fix findbugs and error-prone issues Fixes for hbase-metrics
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/357b3dbc Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/357b3dbc Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/357b3dbc Branch: refs/heads/branch-1 Commit: 357b3dbc3e3c289f2ac0ff6c6210cd226681dcb4 Parents: 24feb1a Author: Andrew Purtell <[email protected]> Authored: Wed Nov 15 18:47:45 2017 -0800 Committer: Andrew Purtell <[email protected]> Committed: Fri Nov 17 17:12:36 2017 -0800 ---------------------------------------------------------------------- .../hadoop/hbase/metrics/impl/HistogramImpl.java | 2 ++ .../hbase/metrics/impl/MetricRegistriesImpl.java | 5 +++++ .../hadoop/hbase/metrics/impl/RefCountingMap.java | 16 +++++++++------- 3 files changed, 16 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/357b3dbc/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java ---------------------------------------------------------------------- diff --git a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java index b52caf8..17a179d 100644 --- a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java +++ b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/HistogramImpl.java @@ -67,6 +67,7 @@ public class HistogramImpl implements Histogram { histogram.add(value, 1); } + @Override public long getCount() { return counter.getCount(); } @@ -75,6 +76,7 @@ public class HistogramImpl implements Histogram { return this.histogram.getMax(); } + @Override public Snapshot snapshot() { return histogram.snapshotAndReset(); } http://git-wip-us.apache.org/repos/asf/hbase/blob/357b3dbc/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/MetricRegistriesImpl.java ---------------------------------------------------------------------- diff --git a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/MetricRegistriesImpl.java b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/MetricRegistriesImpl.java index 3788bd1..2966429 100644 --- a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/MetricRegistriesImpl.java +++ b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/MetricRegistriesImpl.java @@ -60,22 +60,27 @@ public class MetricRegistriesImpl extends MetricRegistries { }); } + @Override public boolean remove(MetricRegistryInfo key) { return registries.remove(key) == null; } + @Override public Optional<MetricRegistry> get(MetricRegistryInfo info) { return Optional.fromNullable(registries.get(info)); } + @Override public Collection<MetricRegistry> getMetricRegistries() { return Collections.unmodifiableCollection(registries.values()); } + @Override public void clear() { registries.clear(); } + @Override public Set<MetricRegistryInfo> getMetricRegistryInfos() { return Collections.unmodifiableSet(registries.keySet()); } http://git-wip-us.apache.org/repos/asf/hbase/blob/357b3dbc/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/RefCountingMap.java ---------------------------------------------------------------------- diff --git a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/RefCountingMap.java b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/RefCountingMap.java index 889b026..25c7634 100644 --- a/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/RefCountingMap.java +++ b/hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/RefCountingMap.java @@ -24,7 +24,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.hbase.classification.InterfaceAudience; @@ -42,22 +42,24 @@ class RefCountingMap<K, V> { private ConcurrentHashMap<K, Payload<V>> map = new ConcurrentHashMap<>(); private static class Payload<V> { V v; - volatile int refCount; + final AtomicInteger refCount = new AtomicInteger(1); // create with ref count = 1 Payload(V v) { this.v = v; - this.refCount = 1; // create with ref count = 1 } } + @edu.umd.cs.findbugs.annotations.SuppressWarnings( + value="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION", + justification="We use the object monitor to serialize operations on the concurrent map") V put(K key, Supplier<V> supplier) { - synchronized (map) { + synchronized (this) { Payload<V> oldValue = map.get(key); if (oldValue == null) { oldValue = new Payload<V>(supplier.get()); map.put(key, oldValue); return oldValue.v; } - oldValue.refCount++; + oldValue.refCount.incrementAndGet(); return oldValue.v; } } @@ -73,10 +75,10 @@ class RefCountingMap<K, V> { * @return the value associated with the specified key or null if key is removed from map. */ V remove(K key) { - synchronized (map) { + synchronized (this) { Payload<V> oldValue = map.get(key); if (oldValue != null) { - if (--oldValue.refCount == 0) { + if (oldValue.refCount.decrementAndGet() == 0) { map.remove(key); return null; }
