Index: hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java =================================================================== --- hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java (revision 1463667) +++ hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java (working copy) @@ -25,10 +25,13 @@ import org.apache.hadoop.metrics2.MetricsRecordBuilder; import java.util.TreeSet; +import java.util.concurrent.locks.ReentrantReadWriteLock; public class MetricsRegionAggregateSourceImpl extends BaseSourceImpl implements MetricsRegionAggregateSource { private final Log LOG = LogFactory.getLog(this.getClass()); + // lock to guard against concurrent access to regionSources + final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final TreeSet regionSources = new TreeSet(); @@ -47,12 +50,22 @@ @Override public void register(MetricsRegionSource source) { - regionSources.add((MetricsRegionSourceImpl) source); + lock.writeLock().lock(); + try { + regionSources.add((MetricsRegionSourceImpl) source); + } finally { + lock.writeLock().unlock(); + } } @Override public void deregister(MetricsRegionSource source) { - regionSources.remove(source); + lock.writeLock().lock(); + try { + regionSources.remove(source); + } finally { + lock.writeLock().unlock(); + } } /** @@ -66,17 +79,20 @@ @Override public void getMetrics(MetricsBuilder metricsBuilder, boolean all) { - MetricsRecordBuilder mrb = metricsBuilder.addRecord(metricsName) .setContext(metricsContext); if (regionSources != null) { - for (MetricsRegionSourceImpl regionMetricSource : regionSources) { - regionMetricSource.snapshot(mrb, all); + lock.readLock().lock(); + try { + for (MetricsRegionSourceImpl regionMetricSource : regionSources) { + regionMetricSource.snapshot(mrb, all); + } + } finally { + lock.readLock().unlock(); } } - metricsRegistry.snapshot(mrb, all); } } Index: hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java =================================================================== --- hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java (revision 1463667) +++ hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java (working copy) @@ -25,11 +25,14 @@ import org.apache.hadoop.metrics2.MetricsRecordBuilder; import java.util.TreeSet; +import java.util.concurrent.locks.ReentrantReadWriteLock; public class MetricsRegionAggregateSourceImpl extends BaseSourceImpl implements MetricsRegionAggregateSource { private final Log LOG = LogFactory.getLog(this.getClass()); + // lock to guard against concurrent access to regionSources + final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final TreeSet regionSources = new TreeSet(); @@ -48,12 +51,22 @@ @Override public void register(MetricsRegionSource source) { - regionSources.add((MetricsRegionSourceImpl) source); + lock.writeLock().lock(); + try { + regionSources.add((MetricsRegionSourceImpl) source); + } finally { + lock.writeLock().unlock(); + } } @Override public void deregister(MetricsRegionSource source) { - regionSources.remove(source); + lock.writeLock().lock(); + try { + regionSources.remove(source); + } finally { + lock.writeLock().unlock(); + } } /** @@ -72,8 +85,13 @@ .setContext(metricsContext); if (regionSources != null) { - for (MetricsRegionSourceImpl regionMetricSource : regionSources) { - regionMetricSource.snapshot(mrb, all); + lock.readLock().lock(); + try { + for (MetricsRegionSourceImpl regionMetricSource : regionSources) { + regionMetricSource.snapshot(mrb, all); + } + } finally { + lock.readLock().unlock(); } }