From f5f1c81e7dcae0272e71ef4e6bedfc00b8c677d6 Mon Sep 17 00:00:00 2001 From: Ray Chiang Date: Fri, 15 Jul 2016 14:35:50 -0700 Subject: [PATCH] YARN-5272. Handle queue names consistently in FairScheduler. (Wilfred Spiegelenburg via rchiang) --- .../fair/AllocationFileLoaderService.java | 4 ++- .../scheduler/fair/QueueManager.java | 6 +++- .../fair/TestAllocationFileLoaderService.java | 28 +++++++++++++++++++ .../scheduler/fair/TestFairScheduler.java | 11 ++++++++ .../scheduler/fair/TestQueueManager.java | 3 ++ 5 files changed, 50 insertions(+), 2 deletions(-) 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 661caa7895..fab536d2a5 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 @@ -53,6 +53,7 @@ import org.w3c.dom.Text; import org.xml.sax.SAXException; +import com.google.common.base.CharMatcher; import com.google.common.annotations.VisibleForTesting; @Public @@ -445,7 +446,8 @@ private void loadQueue(String parentName, Element element, Set reservableQueues, Set nonPreemptableQueues) throws AllocationConfigurationException { - String queueName = element.getAttribute("name").trim(); + String queueName = CharMatcher.WHITESPACE.trimFrom( + element.getAttribute("name")); if (queueName.contains(".")) { throw new AllocationConfigurationException("Bad fair scheduler config " 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 aeadcf6b49..8d308dc9ba 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 @@ -37,6 +37,7 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.xml.sax.SAXException; +import com.google.common.base.CharMatcher; import com.google.common.annotations.VisibleForTesting; /** * Maintains a list of queues as well as scheduling parameters for each queue, @@ -457,6 +458,9 @@ public void updateAllocationConfiguration(AllocationConfiguration queueConf) { */ @VisibleForTesting boolean isQueueNameValid(String node) { - return !node.isEmpty() && node.equals(node.trim()); + // use the same white space trim as in QueueMetrics() otherwise things fail + // guava uses a different definition for whitespace than java. + return !node.isEmpty() && + node.equals(CharMatcher.WHITESPACE.trimFrom(node)); } } 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 cc91ef9c8c..11d49818aa 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 @@ -20,8 +20,11 @@ import static org.junit.Assert.*; import java.io.File; +import java.io.FileOutputStream; import java.io.FileWriter; +import java.io.OutputStreamWriter; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import java.util.List; import org.apache.hadoop.conf.Configuration; @@ -580,6 +583,31 @@ public void testQueueNameContainingOnlyWhitespace() throws Exception { allocLoader.reloadAllocations(); } + /** + * Verify that you can't have the queue name with just a non breaking + * whitespace in the allocations file. + */ + @Test (expected = AllocationConfigurationException.class) + public void testQueueNameContainingNBWhitespace() throws Exception { + Configuration conf = new Configuration(); + conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE); + + PrintWriter out = new PrintWriter(new OutputStreamWriter( + new FileOutputStream(ALLOC_FILE), StandardCharsets.UTF_8)); + out.println(""); + out.println(""); + out.println(""); + out.println(""); + out.println(""); + out.close(); + + AllocationFileLoaderService allocLoader = new AllocationFileLoaderService(); + allocLoader.init(conf); + ReloadListener confHolder = new ReloadListener(); + allocLoader.setReloadListener(confHolder); + allocLoader.reloadAllocations(); + } + /** * Verify that defaultQueueSchedulingMode can't accept FIFO as a value. */ 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 43ebe53917..f92af35e3b 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 @@ -4362,6 +4362,17 @@ public void testQueueNameWithTrailingSpace() throws Exception { assertEquals(1, scheduler.getQueueManager().getLeafQueue("B.C", true) .getNumRunnableApps()); assertNotNull(scheduler.getSchedulerApp(appAttemptId3)); + + // submit app with queue name "A\u00a0" (non-breaking space) + ApplicationAttemptId appAttemptId4 = createAppAttemptId(4, 1); + AppAddedSchedulerEvent appAddedEvent4 = new AppAddedSchedulerEvent( + appAttemptId4.getApplicationId(), "A\u00a0", "user1"); + scheduler.handle(appAddedEvent4); + // submission rejected + assertEquals(3, scheduler.getQueueManager().getLeafQueues().size()); + assertNull(scheduler.getSchedulerApplications().get(appAttemptId4. + getApplicationId())); + assertNull(scheduler.getSchedulerApp(appAttemptId4)); } @Test 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 a9b27a1a44..33d4419252 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 @@ -131,6 +131,9 @@ public void testCheckQueueNodeName() { assertFalse(queueManager.isQueueNameValid(" a")); assertFalse(queueManager.isQueueNameValid("a ")); assertFalse(queueManager.isQueueNameValid(" a ")); + assertFalse(queueManager.isQueueNameValid("\u00a0")); + assertFalse(queueManager.isQueueNameValid("a\u00a0")); + assertFalse(queueManager.isQueueNameValid("\u00a0a\u00a0")); assertTrue(queueManager.isQueueNameValid("a b")); assertTrue(queueManager.isQueueNameValid("a")); }