You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by vi...@apache.org on 2014/11/07 23:16:31 UTC
hadoop git commit: YARN-2753. Fixed a bunch of bugs in the
NodeLabelsManager classes. Contributed by Zhihai xu.
Repository: hadoop
Updated Branches:
refs/heads/trunk 57760c066 -> 4cfd5bc7c
YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. Contributed by Zhihai xu.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/4cfd5bc7
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/4cfd5bc7
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/4cfd5bc7
Branch: refs/heads/trunk
Commit: 4cfd5bc7c18bb9a828f573b5c4d2b13fa28e732a
Parents: 57760c0
Author: Vinod Kumar Vavilapalli <vi...@apache.org>
Authored: Fri Nov 7 14:15:53 2014 -0800
Committer: Vinod Kumar Vavilapalli <vi...@apache.org>
Committed: Fri Nov 7 14:15:53 2014 -0800
----------------------------------------------------------------------
hadoop-yarn-project/CHANGES.txt | 3 ++
.../nodelabels/CommonNodeLabelsManager.java | 25 ++++++++++-------
.../nodelabels/TestRMNodeLabelsManager.java | 29 +++++++++++++++++++-
3 files changed, 46 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/4cfd5bc7/hadoop-yarn-project/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index 97b67b5..6e5f59a 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -903,6 +903,9 @@ Release 2.6.0 - UNRELEASED
YARN-2803. MR distributed cache not working correctly on Windows after
NodeManager privileged account changes. (Craig Welch via cnauroth)
+ YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. (Zhihai xu
+ via vinodkv)
+
Release 2.5.2 - UNRELEASED
INCOMPATIBLE CHANGES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/4cfd5bc7/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 ee1b945..1d86211 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
@@ -144,9 +144,7 @@ public class CommonNodeLabelsManager extends AbstractService {
@Override
public void handle(NodeLabelsStoreEvent event) {
- if (isInState(STATE.STARTED)) {
- handleStoreEvent(event);
- }
+ handleStoreEvent(event);
}
}
@@ -256,7 +254,7 @@ public class CommonNodeLabelsManager extends AbstractService {
if (null == labels || labels.isEmpty()) {
return;
}
-
+ Set<String> newLabels = new HashSet<String>();
labels = normalizeLabels(labels);
// do a check before actual adding them, will throw exception if any of them
@@ -266,11 +264,15 @@ public class CommonNodeLabelsManager extends AbstractService {
}
for (String label : labels) {
- this.labelCollections.put(label, new Label());
+ // shouldn't overwrite it to avoid changing the Label.resource
+ if (this.labelCollections.get(label) == null) {
+ this.labelCollections.put(label, new Label());
+ newLabels.add(label);
+ }
}
- if (null != dispatcher) {
+ if (null != dispatcher && !newLabels.isEmpty()) {
dispatcher.getEventHandler().handle(
- new StoreNewClusterNodeLabels(labels));
+ new StoreNewClusterNodeLabels(newLabels));
}
LOG.info("Add labels: [" + StringUtils.join(labels.iterator(), ",") + "]");
@@ -453,12 +455,15 @@ public class CommonNodeLabelsManager extends AbstractService {
LOG.error(msg);
throw new IOException(msg);
}
-
- if (labels == null || labels.isEmpty()) {
+
+ // the labels will never be null
+ if (labels.isEmpty()) {
continue;
}
- if (!originalLabels.containsAll(labels)) {
+ // originalLabels may be null,
+ // because when a Node is created, Node.labels can be null.
+ if (originalLabels == null || !originalLabels.containsAll(labels)) {
String msg =
"Try to remove labels = [" + StringUtils.join(labels, ",")
+ "], but not all labels contained by NM=" + nodeId;
http://git-wip-us.apache.org/repos/asf/hadoop/blob/4cfd5bc7/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java
----------------------------------------------------------------------
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 0ea7456..ed675f3 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
@@ -74,6 +74,13 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
Assert.assertEquals(mgr.getResourceByLabel("p1", null),
Resources.add(SMALL_RESOURCE, LARGE_NODE));
+ // check add labels multiple times shouldn't overwrite
+ // original attributes on labels like resource
+ mgr.addToCluserNodeLabels(toSet("p1", "p4"));
+ Assert.assertEquals(mgr.getResourceByLabel("p1", null),
+ Resources.add(SMALL_RESOURCE, LARGE_NODE));
+ Assert.assertEquals(mgr.getResourceByLabel("p4", null), EMPTY_RESOURCE);
+
// change the large NM to small, check if resource updated
mgr.updateNodeResource(NodeId.newInstance("n1", 2), SMALL_RESOURCE);
Assert.assertEquals(mgr.getResourceByLabel("p1", null),
@@ -374,7 +381,7 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
Assert.assertEquals(clusterResource,
mgr.getQueueResource("Q5", q5Label, clusterResource));
}
-
+
@Test(timeout=5000)
public void testGetLabelResourceWhenMultipleNMsExistingInSameHost() throws IOException {
// active two NM to n1, one large and one small
@@ -401,4 +408,24 @@ public class TestRMNodeLabelsManager extends NodeLabelTestBase {
mgr.getResourceByLabel("p1", null),
Resources.multiply(SMALL_RESOURCE, 2));
}
+
+ @Test(timeout = 5000)
+ public void testRemoveLabelsFromNode() throws Exception {
+ mgr.addToCluserNodeLabels(toSet("p1", "p2", "p3"));
+ mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1"), toSet("p1"),
+ toNodeId("n2"), toSet("p2"), toNodeId("n3"), toSet("p3")));
+ // active one NM to n1:1
+ mgr.activateNode(NodeId.newInstance("n1", 1), SMALL_RESOURCE);
+ try {
+ mgr.removeLabelsFromNode(ImmutableMap.of(toNodeId("n1:1"), toSet("p1")));
+ Assert.fail("removeLabelsFromNode should trigger IOException");
+ } catch (IOException e) {
+ }
+ mgr.replaceLabelsOnNode(ImmutableMap.of(toNodeId("n1:1"), toSet("p1")));
+ try {
+ mgr.removeLabelsFromNode(ImmutableMap.of(toNodeId("n1:1"), toSet("p1")));
+ } catch (IOException e) {
+ Assert.fail("IOException from removeLabelsFromNode " + e);
+ }
+ }
}