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 zj...@apache.org on 2015/03/27 07:34:07 UTC
[05/50] [abbrv] hadoop git commit: YARN-3241. FairScheduler handles
invalid queue names inconsistently. (Zhihai Xu via kasha)
YARN-3241. FairScheduler handles invalid queue names inconsistently. (Zhihai Xu via kasha)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/bc9adb6b
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/bc9adb6b
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/bc9adb6b
Branch: refs/heads/YARN-2928
Commit: bc9adb6baa2fffa1fdbd0b66ea7008b4f49d0e01
Parents: 9a879a9
Author: Karthik Kambatla <ka...@apache.org>
Authored: Mon Mar 23 13:22:03 2015 -0700
Committer: Zhijie Shen <zj...@apache.org>
Committed: Thu Mar 26 23:29:43 2015 -0700
----------------------------------------------------------------------
hadoop-yarn-project/CHANGES.txt | 3 +
.../fair/AllocationFileLoaderService.java | 8 +-
.../scheduler/fair/FairScheduler.java | 2 +
.../fair/InvalidQueueNameException.java | 39 ++++++++++
.../scheduler/fair/QueueManager.java | 16 ++++
.../fair/TestAllocationFileLoaderService.java | 25 ++++++-
.../scheduler/fair/TestFairScheduler.java | 78 ++++++++++++++++++++
.../scheduler/fair/TestQueueManager.java | 13 +++-
8 files changed, 181 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index 90d906b..8ea93d3 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -144,6 +144,9 @@ Release 2.8.0 - UNRELEASED
YARN-3269. Yarn.nodemanager.remote-app-log-dir could not be configured to
fully qualified path. (Xuan Gong via junping_du)
+ YARN-3241. FairScheduler handles "invalid" queue names inconsistently.
+ (Zhihai Xu via kasha)
+
Release 2.7.0 - UNRELEASED
INCOMPATIBLE CHANGES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
index 76fa588..dab6d9f 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
@@ -426,13 +426,19 @@ public class AllocationFileLoaderService extends AbstractService {
Map<FSQueueType, Set<String>> configuredQueues,
Set<String> reservableQueues)
throws AllocationConfigurationException {
- String queueName = element.getAttribute("name");
+ String queueName = element.getAttribute("name").trim();
if (queueName.contains(".")) {
throw new AllocationConfigurationException("Bad fair scheduler config "
+ "file: queue name (" + queueName + ") shouldn't contain period.");
}
+ if (queueName.isEmpty()) {
+ throw new AllocationConfigurationException("Bad fair scheduler config "
+ + "file: queue name shouldn't be empty or "
+ + "consist only of whitespace.");
+ }
+
if (parentName != null) {
queueName = parentName + "." + queueName;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
index 1d97983..98a8de2 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
@@ -701,6 +701,8 @@ public class FairScheduler extends
appRejectMsg = queueName + " is not a leaf queue";
}
}
+ } catch (InvalidQueueNameException qne) {
+ appRejectMsg = qne.getMessage();
} catch (IOException ioe) {
appRejectMsg = "Error assigning app to queue " + queueName;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/InvalidQueueNameException.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/InvalidQueueNameException.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/InvalidQueueNameException.java
new file mode 100644
index 0000000..fc5ba16
--- /dev/null
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/InvalidQueueNameException.java
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair;
+
+import org.apache.hadoop.classification.InterfaceAudience.Private;
+import org.apache.hadoop.classification.InterfaceStability.Unstable;
+
+/**
+ * Thrown when Queue Name is malformed.
+ */
+@Private
+@Unstable
+public class InvalidQueueNameException extends IllegalArgumentException {
+ private static final long serialVersionUID = -7306320927804540011L;
+
+ public InvalidQueueNameException(String message) {
+ super(message);
+ }
+
+ public InvalidQueueNameException(String message, Throwable t) {
+ super(message, t);
+ }
+}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueueManager.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueueManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueueManager.java
index 27e571e..64442ab 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueueManager.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueueManager.java
@@ -36,6 +36,7 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.yarn.conf.YarnConfiguration;
import org.xml.sax.SAXException;
+import com.google.common.annotations.VisibleForTesting;
/**
* Maintains a list of queues as well as scheduling parameters for each queue,
* such as guaranteed share allocations, from the fair scheduler config file.
@@ -155,7 +156,13 @@ public class QueueManager {
// Move up the queue tree until we reach one that exists.
while (sepIndex != -1) {
+ int prevSepIndex = sepIndex;
sepIndex = name.lastIndexOf('.', sepIndex-1);
+ String node = name.substring(sepIndex+1, prevSepIndex);
+ if (!isQueueNameValid(node)) {
+ throw new InvalidQueueNameException("Illegal node name at offset " +
+ (sepIndex+1) + " for queue name " + name);
+ }
FSQueue queue;
String curName = null;
curName = name.substring(0, sepIndex);
@@ -401,4 +408,13 @@ public class QueueManager {
// recursively
rootQueue.updatePreemptionVariables();
}
+
+ /**
+ * Check whether queue name is valid,
+ * return true if it is valid, otherwise return false.
+ */
+ @VisibleForTesting
+ boolean isQueueNameValid(String node) {
+ return !node.isEmpty() && node.equals(node.trim());
+ }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestAllocationFileLoaderService.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/scheduler/fair/TestAllocationFileLoaderService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestAllocationFileLoaderService.java
index 3c166a5..b09573c 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestAllocationFileLoaderService.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestAllocationFileLoaderService.java
@@ -550,7 +550,30 @@ public class TestAllocationFileLoaderService {
allocLoader.setReloadListener(confHolder);
allocLoader.reloadAllocations();
}
-
+
+ /**
+ * Verify that you can't have the queue name with whitespace only in the
+ * allocations file.
+ */
+ @Test (expected = AllocationConfigurationException.class)
+ public void testQueueNameContainingOnlyWhitespace() throws Exception {
+ Configuration conf = new Configuration();
+ conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE);
+
+ PrintWriter out = new PrintWriter(new FileWriter(ALLOC_FILE));
+ out.println("<?xml version=\"1.0\"?>");
+ out.println("<allocations>");
+ out.println("<queue name=\" \">");
+ out.println("</queue>");
+ out.println("</allocations>");
+ out.close();
+
+ AllocationFileLoaderService allocLoader = new AllocationFileLoaderService();
+ allocLoader.init(conf);
+ ReloadListener confHolder = new ReloadListener();
+ allocLoader.setReloadListener(confHolder);
+ allocLoader.reloadAllocations();
+ }
@Test
public void testReservableQueue() throws Exception {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.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/scheduler/fair/TestFairScheduler.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java
index 267fbc2..7600a35 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java
@@ -4444,4 +4444,82 @@ public class TestFairScheduler extends FairSchedulerTestBase {
assertEquals("Incorrect number of perf metrics", 1,
collector.getRecords().size());
}
+
+ @Test
+ public void testQueueNameWithTrailingSpace() throws Exception {
+ scheduler.init(conf);
+ scheduler.start();
+ scheduler.reinitialize(conf, resourceManager.getRMContext());
+
+ // only default queue
+ assertEquals(1, scheduler.getQueueManager().getLeafQueues().size());
+
+ // submit app with queue name "A"
+ ApplicationAttemptId appAttemptId1 = createAppAttemptId(1, 1);
+ AppAddedSchedulerEvent appAddedEvent1 = new AppAddedSchedulerEvent(
+ appAttemptId1.getApplicationId(), "A", "user1");
+ scheduler.handle(appAddedEvent1);
+ // submission accepted
+ assertEquals(2, scheduler.getQueueManager().getLeafQueues().size());
+ assertNotNull(scheduler.getSchedulerApplications().get(appAttemptId1.
+ getApplicationId()));
+
+ AppAttemptAddedSchedulerEvent attempAddedEvent =
+ new AppAttemptAddedSchedulerEvent(appAttemptId1, false);
+ scheduler.handle(attempAddedEvent);
+ // That queue should have one app
+ assertEquals(1, scheduler.getQueueManager().getLeafQueue("A", true)
+ .getNumRunnableApps());
+ assertNotNull(scheduler.getSchedulerApp(appAttemptId1));
+
+ // submit app with queue name "A "
+ ApplicationAttemptId appAttemptId2 = createAppAttemptId(2, 1);
+ AppAddedSchedulerEvent appAddedEvent2 = new AppAddedSchedulerEvent(
+ appAttemptId2.getApplicationId(), "A ", "user1");
+ scheduler.handle(appAddedEvent2);
+ // submission rejected
+ assertEquals(2, scheduler.getQueueManager().getLeafQueues().size());
+ assertNull(scheduler.getSchedulerApplications().get(appAttemptId2.
+ getApplicationId()));
+ assertNull(scheduler.getSchedulerApp(appAttemptId2));
+
+ // submit app with queue name "B.C"
+ ApplicationAttemptId appAttemptId3 = createAppAttemptId(3, 1);
+ AppAddedSchedulerEvent appAddedEvent3 = new AppAddedSchedulerEvent(
+ appAttemptId3.getApplicationId(), "B.C", "user1");
+ scheduler.handle(appAddedEvent3);
+ // submission accepted
+ assertEquals(3, scheduler.getQueueManager().getLeafQueues().size());
+ assertNotNull(scheduler.getSchedulerApplications().get(appAttemptId3.
+ getApplicationId()));
+
+ attempAddedEvent =
+ new AppAttemptAddedSchedulerEvent(appAttemptId3, false);
+ scheduler.handle(attempAddedEvent);
+ // That queue should have one app
+ assertEquals(1, scheduler.getQueueManager().getLeafQueue("B.C", true)
+ .getNumRunnableApps());
+ assertNotNull(scheduler.getSchedulerApp(appAttemptId3));
+ }
+
+ @Test
+ public void testEmptyQueueNameInConfigFile() throws IOException {
+ conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE);
+ // set empty queue name
+ PrintWriter out = new PrintWriter(new FileWriter(ALLOC_FILE));
+ out.println("<?xml version=\"1.0\"?>");
+ out.println("<allocations>");
+ out.println("<queue name=\"\">");
+ out.println("</queue>");
+ out.println("</allocations>");
+ out.close();
+ try {
+ scheduler.init(conf);
+ Assert.fail("scheduler init should fail because" +
+ " empty queue name.");
+ } catch (Exception e) {
+ Assert.assertTrue(e.getMessage().contains(
+ "Failed to initialize FairScheduler"));
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/bc9adb6b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.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/scheduler/fair/TestQueueManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java
index ef0ec7e..b3ed542 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestQueueManager.java
@@ -123,7 +123,18 @@ public class TestQueueManager {
assertTrue(queueManager.getParentQueue("root.queue1", false)
.getChildQueues().isEmpty());
}
-
+
+ @Test
+ public void testCheckQueueNodeName() {
+ assertFalse(queueManager.isQueueNameValid(""));
+ assertFalse(queueManager.isQueueNameValid(" "));
+ assertFalse(queueManager.isQueueNameValid(" a"));
+ assertFalse(queueManager.isQueueNameValid("a "));
+ assertFalse(queueManager.isQueueNameValid(" a "));
+ assertTrue(queueManager.isQueueNameValid("a b"));
+ assertTrue(queueManager.isQueueNameValid("a"));
+ }
+
private void updateConfiguredLeafQueues(QueueManager queueMgr, String... confLeafQueues) {
AllocationConfiguration allocConf = new AllocationConfiguration(conf);
allocConf.configuredQueues.get(FSQueueType.LEAF).addAll(Sets.newHashSet(confLeafQueues));