From f5da5566d9c392a5df71a2dce4c2d0d50eea51ee Mon Sep 17 00:00:00 2001 From: Jian He Date: Wed, 18 Feb 2015 11:51:51 -0800 Subject: [PATCH] YARN-3132. RMNodeLabelsManager should remove node from node-to-label mapping when node becomes deactivated. Contributed by Wangda Tan --- hadoop-yarn-project/CHANGES.txt | 3 + .../nodelabels/CommonNodeLabelsManager.java | 3 +- .../nodelabels/RMNodeLabelsManager.java | 20 +++++- .../nodelabels/TestRMNodeLabelsManager.java | 63 +++++++++++++++++-- 4 files changed, 78 insertions(+), 11 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index cbba0460b95..884b506d86a 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/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 diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java index cb6f1f39148..e2da664a182 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java +++ b/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 labels) { + protected void removeNodeFromLabels(NodeId node, Set labels) { for(String l : labels) { labelCollections.get(l).removeNodeId(node); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/RMNodeLabelsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/RMNodeLabelsManager.java index 9942d80406f..e5abdc99f97 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/RMNodeLabelsManager.java +++ b/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,9 +228,23 @@ public class RMNodeLabelsManager extends CommonNodeLabelsManager { Map before = cloneNodeMap(ImmutableSet.of(nodeId)); Node nm = getNMInNodeSet(nodeId); if (null != nm) { - // set nm is not running, and its resource = 0 - nm.running = false; - nm.resource = Resource.newInstance(0, 0); + 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 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 diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java index a91947fa58b..8a37c2492de 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java +++ b/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 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.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"), - toNodeId("n2"), toSet("p2"), toNodeId("n3"), toSet("p3"))); + mgr.addToCluserNodeLabels(toSet("p1")); + mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"))); + // 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())); }