From 59795ec3d6cc30327a1bc3c7c9de2337abd7c7c2 Mon Sep 17 00:00:00 2001 From: Szilard Nemeth Date: Fri, 8 Jan 2021 12:49:58 +0100 Subject: [PATCH] YARN-10528. maxAMShare should only be accepted for leaf queues, not parent queues. Contributed by Siddharth Ahuja --- .../allocation/AllocationFileQueueParser.java | 25 +++++-- .../fair/TestAllocationFileLoaderService.java | 71 +++++++++++++++++++ 2 files changed, 91 insertions(+), 5 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/allocation/AllocationFileQueueParser.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/allocation/AllocationFileQueueParser.java index 72c6c6801b3..e89682d789f 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/allocation/AllocationFileQueueParser.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/allocation/AllocationFileQueueParser.java @@ -126,6 +126,7 @@ public class AllocationFileQueueParser { NodeList fields = element.getChildNodes(); boolean isLeaf = true; boolean isReservable = false; + boolean isMaxAMShareSet = false; for (int j = 0; j < fields.getLength(); j++) { Node fieldNode = fields.item(j); @@ -157,6 +158,7 @@ public class AllocationFileQueueParser { float val = Float.parseFloat(text); val = Math.min(val, 1.0f); builder.queueMaxAMShares(queueName, val); + isMaxAMShareSet = true; } else if (MAX_CONTAINER_ALLOCATION.equals(field.getTagName())) { String text = getTrimmedTextData(field); ConfigurableResource val = @@ -220,7 +222,6 @@ public class AllocationFileQueueParser { isLeaf = false; } } - // if a leaf in the alloc file is marked as type='parent' // then store it as a parent queue if (isLeaf && !"parent".equals(element.getAttribute("type"))) { @@ -230,10 +231,11 @@ public class AllocationFileQueueParser { } } else { if (isReservable) { - throw new AllocationConfigurationException("The configuration settings" - + " for " + queueName + " are invalid. A queue element that " - + "contains child queue elements or that has the type='parent' " - + "attribute cannot also include a reservation element."); + throw new AllocationConfigurationException( + getErrorString(queueName, RESERVATION)); + } else if (isMaxAMShareSet) { + throw new AllocationConfigurationException( + getErrorString(queueName, MAX_AMSHARE)); } builder.configuredQueues(FSQueueType.PARENT, queueName); } @@ -253,6 +255,19 @@ public class AllocationFileQueueParser { builder.getMaxQueueResources(), queueName); } + /** + * Set up the error string based on the supplied parent queueName and element. + * @param parentQueueName the parent queue name. + * @param element the element that should not be present for the parent queue. + * @return the error string. + */ + private String getErrorString(String parentQueueName, String element) { + return "The configuration settings" + + " for " + parentQueueName + " are invalid. A queue element that " + + "contains child queue elements or that has the type='parent' " + + "attribute cannot also include a " + element + " element."; + } + private String getTrimmedTextData(Element element) { return ((Text) element.getFirstChild()).getData().trim(); } 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 ac30b237472..b7cb249d916 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 @@ -699,6 +699,77 @@ public class TestAllocationFileLoaderService { } } + /** + * Verify that a parent queue (type = parent) cannot have a maxAMShare element + * as dynamic queues won't be able to inherit this setting. + */ + @Test + public void testParentTagWitMaxAMShare() throws Exception { + Configuration conf = new Configuration(); + conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE); + + PrintWriter out = new PrintWriter(new FileWriter(ALLOC_FILE)); + out.println(""); + out.println(""); + out.println(""); + out.println("0.75"); + out.println(""); + out.println(""); + out.close(); + + AllocationFileLoaderService allocLoader = new AllocationFileLoaderService(); + allocLoader.init(conf); + ReloadListener confHolder = new ReloadListener(); + allocLoader.setReloadListener(confHolder); + try { + allocLoader.reloadAllocations(); + fail("Expect allocation parsing to fail as maxAMShare cannot be set for" + + " a parent queue."); + } catch (AllocationConfigurationException ex) { + assertEquals(ex.getMessage(), "The configuration settings for root.parent" + + " are invalid. A queue element that contains child queue elements" + + " or that has the type='parent' attribute cannot also include a" + + " maxAMShare element."); + } + } + + /** + * Verify that a parent queue that is not explicitly tagged with "type" + * as "parent" but has a child queue (implicit parent) cannot have a + * maxAMShare element. + */ + @Test + public void testParentWithMaxAMShare() throws Exception { + Configuration conf = new Configuration(); + conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE); + + PrintWriter out = new PrintWriter(new FileWriter(ALLOC_FILE)); + out.println(""); + out.println(""); + out.println(""); + out.println("0.76"); + out.println(" "); + out.println(" "); + out.println(""); + out.println(""); + out.close(); + + AllocationFileLoaderService allocLoader = new AllocationFileLoaderService(); + allocLoader.init(conf); + ReloadListener confHolder = new ReloadListener(); + allocLoader.setReloadListener(confHolder); + try { + allocLoader.reloadAllocations(); + fail("Expect allocation parsing to fail as maxAMShare cannot be set for" + + " a parent queue."); + } catch (AllocationConfigurationException ex) { + assertEquals(ex.getMessage(), "The configuration settings for root.parent" + + " are invalid. A queue element that contains child queue elements" + + " or that has the type='parent' attribute cannot also include a" + + " maxAMShare element."); + } + } + @Test public void testParentWithReservation() throws Exception { Configuration conf = new Configuration();