From 3fab88540f079591e5dae1c9184f8b26bb843427 Mon Sep 17 00:00:00 2001
From: Junping Du <junping_du@apache.org>
Date: Mon, 22 Feb 2016 07:04:19 -0800
Subject: [PATCH] YARN-4386. refreshNodesGracefully() should send recommission
 event to active RMNodes only. Contributed by Kuhu Shukla.

---
 hadoop-yarn-project/CHANGES.txt               |  3 ++
 .../resourcemanager/NodesListManager.java     |  3 +-
 .../TestResourceTrackerService.java           | 35 +++++++++++++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index e39917f76dd..9b57666c3f8 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -1433,6 +1433,9 @@ Release 2.8.0 - UNRELEASED
     YARN-4654. Yarn node label CLI should parse "=" correctly when trying to 
     remove all labels on a node. (Naganarasimha G R via rohithsharmaks)
 
+    YARN-4386. refreshNodesGracefully() should send recommission event to active
+    RMNodes only. (Kuhu Shukla via junping_du)
+
 Release 2.7.3 - UNRELEASED
 
   INCOMPATIBLE CHANGES
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java
index e6251fe590e..4cdf67ac6aa 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java
@@ -400,8 +400,7 @@ public class NodesListManager extends CompositeService implements
             new RMNodeEvent(nodeId, RMNodeEventType.GRACEFUL_DECOMMISSION));
       } else {
         // Recommissioning the nodes
-        if (entry.getValue().getState() == NodeState.DECOMMISSIONING
-            || entry.getValue().getState() == NodeState.DECOMMISSIONED) {
+        if (entry.getValue().getState() == NodeState.DECOMMISSIONING) {
           this.rmContext.getDispatcher().getEventHandler()
               .handle(new RMNodeEvent(nodeId, RMNodeEventType.RECOMMISSION));
         }
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java
index e0fd9ab186a..9ed79a31c23 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java
@@ -1269,6 +1269,41 @@ public class TestResourceTrackerService extends NodeLabelTestBase {
     rm1.stop();
   }
 
+  @Test
+  public void testIncorrectRecommission() throws Exception {
+    //Check decommissioned node not get recommissioned with graceful refresh
+    Configuration conf = new Configuration();
+    rm = new MockRM(conf);
+    rm.start();
+    MockNM nm1 = rm.registerNode("host1:1234", 5120);
+    MockNM nm2 = rm.registerNode("host2:5678", 10240);
+    nm1.nodeHeartbeat(true);
+    nm2.nodeHeartbeat(true);
+    File excludeHostFile =
+        new File(TEMP_DIR + File.separator + "excludeHostFile.txt");
+    writeToHostsFile(excludeHostFile, "host3", "host2");
+    conf.set(YarnConfiguration.RM_NODES_EXCLUDE_FILE_PATH,
+        excludeHostFile.getAbsolutePath());
+    writeToHostsFile(hostFile, "host1", "host2");
+    writeToHostsFile(excludeHostFile, "host1");
+    rm.getNodesListManager().refreshNodesGracefully(conf);
+    rm.drainEvents();
+    nm1.nodeHeartbeat(true);
+    rm.drainEvents();
+    Assert.assertTrue("Node " + nm1.getNodeId().getHost() +
+        " should be Decommissioned", rm.getRMContext()
+        .getInactiveRMNodes().get(nm1.getNodeId()).getState() == NodeState
+        .DECOMMISSIONED);
+    writeToHostsFile(excludeHostFile, "");
+    rm.getNodesListManager().refreshNodesGracefully(conf);
+    rm.drainEvents();
+    Assert.assertTrue("Node " + nm1.getNodeId().getHost() +
+        " should be Decommissioned", rm.getRMContext()
+        .getInactiveRMNodes().get(nm1.getNodeId()).getState() == NodeState
+        .DECOMMISSIONED);
+    rm.stop();
+  }
+
   private void writeToHostsFile(String... hosts) throws IOException {
    writeToHostsFile(hostFile, hosts);
   }