svn commit: r1291078 - in /hadoop/common/branches/branch-1.0: CHANGES.txt src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java

[mattf]

Author: mattf
Date: Sun Feb 19 22:33:34 2012
New Revision: 1291078

URL: http://svn.apache.org/viewvc?rev=1291078&view=rev
Log:
HADOOP-8050. Deadlock in metrics. Contributed by Kihwal Lee.

Modified:
    hadoop/common/branches/branch-1.0/CHANGES.txt
    
hadoop/common/branches/branch-1.0/src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java

Modified: hadoop/common/branches/branch-1.0/CHANGES.txt
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-1.0/CHANGES.txt?rev=1291078&r1=1291077&r2=1291078&view=diff
==============================================================================
--- hadoop/common/branches/branch-1.0/CHANGES.txt (original)
+++ hadoop/common/branches/branch-1.0/CHANGES.txt Sun Feb 19 22:33:34 2012
@@ -1,6 +1,6 @@
 Hadoop Change Log
 
-Release 1.0.1 - 2012.02.12
+Release 1.0.1 - 2012.02.19
 
   NEW FEATURES
 
@@ -49,6 +49,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
 

Modified: 
hadoop/common/branches/branch-1.0/src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java
URL: 
http://svn.apache.org/viewvc/hadoop/common/branches/branch-1.0/src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java?rev=1291078&r1=1291077&r2=1291078&view=diff
==============================================================================
--- 
hadoop/common/branches/branch-1.0/src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java
 (original)
+++ 
hadoop/common/branches/branch-1.0/src/core/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java
 Sun Feb 19 22:33:34 2012
@@ -92,17 +92,19 @@ class MetricsSourceAdapter implements Dy
   }
 
   @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 Dy
   }
 
   @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 Dy
   }
 
   @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) {