Repository: hadoop Updated Branches: refs/heads/trunk 061bc293c -> 0fd97f9c1
YARN-2843. Fixed NodeLabelsManager to trim inputs for hosts and labels so as to make them work correctly. Contributed by Wangda Tan. Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/0fd97f9c Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/0fd97f9c Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/0fd97f9c Branch: refs/heads/trunk Commit: 0fd97f9c1989a793b882e6678285607472a3f75a Parents: 061bc29 Author: Vinod Kumar Vavilapalli <[email protected]> Authored: Tue Nov 11 12:33:10 2014 -0800 Committer: Vinod Kumar Vavilapalli <[email protected]> Committed: Tue Nov 11 12:33:10 2014 -0800 ---------------------------------------------------------------------- hadoop-yarn-project/CHANGES.txt | 3 +++ .../hadoop/yarn/client/cli/RMAdminCLI.java | 14 ++---------- .../nodelabels/CommonNodeLabelsManager.java | 20 ++++++++++++++++- .../apache/hadoop/yarn/util/ConverterUtils.java | 2 +- .../yarn/nodelabels/NodeLabelTestBase.java | 12 +++++----- .../nodelabels/TestCommonNodeLabelsManager.java | 23 ++++++++++++++++++++ 6 files changed, 53 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index c6a063c..3dc6d9f 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 http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java ---------------------------------------------------------------------- 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 65978c7..89d87cf 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.RefreshSuperUserGroupsC 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 @@ public class RMAdminCLI extends HAAdmin { 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<String>()); for (int i = 1; i < splits.length; i++) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java ---------------------------------------------------------------------- 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 1d86211..daefe8d 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 @@ public class CommonNodeLabelsManager extends AbstractService { */ public void addLabelsToNode(Map<NodeId, Set<String>> addedLabelsToNode) throws IOException { + addedLabelsToNode = normalizeNodeIdToLabels(addedLabelsToNode); checkAddLabelsToNode(addedLabelsToNode); internalAddLabelsToNode(addedLabelsToNode); } @@ -409,6 +410,8 @@ public class CommonNodeLabelsManager extends AbstractService { */ public void removeFromClusterNodeLabels(Collection<String> labelsToRemove) throws IOException { + labelsToRemove = normalizeLabels(labelsToRemove); + checkRemoveFromClusterNodeLabels(labelsToRemove); internalRemoveFromClusterNodeLabels(labelsToRemove); @@ -518,6 +521,8 @@ public class CommonNodeLabelsManager extends AbstractService { public void removeLabelsFromNode(Map<NodeId, Set<String>> removeLabelsFromNode) throws IOException { + removeLabelsFromNode = normalizeNodeIdToLabels(removeLabelsFromNode); + checkRemoveLabelsFromNode(removeLabelsFromNode); internalRemoveLabelsFromNode(removeLabelsFromNode); @@ -590,6 +595,8 @@ public class CommonNodeLabelsManager extends AbstractService { */ public void replaceLabelsOnNode(Map<NodeId, Set<String>> replaceLabelsToNode) throws IOException { + replaceLabelsToNode = normalizeNodeIdToLabels(replaceLabelsToNode); + checkReplaceLabelsOnNode(replaceLabelsToNode); internalReplaceLabelsOnNode(replaceLabelsToNode); @@ -665,7 +672,7 @@ public class CommonNodeLabelsManager extends AbstractService { return NO_LABEL; } - private Set<String> normalizeLabels(Set<String> labels) { + private Set<String> normalizeLabels(Collection<String> labels) { Set<String> newLabels = new HashSet<String>(); for (String label : labels) { newLabels.add(normalizeLabel(label)); @@ -732,4 +739,15 @@ public class CommonNodeLabelsManager extends AbstractService { nodeCollections.put(hostName, host); } } + + protected Map<NodeId, Set<String>> normalizeNodeIdToLabels( + Map<NodeId, Set<String>> nodeIdToLabels) { + Map<NodeId, Set<String>> newMap = new HashMap<NodeId, Set<String>>(); + for (Entry<NodeId, Set<String>> entry : nodeIdToLabels.entrySet()) { + NodeId id = entry.getKey(); + Set<String> labels = entry.getValue(); + newMap.put(id, normalizeLabels(labels)); + } + return newMap; + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java ---------------------------------------------------------------------- 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 012d799..73ec906 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 class ConverterUtils { } 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); http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java ---------------------------------------------------------------------- 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 9749299..ff0e101 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 class NodeLabelTestBase { public static void assertCollectionEquals(Collection<String> c1, Collection<String> c2) { - Assert.assertEquals(c1.size(), c2.size()); - Iterator<String> i1 = c1.iterator(); - Iterator<String> i2 = c2.iterator(); - while (i1.hasNext()) { - Assert.assertEquals(i1.next(), i2.next()); - } + Set<String> s1 = new HashSet<String>(c1); + Set<String> s2 = new HashSet<String>(c2); + Assert.assertEquals(s1, s2); + Assert.assertTrue(s1.containsAll(s2)); } public static <E> Set<E> toSet(E... elements) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/0fd97f9c/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java ---------------------------------------------------------------------- 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 ea29f3a..a56a595 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 class TestCommonNodeLabelsManager extends NodeLabelTestBase { 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
