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 @@ 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"
+        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 @@ 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();
   }

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