brumi1024 commented on code in PR #8802:
URL: https://github.com/apache/hadoop/pull/8802#discussion_r4263717202


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractParentQueue.java:
##########
@@ -310,86 +264,46 @@ void setChildQueues(Collection<CSQueue> childQueues) 
throws IOException {
             || parentCapacityType == QueueCapacityType.ABSOLUTE_RESOURCE) {
           // We don't allow any mixed absolute + {weight, percentage} between
           // children and parent
-          if (childrenCapacityType != parentCapacityType && 
!this.getQueuePath()
-              .equals(CapacitySchedulerConfiguration.ROOT)) {
-            throw new IOException("Parent=" + this.getQueuePath()
-                + ": When absolute minResource is used, we must make sure both 
"
-                + "parent and child all use absolute minResource");
+          String mixedError =
+              QueueCapacityChecks.checkAbsoluteResourceUsedByParentAndChildren(
+                  getQueuePath(), parentCapacityType, childrenCapacityType);
+          if (mixedError != null) {
+            throw new IOException(mixedError);
           }
 
           // Ensure that for each parent queue: parent.min-resource >=
           // Σ(child.min-resource).
           for (String nodeLabel : queueCapacities.getExistingNodeLabels()) {
-            Resource minRes = Resources.createResource(0, 0);
+            List<Resource> childrenMinResources = new 
ArrayList<>(childQueues.size());
             for (CSQueue queue : childQueues) {
-              // Accumulate all min/max resource configured for all child 
queues.
-              Resources.addTo(minRes, queue.getQueueResourceQuotas()
+              childrenMinResources.add(queue.getQueueResourceQuotas()
                   .getConfiguredMinResource(nodeLabel));
             }
             Resource resourceByLabel = 
labelManager.getResourceByLabel(nodeLabel,
                 queueContext.getClusterResource());
             Resource parentMinResource =
                 
usageTracker.getQueueResourceQuotas().getConfiguredMinResource(nodeLabel);
-            if (!parentMinResource.equals(Resources.none()) && 
Resources.lessThan(
-                resourceCalculator, resourceByLabel, parentMinResource, 
minRes)) {
-              throw new IOException(
-                  "Parent Queues" + " capacity: " + parentMinResource
-                      + " is less than" + " to its children:" + minRes
-                      + " for queue:" + getQueueName());
+            String minResourceError = 
QueueCapacityChecks.checkChildrenMinResourceWithinParent(
+                getQueueName(), parentMinResource, childrenMinResources, 
resourceByLabel,
+                resourceCalculator);
+            if (minResourceError != null) {
+              throw new IOException(minResourceError);
             }
           }
         }
 
         // When child uses percent
         if (childrenCapacityType == QueueCapacityType.PERCENT) {
-          float childrenPctSum = 0;
+          Collection<String> labels = queueCapacities.getExistingNodeLabels();
+          List<QueueCapacityChecks.QueueCapacityInput> children =
+              getCapacityInputs(childQueues, labels);

Review Comment:
   Done.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/QueueStateHelper.java:
##########
@@ -78,21 +78,111 @@ private static void reinitializeState(
 
   private static void initializeState(
       AbstractCSQueue queue, QueueState configuredState, QueueState 
parentState) {
-    QueueState currentState = configuredState == null ? DEFAULT_STATE : 
configuredState;
-
     if (parentState != null) {
-      if (configuredState == QueueState.RUNNING && parentState != 
QueueState.RUNNING) {
-        throw new IllegalArgumentException(
-            "The parent queue:" + queue.getParent().getQueuePath()
-                + " cannot be STOPPED as the child queue:" + 
queue.getQueuePath()
-                + " is in RUNNING state.");
+      String initialStateError = checkInitialState(new InitialStateInput(
+          queue.getQueuePath(), configuredState, 
queue.getParent().getQueuePath(), parentState));
+      if (initialStateError != null) {
+        throw new IllegalArgumentException(initialStateError);
       }
+    }
 
-      if (configuredState == null) {
-        currentState = parentState == QueueState.DRAINING ? QueueState.STOPPED 
: parentState;
-      }
+    queue.updateQueueState(getInitialState(configuredState, parentState));
+  }
+
+  /**
+   * Checks that the configured state is one that can be set in the 
configuration.
+   * The parsing of the value itself happens in
+   * {@link CapacitySchedulerConfiguration#getConfiguredState(QueuePath)}.
+   * @param configuredState the configured state of the queue, null if not 
configured
+   * @return null if the state is valid, otherwise the error message
+   */
+  public static String checkConfiguredState(QueueState configuredState) {
+    if (configuredState != null && 
!VALID_STATE_CONFIGURATIONS.contains(configuredState)) {
+      return "Invalid queue state configuration. We can only use RUNNING or 
STOPPED.";
+    }
+    return null;
+  }
+
+  /**
+   * Checks that a newly created queue configured as RUNNING is not placed 
under a parent that
+   * is not RUNNING.
+   * @param input the queue and its parent state
+   * @return null if the check passes, otherwise the error message
+   */
+  public static String checkInitialState(InitialStateInput input) {
+    if (input.getParentState() != null && input.getConfiguredState() == 
QueueState.RUNNING
+        && input.getParentState() != QueueState.RUNNING) {
+      return "The parent queue:" + input.getParentQueuePath()
+          + " cannot be STOPPED as the child queue:" + input.getQueuePath()
+          + " is in RUNNING state.";
+    }
+    return null;
+  }
+
+  /**
+   * Checks that a queue is activated only under a RUNNING parent. The
+   * exception type of a failure is {@link YarnException}.
+   * @param parentQueuePath the full path of the parent, null for root
+   * @param parentState the state of the parent, null for root
+   * @return null if the queue can be activated, otherwise the error message
+   */
+  public static String checkParentRunning(String parentQueuePath,

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to