From f6b9f82b3fcb2151bf3b920419b9d67578d2698c Mon Sep 17 00:00:00 2001 From: Szilard Nemeth Date: Fri, 8 Jan 2021 12:41:17 +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 | 66 +++++++++++++++++++ 2 files changed, 86 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 72c6c6801b..e89682d789 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 @@ private void loadQueue(String parentName, Element element, 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 @@ private void loadQueue(String parentName, Element element, 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 @@ private void loadQueue(String parentName, Element element, 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 @@ private void loadQueue(String parentName, Element element, } } 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 @@ private void loadQueue(String parentName, Element element, 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 0650027b8d..9fb76cf261 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 @@ -736,6 +736,72 @@ public void testParentWithReservation() throws Exception { } } + /** + * 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 testParentTagWithMaxAMShare() throws Exception { + conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE); + + AllocationFileWriter.create() + .addQueue(new AllocationFileQueue.Builder("parent") + .parent(true) + .maxAMShare(0.75) + .build()) + .writeToFile(ALLOC_FILE); + + AllocationFileLoaderService allocLoader = + new AllocationFileLoaderService(scheduler); + 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 { + conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE); + + AllocationFileWriter.create() + .addQueue(new AllocationFileQueue.Builder("parent") + .parent(false) + .maxAMShare(0.76) + .subQueue(new AllocationFileQueue.Builder("child").build()) + .build()) + .writeToFile(ALLOC_FILE); + + AllocationFileLoaderService allocLoader = + new AllocationFileLoaderService(scheduler); + 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 testParentTagWithChild() throws Exception { conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE);