From 0fd97f9c1989a793b882e6678285607472a3f75a Mon Sep 17 00:00:00 2001 From: Vinod Kumar Vavilapalli Date: Tue, 11 Nov 2014 12:33:10 -0800 Subject: [PATCH] YARN-2843. Fixed NodeLabelsManager to trim inputs for hosts and labels so as to make them work correctly. Contributed by Wangda Tan. --- hadoop-yarn-project/CHANGES.txt | 3 +++ .../hadoop/yarn/client/cli/RMAdminCLI.java | 14 ++--------- .../nodelabels/CommonNodeLabelsManager.java | 20 +++++++++++++++- .../hadoop/yarn/util/ConverterUtils.java | 2 +- .../yarn/nodelabels/NodeLabelTestBase.java | 12 ++++------ .../TestCommonNodeLabelsManager.java | 23 +++++++++++++++++++ 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index c6a063c8180..3dc6d9fa37c 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -77,6 +77,9 @@ Release 2.7.0 - UNRELEASED YARN-2841. RMProxy should retry EOFException. (Jian He via xgong) + YARN-2843. Fixed NodeLabelsManager to trim inputs for hosts and labels so + as to make them work correctly. (Wangda Tan via vinodkv) + Release 2.6.0 - 2014-11-15 INCOMPATIBLE CHANGES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java index 65978c75cb3..89d87cfc4e2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java @@ -57,6 +57,7 @@ import org.apache.hadoop.yarn.server.api.protocolrecords.RefreshUserToGroupsMappingsRequest; import org.apache.hadoop.yarn.server.api.protocolrecords.RemoveFromClusterNodeLabelsRequest; import org.apache.hadoop.yarn.server.api.protocolrecords.ReplaceLabelsOnNodeRequest; +import org.apache.hadoop.yarn.util.ConverterUtils; import com.google.common.collect.ImmutableMap; @@ -393,18 +394,7 @@ private Map> buildNodeLabelsFromStr(String args) throw new IOException("node name cannot be empty"); } - String nodeName; - int port; - if (nodeIdStr.contains(":")) { - nodeName = nodeIdStr.substring(0, nodeIdStr.indexOf(":")); - port = Integer.valueOf(nodeIdStr.substring(nodeIdStr.indexOf(":") + 1)); - } else { - nodeName = nodeIdStr; - port = 0; - } - - NodeId nodeId = NodeId.newInstance(nodeName, port); - + NodeId nodeId = ConverterUtils.toNodeIdWithDefaultPort(nodeIdStr); map.put(nodeId, new HashSet()); for (int i = 1; i < splits.length; i++) { 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 1d862119e85..daefe8d33f0 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 @@ -344,6 +344,7 @@ protected void internalAddLabelsToNode( */ public void addLabelsToNode(Map> addedLabelsToNode) throws IOException { + addedLabelsToNode = normalizeNodeIdToLabels(addedLabelsToNode); checkAddLabelsToNode(addedLabelsToNode); internalAddLabelsToNode(addedLabelsToNode); } @@ -409,6 +410,8 @@ protected void internalRemoveFromClusterNodeLabels(Collection labelsToRe */ public void removeFromClusterNodeLabels(Collection labelsToRemove) throws IOException { + labelsToRemove = normalizeLabels(labelsToRemove); + checkRemoveFromClusterNodeLabels(labelsToRemove); internalRemoveFromClusterNodeLabels(labelsToRemove); @@ -518,6 +521,8 @@ protected void internalRemoveLabelsFromNode( public void removeLabelsFromNode(Map> removeLabelsFromNode) throws IOException { + removeLabelsFromNode = normalizeNodeIdToLabels(removeLabelsFromNode); + checkRemoveLabelsFromNode(removeLabelsFromNode); internalRemoveLabelsFromNode(removeLabelsFromNode); @@ -590,6 +595,8 @@ protected void internalReplaceLabelsOnNode( */ public void replaceLabelsOnNode(Map> replaceLabelsToNode) throws IOException { + replaceLabelsToNode = normalizeNodeIdToLabels(replaceLabelsToNode); + checkReplaceLabelsOnNode(replaceLabelsToNode); internalReplaceLabelsOnNode(replaceLabelsToNode); @@ -665,7 +672,7 @@ protected String normalizeLabel(String label) { return NO_LABEL; } - private Set normalizeLabels(Set labels) { + private Set normalizeLabels(Collection labels) { Set newLabels = new HashSet(); for (String label : labels) { newLabels.add(normalizeLabel(label)); @@ -732,4 +739,15 @@ protected void createHostIfNonExisted(String hostName) { nodeCollections.put(hostName, host); } } + + protected Map> normalizeNodeIdToLabels( + Map> nodeIdToLabels) { + Map> newMap = new HashMap>(); + for (Entry> entry : nodeIdToLabels.entrySet()) { + NodeId id = entry.getKey(); + Set labels = entry.getValue(); + newMap.put(id, normalizeLabels(labels)); + } + return newMap; + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java index 012d7999f9d..73ec90607e2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java @@ -167,7 +167,7 @@ public static NodeId toNodeId(String nodeIdStr) { } try { NodeId nodeId = - NodeId.newInstance(parts[0], Integer.parseInt(parts[1])); + NodeId.newInstance(parts[0].trim(), Integer.parseInt(parts[1])); return nodeId; } catch (NumberFormatException e) { throw new IllegalArgumentException("Invalid port: " + parts[1], e); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java index 9749299c375..ff0e1019b74 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java @@ -19,7 +19,7 @@ package org.apache.hadoop.yarn.nodelabels; import java.util.Collection; -import java.util.Iterator; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -49,12 +49,10 @@ public static void assertMapContains(Map> m1, public static void assertCollectionEquals(Collection c1, Collection c2) { - Assert.assertEquals(c1.size(), c2.size()); - Iterator i1 = c1.iterator(); - Iterator i2 = c2.iterator(); - while (i1.hasNext()) { - Assert.assertEquals(i1.next(), i2.next()); - } + Set s1 = new HashSet(c1); + Set s2 = new HashSet(c2); + Assert.assertEquals(s1, s2); + Assert.assertTrue(s1.containsAll(s2)); } public static Set toSet(E... elements) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java index ea29f3acc58..a56a5955770 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java @@ -258,4 +258,27 @@ public void testRemovelabelWithNodes() throws Exception { Assert.assertTrue(mgr.getClusterNodeLabels().isEmpty()); assertCollectionEquals(mgr.lastRemovedlabels, Arrays.asList("p2", "p3")); } + + @Test(timeout = 5000) + public void testTrimLabelsWhenAddRemoveNodeLabels() throws IOException { + mgr.addToCluserNodeLabels(toSet(" p1")); + assertCollectionEquals(mgr.getClusterNodeLabels(), toSet("p1")); + mgr.removeFromClusterNodeLabels(toSet("p1 ")); + Assert.assertTrue(mgr.getClusterNodeLabels().isEmpty()); + } + + @Test(timeout = 5000) + public void testTrimLabelsWhenModifyLabelsOnNodes() throws IOException { + mgr.addToCluserNodeLabels(toSet(" p1", "p2")); + mgr.addLabelsToNode(ImmutableMap.of(toNodeId("n1"), toSet("p1 ", "p2"))); + assertMapEquals( + mgr.getNodeLabels(), + ImmutableMap.of(toNodeId("n1"), toSet("p1", "p2"))); + mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet(" p2"))); + assertMapEquals( + mgr.getNodeLabels(), + ImmutableMap.of(toNodeId("n1"), toSet("p2"))); + mgr.removeLabelsFromNode(ImmutableMap.of(toNodeId("n1"), toSet(" p2 "))); + Assert.assertTrue(mgr.getNodeLabels().isEmpty()); + } } \ No newline at end of file