HADOOP-11361. Fix a race condition in MetricsSourceAdapter.updateJmxCache. Contributed by Vinayakumar B, Yongjun Zhang, and Brahma Reddy Battula. (ozawa)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/77ffe762 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/77ffe762 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/77ffe762 Branch: refs/heads/YARN-2915 Commit: 77ffe7621212be9f462ca37a542a13d167eca4e0 Parents: 438b7c5 Author: Tsuyoshi Ozawa <[email protected]> Authored: Wed Jul 13 21:28:04 2016 +0900 Committer: Tsuyoshi Ozawa <[email protected]> Committed: Wed Jul 13 21:28:04 2016 +0900 ---------------------------------------------------------------------- .../metrics2/impl/MetricsSourceAdapter.java | 33 +++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/77ffe762/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java index cbba014..3406ace 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java @@ -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); } catch (Exception e) { @@ -209,10 +205,7 @@ class MetricsSourceAdapter implements DynamicMBean { rb.add(t); } } - synchronized(this) { - lastRecs = builder.getRecords(); - return lastRecs; - } + return builder.getRecords(); } synchronized void stop() { @@ -246,13 +239,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; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
