This appears to have been caused by YARN-2984. Karthik Kambatla would you mind taking a look?

Zhihai pinged me offline mentioning he knows the root cause behind this. zhihai xu - assigning this to you.

thanks Karthik Kambatla for assigning this JIRA to me.
The root cause is exactly what Jason Lowe said. I just added a little more details based on Jason Lowe succinct comment.
sampleMetrics will be called periodically in MetricsSystemImpl. sampleMetrics will iterate the sources in the following code:

    for (Entry<String, MetricsSourceAdapter> entry : sources.entrySet()) {
      if (sourceFilter == null || sourceFilter.accepts(entry.getKey())) {
        snapshotMetrics(entry.getValue(), bufferBuilder);
      }
    }

snapshotMetrics will be called to process every entry from sources
The calling sequence which leads to a ConcurrentModificationException is
snapshotMetrics => MetricsSourceAdapter#getMetrics => ContainerMetrics#getMetrics => MetricsSystemImpl#unregisterSource => sources.remove(name)
the entry in the sources is removed when iterate the sources. So unregisterSource can't be called from getMetrics.

I will prepare a patch for review.

I attached a test patch which can reproduce this issue with the following stack trace:

Running org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics
Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.92 sec <<< FAILURE! - in org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics
testContainerMetricsFinished(org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics)  Time elapsed: 1.194 sec  <<< ERROR!
java.util.ConcurrentModificationException: null
	at java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:394)
	at java.util.LinkedHashMap$EntryIterator.next(LinkedHashMap.java:413)
	at java.util.LinkedHashMap$EntryIterator.next(LinkedHashMap.java:412)
	at org.apache.hadoop.metrics2.impl.MetricsSystemImpl.sampleMetrics(MetricsSystemImpl.java:403)
	at org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics.testContainerMetricsFinished(TestContainerMetrics.java:144)
Results :
Tests in error: 
  TestContainerMetrics.testContainerMetricsFinished:144 » ConcurrentModification
Tests run: 3, Failures: 0, Errors: 1, Skipped: 0

I uploaded a patch YARN-3619.000.patch for review. I added a configuration NM_CONTAINER_METRICS_UNREGISTER_DELAY_MS to configure when to unregister the container metrics after it is finished. Because it may have potential memory leak If I schedule a thread to do unregistration at getMetrics.
It looks like getMetrics will be called from two places:MetricsSystemImpl#sampleMetrics and MetricsSourceAdapter#getMBeanInfo.
sampleMetrics won't be called if no sinks in MetricsSystemImpl. getMBeanInfo may not be called after registration if JMXJsonServlet#doGet is not called(no http Get request from JMX clients). It looks like there is a possibility that getMetrics won't be called after registration.

+1 overall

This message was automatically generated.

Moving out all non-critical / non-blocker issues that didn't make it out of 2.7.1 into 2.7.2.

My apologies for the long delay, as this fell off my radar. The approach seems reasonable.

The patch needs to be upmerged to trunk. In addition I'm wondering about the Timer handling. I think the Timer should be a daemon thread (we don't want to prolong NM shutdown due to this). Also it seems wasteful to dedicate a separate timer thread for every container that finished. It would be more efficient to share a timer that handles multiple timer tasks rather than spawn a thread for every timer task.

-1 overall

This message was automatically generated.

-1 overall

This message was automatically generated.

Jason Lowe, thanks for the review! Yes, that is a very good suggestion. I uploaded a new patch YARN-3619.001.patch, which addressed all your comments(daemon thread and share a timer), please review it.

Thanks for updating the patch! In the future, please do not remove patches unless absolutely necessary as it generates a lot of email churn and can lead to confusion as in this case. The patchbot comments above are not for the most recent patch applied despite the patch basename being the same. This subsequent patch should have simply been version 002.

Latest patch looks good to me, checkstyle and release audit warnings were pre-existing. Kicking Jenkins again to comment on the latest patch.

-1 overall

This message was automatically generated.

Thanks for the good suggestions Jason Lowe! Will do it next time to avoid confusion! Thanks for the thorough review!

The checkstyle issues and release audit warnings for the latest patch YARN-3619.001.patch were pre-existing.

I just attached the previous confused patch(YARN-3619.alt.patch) which I removed for comparison. It shares both timer and timer task, which is more complicated than YARN-3619.001.patch, So I think YARN-3619.001.patch(share timer only) is a better approach, which is simpler and more accurately controls the time to unregister the container metrics.

-1 overall

This message was automatically generated.

+1 for the .001 patch. The patch doesn't apply to branch-2.7 cleanly, and I'd like to get it fixed in 2.7.2. Can you provide a patch against branch-2.7?

thanks Jason Lowe! Yes, I uploaded a patch YARN-3619.branch-2.7.patch based on branch-2.7.

-1 overall

This message was automatically generated.

+1 for the branch-2.7 patch. Committing this.

Thanks, Zhihai! I committed this to trunk, branch-2, and branch-2.7

FAILURE: Integrated in Hadoop-trunk-Commit #8559 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8559/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• 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
• hadoop-yarn-project/CHANGES.txt
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #481 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/481/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• 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
• hadoop-yarn-project/CHANGES.txt
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• 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

Thanks Jason Lowe for reviewing and committing the very old patch! I really appreciate it.

FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #473 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/473/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• 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
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• hadoop-yarn-project/CHANGES.txt
• 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
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

FAILURE: Integrated in Hadoop-Yarn-trunk #1212 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1212/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• hadoop-yarn-project/CHANGES.txt
• 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
• 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
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java

FAILURE: Integrated in Hadoop-Mapreduce-trunk #2417 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2417/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• hadoop-yarn-project/CHANGES.txt
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• 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
• 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

FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #447 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/447/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• 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
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• 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
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

FAILURE: Integrated in Hadoop-Hdfs-trunk #2387 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2387/)
YARN-3619. ContainerMetrics unregisters during getMetrics and leads to (jlowe: rev fdf02d1f26cea372bf69e071f57b8bfc09c092c4)

• 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
• 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
• 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
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
• hadoop-yarn-project/CHANGES.txt