From f8041b05404dce7088d73f5befc500cb2786c9bd Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Fri, 20 Jun 2014 18:24:03 +0000 Subject: [PATCH] HADOOP-9559. When metrics system is restarted MBean names get incorrectly flagged as dupes. Contributed by Mostafa Elhemali and Mike Liddell. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1604225 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 +++ .../metrics2/impl/MetricsSourceAdapter.java | 7 +++++++ .../metrics2/impl/MetricsSystemImpl.java | 6 ++++++ .../metrics2/lib/DefaultMetricsSystem.java | 9 +++++++++ .../apache/hadoop/metrics2/util/MBeans.java | 1 + .../metrics2/impl/TestMetricsSystemImpl.java | 18 ++++++++++++++++++ 6 files changed, 44 insertions(+) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index d4067966169..fe80214f79a 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -582,6 +582,9 @@ Release 2.5.0 - UNRELEASED HADOOP-10716. Cannot use more than 1 har filesystem. (Rushabh Shah via cnauroth) + HADOOP-9559. When metrics system is restarted MBean names get incorrectly + flagged as dupes. (Mostafa Elhemali and Mike Liddell via cnauroth) + BREAKDOWN OF HADOOP-10514 SUBTASKS AND RELATED JIRAS HADOOP-10520. Extended attributes definition and FileSystem APIs for 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 23c4492663b..cf11e6db14e 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 @@ -30,6 +30,7 @@ import javax.management.ObjectName; import javax.management.ReflectionException; import static com.google.common.base.Preconditions.*; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -226,7 +227,13 @@ class MetricsSourceAdapter implements DynamicMBean { mbeanName = null; } } + + @VisibleForTesting + ObjectName getMBeanName() { + return mbeanName; + } + private void updateInfoCache() { LOG.debug("Updating info cache..."); infoCache = infoBuilder.reset(lastRecs).get(); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java index e386ce608f0..cf2dda4e380 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java @@ -32,6 +32,7 @@ import javax.management.ObjectName; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.annotations.VisibleForTesting; import java.util.Locale; import static com.google.common.base.Preconditions.*; @@ -573,6 +574,11 @@ public class MetricsSystemImpl extends MetricsSystem implements MetricsSource { return allSources.get(name); } + @VisibleForTesting + MetricsSourceAdapter getSourceAdapter(String name) { + return sources.get(name); + } + private InitMode initMode() { LOG.debug("from system property: "+ System.getProperty(MS_INIT_MODE_KEY)); LOG.debug("from environment variable: "+ System.getenv(MS_INIT_MODE_KEY)); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystem.java index be785b98544..c761b583937 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystem.java @@ -110,6 +110,11 @@ public enum DefaultMetricsSystem { return INSTANCE.newObjectName(name); } + @InterfaceAudience.Private + public static void removeMBeanName(ObjectName name) { + INSTANCE.removeObjectName(name.toString()); + } + @InterfaceAudience.Private public static String sourceName(String name, boolean dupOK) { return INSTANCE.newSourceName(name, dupOK); @@ -126,6 +131,10 @@ public enum DefaultMetricsSystem { } } + synchronized void removeObjectName(String name) { + mBeanNames.map.remove(name); + } + synchronized String newSourceName(String name, boolean dupOK) { if (sourceNames.map.containsKey(name)) { if (dupOK) { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java index 8f2c01209ce..aec9dad40c2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java @@ -84,6 +84,7 @@ public class MBeans { } catch (Exception e) { LOG.warn("Error unregistering "+ mbeanName, e); } + DefaultMetricsSystem.removeMBeanName(mbeanName); } static private ObjectName getMBeanName(String serviceName, String nameName) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestMetricsSystemImpl.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestMetricsSystemImpl.java index 49a54af5998..564214bba65 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestMetricsSystemImpl.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestMetricsSystemImpl.java @@ -360,6 +360,24 @@ public class TestMetricsSystemImpl { ms.register(ts); } + @Test public void testStartStopStart() { + DefaultMetricsSystem.shutdown(); // Clear pre-existing source names. + MetricsSystemImpl ms = new MetricsSystemImpl("test"); + TestSource ts = new TestSource("ts"); + ms.start(); + ms.register("ts", "", ts); + MetricsSourceAdapter sa = ms.getSourceAdapter("ts"); + assertNotNull(sa); + assertNotNull(sa.getMBeanName()); + ms.stop(); + ms.shutdown(); + ms.start(); + sa = ms.getSourceAdapter("ts"); + assertNotNull(sa); + assertNotNull(sa.getMBeanName()); + ms.stop(); + ms.shutdown(); + } private void checkMetricsRecords(List recs) { LOG.debug(recs);