HADOOP-11361. Fix a race condition in MetricsSourceAdapter.updateJmxCache. Contributed by Vinayakumar B, Yongjun Zhang, and Brahma Reddy Battula. (ozawa)

(cherry picked from commit 77ffe76212)
This commit is contained in:
Tsuyoshi Ozawa 2016-07-13 21:28:04 +09:00
parent 5b4e916b3c
commit 22e719b16e
1 changed files with 14 additions and 19 deletions

View File

@ -31,6 +31,7 @@ import javax.management.ReflectionException;
import static com.google.common.base.Preconditions.*;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@ -59,7 +60,6 @@ class MetricsSourceAdapter implements DynamicMBean {
private final MBeanInfoBuilder infoBuilder;
private final Iterable<MetricsTag> injectedTags;
private Iterable<MetricsRecordImpl> lastRecs;
private boolean lastRecsCleared;
private long jmxCacheTS = 0;
private long jmxCacheTTL;
@ -175,18 +175,19 @@ class MetricsSourceAdapter implements DynamicMBean {
}
}
// HADOOP-11361: Release lock here for avoid deadlock between
// MetricsSystemImpl's lock and MetricsSourceAdapter's lock.
Iterable<MetricsRecordImpl> lastRecs = null;
if (getAllMetrics) {
MetricsCollectorImpl builder = new MetricsCollectorImpl();
getMetrics(builder, true);
lastRecs = getMetrics(new MetricsCollectorImpl(), true);
}
synchronized(this) {
updateAttrCache();
if (getAllMetrics) {
updateInfoCache();
synchronized (this) {
if (lastRecs != null) {
updateAttrCache(lastRecs);
updateInfoCache(lastRecs);
}
jmxCacheTS = Time.now();
lastRecs = null; // in case regular interval update is not running
lastRecsCleared = true;
}
}
@ -194,11 +195,6 @@ class MetricsSourceAdapter implements DynamicMBean {
Iterable<MetricsRecordImpl> getMetrics(MetricsCollectorImpl builder,
boolean all) {
builder.setRecordFilter(recordFilter).setMetricFilter(metricFilter);
synchronized(this) {
if (lastRecs == null && jmxCacheTS == 0) {
all = true; // Get all the metrics to populate the sink caches
}
}
try {
source.getMetrics(builder, all);
}
@ -210,10 +206,7 @@ class MetricsSourceAdapter implements DynamicMBean {
rb.add(t);
}
}
synchronized(this) {
lastRecs = builder.getRecords();
return lastRecs;
}
return builder.getRecords();
}
synchronized void stop() {
@ -247,13 +240,15 @@ class MetricsSourceAdapter implements DynamicMBean {
return jmxCacheTTL;
}
private void updateInfoCache() {
private void updateInfoCache(Iterable<MetricsRecordImpl> lastRecs) {
Preconditions.checkNotNull(lastRecs, "LastRecs should not be null");
LOG.debug("Updating info cache...");
infoCache = infoBuilder.reset(lastRecs).get();
LOG.debug("Done");
}
private int updateAttrCache() {
private int updateAttrCache(Iterable<MetricsRecordImpl> lastRecs) {
Preconditions.checkNotNull(lastRecs, "LastRecs should not be null");
LOG.debug("Updating attr cache...");
int recNo = 0;
int numMetrics = 0;