HADOOP-8050. Deadlock in metrics. Contributed by Kihwal Lee.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1@1291080 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -152,7 +152,7 @@ Release 1.1.0 - unreleased
     HDFS-2741. Document the max transfer threads property for branch-1. Backport of HDFS-1866. (Markus Jelsma via harsh)
 
 
-Release 1.0.1 - 2012.02.12
+Release 1.0.1 - 2012.02.19
 
   NEW FEATURES
 
@@ -198,6 +198,8 @@ Release 1.0.1 - 2012.02.12
     HADOOP-8037. Binary tarball does not preserve platform info for native builds,
     and RPMs fail to provide needed symlinks for libhadoop.so.  (Matt Foley)
 
+    HADOOP-8050. Deadlock in metrics. (Kihwal Lee via mattf)
+
 
 Release 1.0.0 - 2011.12.15
 

src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java

@@ -92,17 +92,19 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized Object getAttribute(String attribute)
+  public Object getAttribute(String attribute)
       throws AttributeNotFoundException, MBeanException, ReflectionException {
     updateJmxCache();
-    Attribute a = attrCache.get(attribute);
-    if (a == null) {
-      throw new AttributeNotFoundException(attribute +" not found");
-    }
-    if (LOG.isDebugEnabled()) {
-      LOG.debug(attribute +": "+ a.getName() +"="+ a.getValue());
+    synchronized(this) {
+      Attribute a = attrCache.get(attribute);
+      if (a == null) {
+        throw new AttributeNotFoundException(attribute +" not found");
+      }
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(attribute +": "+ a.getName() +"="+ a.getValue());
+      }
+      return a.getValue();
     }
-    return a.getValue();
   }
 
   public void setAttribute(Attribute attribute)
@@ -112,17 +114,19 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized AttributeList getAttributes(String[] attributes) {
+  public AttributeList getAttributes(String[] attributes) {
     updateJmxCache();
-    AttributeList ret = new AttributeList();
-    for (String key : attributes) {
-      Attribute attr = attrCache.get(key);
-      if (LOG.isDebugEnabled()) {
-        LOG.debug(key +": "+ attr.getName() +"="+ attr.getValue());
+    synchronized(this) {
+      AttributeList ret = new AttributeList();
+      for (String key : attributes) {
+        Attribute attr = attrCache.get(key);
+        if (LOG.isDebugEnabled()) {
+          LOG.debug(key +": "+ attr.getName() +"="+ attr.getValue());
+        }
+        ret.add(attr);
       }
-      ret.add(attr);
+      return ret;
     }
-    return ret;
   }
 
   @Override
@@ -137,17 +141,32 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized MBeanInfo getMBeanInfo() {
+  public MBeanInfo getMBeanInfo() {
     updateJmxCache();
     return infoCache;
   }
 
   private void updateJmxCache() {
-    if (System.currentTimeMillis() - jmxCacheTS >= jmxCacheTTL) {
-      if (lastRecs == null) {
-        MetricsBuilderImpl builder = new MetricsBuilderImpl();
-        getMetrics(builder, true);
+    boolean getAllMetrics = false;
+    synchronized(this) {
+      if (System.currentTimeMillis() - jmxCacheTS >= jmxCacheTTL) {
+        // temporarilly advance the expiry while updating the cache
+        jmxCacheTS = System.currentTimeMillis() + jmxCacheTTL;
+        if (lastRecs == null) {
+          getAllMetrics = true;
+        }
+      }
+      else {
+        return;
       }
+    }
+
+    if (getAllMetrics) {
+      MetricsBuilderImpl builder = new MetricsBuilderImpl();
+      getMetrics(builder, true);
+    }
+
+    synchronized(this) {
       int cacheSize = attrCache.size(); // because updateAttrCache changes it!
       int numMetrics = updateAttrCache();
       if (cacheSize < numMetrics) {