Repository: hadoop Updated Branches: refs/heads/branch-2.8 01a3f7899 -> 3c2bd19fa
YARN-5190. Registering/unregistering container metrics in ContainerMonitorImpl and ContainerImpl causing uncaught exception in ContainerMonitorImpl. Contributed by Junping Du (cherry picked from commit 99cc439e29794f8e61bebe03b2a7ca4b6743ec92) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/3c2bd19f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/3c2bd19f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/3c2bd19f Branch: refs/heads/branch-2.8 Commit: 3c2bd19fa5e4eebd11f12c0981f222e60109e71a Parents: 01a3f78 Author: Jian He <jia...@apache.org> Authored: Fri Jun 3 11:10:42 2016 -0700 Committer: Jian He <jia...@apache.org> Committed: Fri Jun 3 11:11:49 2016 -0700 ---------------------------------------------------------------------- .../hadoop/metrics2/impl/MetricsSystemImpl.java | 1 + .../hadoop/metrics2/lib/DefaultMetricsSystem.java | 9 +++++++++ .../monitor/ContainerMetrics.java | 18 +++++++++++++----- .../monitor/ContainersMonitorImpl.java | 16 ++++++++++++---- .../monitor/TestContainerMetrics.java | 4 +++- 5 files changed, 38 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/3c2bd19f/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java ---------------------------------------------------------------------- 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 866c846..3ce2a32 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 @@ -255,6 +255,7 @@ public class MetricsSystemImpl extends MetricsSystem implements MetricsSource { if (namedCallbacks.containsKey(name)) { namedCallbacks.remove(name); } + DefaultMetricsSystem.removeSourceName(name); } synchronized http://git-wip-us.apache.org/repos/asf/hadoop/blob/3c2bd19f/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystem.java ---------------------------------------------------------------------- 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 d3a5508..3c4aa78 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 @@ -108,6 +108,11 @@ public enum DefaultMetricsSystem { } @InterfaceAudience.Private + public static void removeSourceName(String name) { + INSTANCE.removeSource(name); + } + + @InterfaceAudience.Private public static String sourceName(String name, boolean dupOK) { return INSTANCE.newSourceName(name, dupOK); } @@ -127,6 +132,10 @@ public enum DefaultMetricsSystem { mBeanNames.map.remove(name); } + synchronized void removeSource(String name) { + sourceNames.map.remove(name); + } + synchronized String newSourceName(String name, boolean dupOK) { if (sourceNames.map.containsKey(name)) { if (dupOK) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/3c2bd19f/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java index 48128c1..706d4f9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java @@ -160,6 +160,12 @@ public class ContainerMetrics implements MetricsSource { DefaultMetricsSystem.instance(), containerId, flushPeriodMs, delayMs); } + public synchronized static ContainerMetrics getContainerMetrics( + ContainerId containerId) { + // could be null + return usageMetrics.get(containerId); + } + synchronized static ContainerMetrics forContainer( MetricsSystem ms, ContainerId containerId, long flushPeriodMs, long delayMs) { @@ -205,12 +211,14 @@ public class ContainerMetrics implements MetricsSource { } public synchronized void finished() { - this.finished = true; - if (timer != null) { - timer.cancel(); - timer = null; + if (!finished) { + this.finished = true; + if (timer != null) { + timer.cancel(); + timer = null; + } + scheduleTimerTaskForUnregistration(); } - scheduleTimerTaskForUnregistration(); } public void recordMemoryUsage(int memoryMBs) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/3c2bd19f/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java index 446e7a1..8c907c7 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java @@ -615,15 +615,16 @@ public class ContainersMonitorImpl extends AbstractService implements } ContainerId containerId = monitoringEvent.getContainerId(); - ContainerMetrics usageMetrics = ContainerMetrics - .forContainer(containerId, containerMetricsPeriodMs, - containerMetricsUnregisterDelayMs); + ContainerMetrics usageMetrics; int vmemLimitMBs; int pmemLimitMBs; int cpuVcores; switch (monitoringEvent.getType()) { case START_MONITORING_CONTAINER: + usageMetrics = ContainerMetrics + .forContainer(containerId, containerMetricsPeriodMs, + containerMetricsUnregisterDelayMs); ContainerStartMonitoringEvent startEvent = (ContainerStartMonitoringEvent) monitoringEvent; usageMetrics.recordStateChangeDurations( @@ -636,9 +637,16 @@ public class ContainersMonitorImpl extends AbstractService implements vmemLimitMBs, pmemLimitMBs, cpuVcores); break; case STOP_MONITORING_CONTAINER: - usageMetrics.finished(); + usageMetrics = ContainerMetrics.getContainerMetrics( + containerId); + if (usageMetrics != null) { + usageMetrics.finished(); + } break; case CHANGE_MONITORING_CONTAINER_RESOURCE: + usageMetrics = ContainerMetrics + .forContainer(containerId, containerMetricsPeriodMs, + containerMetricsUnregisterDelayMs); ChangeMonitoringContainerResourceEvent changeEvent = (ChangeMonitoringContainerResourceEvent) monitoringEvent; Resource resource = changeEvent.getResource(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/3c2bd19f/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainerMetrics.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainerMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainerMetrics.java index 2beb927..040647e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainerMetrics.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainerMetrics.java @@ -146,7 +146,6 @@ public class TestContainerMetrics { system.sampleMetrics(); system.sampleMetrics(); Thread.sleep(100); - system.stop(); // verify metrics1 is unregistered assertTrue(metrics1 != ContainerMetrics.forContainer( system, containerId1, 1, 0)); @@ -156,6 +155,9 @@ public class TestContainerMetrics { // verify metrics3 is still registered assertTrue(metrics3 == ContainerMetrics.forContainer( system, containerId3, 1, 0)); + // YARN-5190: move stop() to the end to verify registering containerId1 and + // containerId2 won't get MetricsException thrown. + system.stop(); system.shutdown(); } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
