diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java index 507b798..308cd74 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java @@ -82,7 +82,7 @@ "Metrics for the resource scheduler"); protected static final MetricsInfo QUEUE_INFO = info("Queue", "Metrics by queue"); static final MetricsInfo USER_INFO = info("User", "Metrics by user"); - static final Splitter Q_SPLITTER = + public static final Splitter Q_SPLITTER = Splitter.on('.').omitEmptyStrings().trimResults(); final MetricsRegistry registry; diff --git 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 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 e8a9555..0c3bdd1 100644 --- 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 +++ 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 @@ -577,20 +577,10 @@ public FairSchedulerEventLog getEventLog() { */ protected synchronized void addApplication(ApplicationId applicationId, String queueName, String user, boolean isAppRecovering) { - if (queueName == null || queueName.isEmpty()) { - String message = "Reject application " + applicationId + - " submitted by user " + user + " with an empty queue name."; - LOG.info(message); - rmContext.getDispatcher().getEventHandler() - .handle(new RMAppRejectedEvent(applicationId, message)); - return; - } - - if (queueName.startsWith(".") || queueName.endsWith(".")) { + if (!checkQueueName(queueName)) { String message = "Reject application " + applicationId + " submitted by user " + user + " with an illegal queue name " - + queueName + ". " - + "The queue name cannot start/end with period."; + + queueName + "."; LOG.info(message); rmContext.getDispatcher().getEventHandler() .handle(new RMAppRejectedEvent(applicationId, message)); @@ -1682,4 +1672,21 @@ private String handleMoveToPlanQueue(String targetQueueName) { } return targetQueueName; } + + /** + * Check whether queue name is valid, + * return true if it is valid, otherwise return false. + */ + @VisibleForTesting + boolean checkQueueName(String queueName) { + if (queueName == null) { + return false; + } + + StringBuilder sb = new StringBuilder(); + for (String node : QueueMetrics.Q_SPLITTER.split(queueName)) { + sb.append(node).append('.'); + } + return sb.toString().equals(queueName + "."); + } } diff --git 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 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 9fadba9..9a863aa 100644 --- 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 +++ 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 @@ -4431,4 +4431,94 @@ public void testPerfMetricsInited() { 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 testCheckQueueName() { + assertFalse(scheduler.checkQueueName(null)); + assertFalse(scheduler.checkQueueName("")); + assertFalse(scheduler.checkQueueName("..")); + assertFalse(scheduler.checkQueueName("...")); + + assertFalse(scheduler.checkQueueName(".a.b")); + assertFalse(scheduler.checkQueueName("a.b.")); + + assertFalse(scheduler.checkQueueName("a.b ")); + assertFalse(scheduler.checkQueueName(" a.b")); + + assertFalse(scheduler.checkQueueName("a..b")); + assertFalse(scheduler.checkQueueName("a..b.c")); + assertFalse(scheduler.checkQueueName("a.b..c")); + assertFalse(scheduler.checkQueueName("a...b")); + assertFalse(scheduler.checkQueueName("a. .b")); + assertFalse(scheduler.checkQueueName("a. . .b")); + + assertFalse(scheduler.checkQueueName("a.b .c")); + assertFalse(scheduler.checkQueueName("a. b.c")); + assertFalse(scheduler.checkQueueName("a.b. c")); + assertFalse(scheduler.checkQueueName("a .b.c")); + + assertTrue(scheduler.checkQueueName("a.bb.c")); + assertTrue(scheduler.checkQueueName("a.b b.c")); + assertTrue(scheduler.checkQueueName("ab")); + assertTrue(scheduler.checkQueueName("a.b")); + assertTrue(scheduler.checkQueueName("a a.b")); + assertTrue(scheduler.checkQueueName("a.b b")); + } }