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 sn...@apache.org on 2021/01/08 11:52:20 UTC

[hadoop] branch branch-3.2 updated: YARN-10528. maxAMShare should only be accepted for leaf queues, not parent queues. Contributed by Siddharth Ahuja

This is an automated email from the ASF dual-hosted git repository.

snemeth pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/branch-3.2 by this push:
     new 59795ec  YARN-10528. maxAMShare should only be accepted for leaf queues, not parent queues. Contributed by Siddharth Ahuja
59795ec is described below

commit 59795ec3d6cc30327a1bc3c7c9de2337abd7c7c2
Author: Szilard Nemeth <sn...@apache.org>
AuthorDate: Fri Jan 8 12:49:58 2021 +0100

    YARN-10528. maxAMShare should only be accepted for leaf queues, not parent queues. Contributed by Siddharth Ahuja
---
 .../fair/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 72c6c68..e89682d 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 ac30b23..b7cb249 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("<?xml version=\"1.0\"?>");
+    out.println("<allocations>");
+    out.println("<queue name=\"parent\" type=\"parent\">");
+    out.println("<maxAMShare>0.75</maxAMShare>");
+    out.println("</queue>");
+    out.println("</allocations>");
+    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("<?xml version=\"1.0\"?>");
+    out.println("<allocations>");
+    out.println("<queue name=\"parent\">");
+    out.println("<maxAMShare>0.76</maxAMShare>");
+    out.println(" <queue name=\"child\">");
+    out.println(" </queue>");
+    out.println("</queue>");
+    out.println("</allocations>");
+    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();


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org