From 2dc43a20549dbfe788e8284c93fc26a60acfdd8a Mon Sep 17 00:00:00 2001
From: Jason Lowe <jlowe@apache.org>
Date: Tue, 9 Aug 2016 22:18:24 +0000
Subject: [PATCH] YARN-5262. Optimize sending
 RMNodeFinishedContainersPulledByAMEvent for every AM heartbeat. Contributed
 by Rohith Sharma K S (cherry picked from commit
 26b5e6116f392b6be91dca57968259f87554ce33)

---
 hadoop-yarn-project/CHANGES.txt                          | 3 +++
 .../resourcemanager/rmapp/attempt/RMAppAttemptImpl.java  | 9 ++++++---
 .../rmapp/attempt/TestRMAppAttemptTransitions.java       | 7 +++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index 64d9401b786..26daf0736f1 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -13,6 +13,9 @@ Release 2.6.5 - UNRELEASED
     YARN-4690. Skip object allocation in FSAppAttempt#getResourceUsage when
     possible (Ming Ma via sjlee)
 
+    YARN-5262. Optimize sending RMNodeFinishedContainersPulledByAMEvent for
+    every AM heartbeat (Rohith Sharma K S via jlowe)
+
   BUG FIXES
 
     YARN-2046. Out of band heartbeats are sent only on container kill and
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
index a19f5baa10c..c76a1e4921f 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
@@ -712,9 +712,11 @@ public List<ContainerStatus> pullJustFinishedContainers() {
           }
         }
 
-        finishedContainersSentToAM.putIfAbsent(nodeId, new ArrayList
-              <ContainerStatus>());
-        finishedContainersSentToAM.get(nodeId).addAll(finishedContainers);
+        if (!finishedContainers.isEmpty()) {
+          finishedContainersSentToAM.putIfAbsent(nodeId,
+              new ArrayList<ContainerStatus>());
+          finishedContainersSentToAM.get(nodeId).addAll(finishedContainers);
+        }
       }
 
       return returnList;
@@ -1629,6 +1631,7 @@ private void sendFinishedContainersToNM() {
       eventHandler.handle(new RMNodeFinishedContainersPulledByAMEvent(nodeId,
         containerIdList));
     }
+    this.finishedContainersSentToAM.clear();
   }
 
   // Add am container to the list so that am container instance will be
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
index ccd26137e40..97e88879882 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
@@ -1317,6 +1317,13 @@ public void testFinishedContainer() {
     Assert.assertTrue(applicationAttempt.getJustFinishedContainers().isEmpty());
     Assert.assertEquals(0, getFinishedContainersSentToAM(applicationAttempt)
         .size());
+
+    // verify if no containers to acknowledge to NM then event should not be
+    // triggered. Number of times event invoked is 1 i.e on second pull
+    containerStatuses = applicationAttempt.pullJustFinishedContainers();
+    Assert.assertEquals(0, containerStatuses.size());
+    Mockito.verify(rmnodeEventHandler, times(1))
+        .handle(Mockito.any(RMNodeEvent.class));
   }
 
   private static List<ContainerStatus> getFinishedContainersSentToAM(