YARN-3132. RMNodeLabelsManager should remove node from node-to-label mapping when node becomes deactivated. Contributed by Wangda Tan

hadoop-yarn-project/CHANGES.txt

@@ -611,6 +611,9 @@ Release 2.7.0 - UNRELEASED
     YARN-3207. Secondary filter matches entites which do not have the key being
     filtered for. (Zhijie Shen via xgong)
 
+    YARN-3132. RMNodeLabelsManager should remove node from node-to-label mapping
+    when node becomes deactivated. (Wangda Tan via jianhe)
+
 Release 2.6.0 - 2014-11-18
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java

@@ -45,7 +45,6 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.event.AsyncDispatcher;
 import org.apache.hadoop.yarn.event.Dispatcher;
 import org.apache.hadoop.yarn.event.EventHandler;
-import org.apache.hadoop.yarn.exceptions.YarnException;
 import org.apache.hadoop.yarn.exceptions.YarnRuntimeException;
 import org.apache.hadoop.yarn.nodelabels.event.NodeLabelsStoreEvent;
 import org.apache.hadoop.yarn.nodelabels.event.NodeLabelsStoreEventType;
@@ -496,7 +495,7 @@ public class CommonNodeLabelsManager extends AbstractService {
     }
   }
 
-  private void removeNodeFromLabels(NodeId node, Set<String> labels) {
+  protected void removeNodeFromLabels(NodeId node, Set<String> labels) {
     for(String l : labels) {
       labelCollections.get(l).removeNodeId(node);
     }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/RMNodeLabelsManager.java

@@ -228,10 +228,24 @@ public class RMNodeLabelsManager extends CommonNodeLabelsManager {
       Map<String, Host> before = cloneNodeMap(ImmutableSet.of(nodeId));
       Node nm = getNMInNodeSet(nodeId);
       if (null != nm) {
+        if (null == nm.labels) {
+          // When node deactivated, remove the nm from node collection if no
+          // labels explicitly set for this particular nm
+
+          // Save labels first, we need to remove label->nodes relation later
+          Set<String> savedNodeLabels = getLabelsOnNode(nodeId);
+          
+          // Remove this node in nodes collection
+          nodeCollections.get(nodeId.getHost()).nms.remove(nodeId);
+          
+          // Remove this node in labels->node
+          removeNodeFromLabels(nodeId, savedNodeLabels);
+        } else {
           // set nm is not running, and its resource = 0
           nm.running = false;
           nm.resource = Resource.newInstance(0, 0);
         }
+      }
       
       // get the node after edition
       Map<String, Host> after = cloneNodeMap(ImmutableSet.of(nodeId));

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java

@@ -62,7 +62,7 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
   }
   
   @Test(timeout = 5000)
-  public void testNodeActiveDeactiveUpdate() throws Exception {
+  public void testGetLabelResourceWhenNodeActiveDeactive() throws Exception {
     mgr.addToCluserNodeLabels(toSet("p1", "p2", "p3"));
     mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"),
         toNodeId("n2"), toSet("p2"), toNodeId("n3"), toSet("p3")));
@@ -119,7 +119,7 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
 
   @SuppressWarnings({ "unchecked", "rawtypes" })
   @Test(timeout = 5000)
-  public void testUpdateNodeLabelWithActiveNode() throws Exception {
+  public void testGetLabelResource() throws Exception {
     mgr.addToCluserNodeLabels(toSet("p1", "p2", "p3"));
     mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"),
         toNodeId("n2"), toSet("p2"), toNodeId("n3"), toSet("p3")));
@@ -430,6 +430,52 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
     }
   }
   
+  @Test(timeout = 5000)
+  public void testGetLabelsOnNodesWhenNodeActiveDeactive() throws Exception {
+    mgr.addToCluserNodeLabels(toSet("p1", "p2", "p3"));
+    mgr.replaceLabelsOnNode(ImmutableMap.of(
+        toNodeId("n1"), toSet("p2")));
+    mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1:1"), toSet("p1")));
+    
+    // Active/Deactive a node directly assigned label, should not remove from
+    // node->label map
+    mgr.activateNode(toNodeId("n1:1"), SMALL_RESOURCE);
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1:1")),
+        toSet("p1"));
+    mgr.deactivateNode(toNodeId("n1:1"));
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1:1")),
+        toSet("p1"));
+    // Host will not affected
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1")),
+        toSet("p2"));
+    
+    // Active/Deactive a node doesn't directly assigned label, should remove
+    // from node->label map
+    mgr.activateNode(toNodeId("n1:2"), SMALL_RESOURCE);
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1:2")),
+        toSet("p2"));
+    mgr.deactivateNode(toNodeId("n1:2"));
+    Assert.assertNull(mgr.getNodeLabels().get(toNodeId("n1:2")));
+    // Host will not affected too
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1")),
+        toSet("p2"));
+    
+    // When we change label on the host after active a node without directly
+    // assigned label, such node will still be removed after deactive
+    // Active/Deactive a node doesn't directly assigned label, should remove
+    // from node->label map
+    mgr.activateNode(toNodeId("n1:2"), SMALL_RESOURCE);
+    mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p3")));
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1:2")),
+        toSet("p3"));
+    mgr.deactivateNode(toNodeId("n1:2"));
+    Assert.assertNull(mgr.getNodeLabels().get(toNodeId("n1:2")));
+    // Host will not affected too
+    assertCollectionEquals(mgr.getNodeLabels().get(toNodeId("n1")),
+        toSet("p3"));
+    
+  }
+  
   private void checkNodeLabelInfo(List<NodeLabel> infos, String labelName, int activeNMs, int memory) {
     for (NodeLabel info : infos) {
       if (info.getLabelName().equals(labelName)) {
@@ -470,19 +516,24 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
         mgr.getLabelsToNodes(), transposeNodeToLabels(mgr.getNodeLabels()));
 
     // Add labels and replace labels on node
-    mgr.addToCluserNodeLabels(toSet("p1", "p2", "p3"));
+    mgr.addToCluserNodeLabels(toSet("p1"));
-    mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"),
+    mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1")));
-        toNodeId("n2"), toSet("p2"), toNodeId("n3"), toSet("p3")));
+    // p1 -> n1, n1:1
+    Assert.assertEquals(2, mgr.getLabelsToNodes().get("p1").size());
     assertLabelsToNodesEquals(
         mgr.getLabelsToNodes(), transposeNodeToLabels(mgr.getNodeLabels()));
 
     // Activate a node for which host to label mapping exists
     mgr.activateNode(NodeId.newInstance("n1", 2), Resource.newInstance(10, 0));
+    // p1 -> n1, n1:1, n1:2
+    Assert.assertEquals(3, mgr.getLabelsToNodes().get("p1").size());
     assertLabelsToNodesEquals(
         mgr.getLabelsToNodes(), transposeNodeToLabels(mgr.getNodeLabels()));
 
-    // Deactivate a node. Label mapping should still exist.
+    // Deactivate a node. n1:1 will be removed from the map
     mgr.deactivateNode(NodeId.newInstance("n1", 1));
+    // p1 -> n1, n1:2
+    Assert.assertEquals(2, mgr.getLabelsToNodes().get("p1").size());
     assertLabelsToNodesEquals(
         mgr.getLabelsToNodes(), transposeNodeToLabels(mgr.getNodeLabels()));
   }