YARN-10528. maxAMShare should only be accepted for leaf queues, not parent queues. Contributed by Siddharth Ahuja

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"
+        throw new AllocationConfigurationException(
-            + " for " + queueName + " are invalid. A queue element that "
+            getErrorString(queueName, RESERVATION));
-            + "contains child queue elements or that has the type='parent' "
+      } else if (isMaxAMShareSet) {
-            + "attribute cannot also include a reservation element.");
+        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();
   }

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();