From 63c892278fe6a7c854b5f02c17048b047d419868 Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Tue, 14 Sep 2021 17:30:44 +0200 Subject: [PATCH] YARN-10912. AbstractCSQueue#updateConfigurableResourceRequirement: Separate validation logic from initialization logic (#3390) - capacityConfigType update is extracted to a separate method - validation logic is extracted to a helper function - min resource must not be greater than max resource is now checked after the max resource is updated Change-Id: I731c2639281721afed32c30854bafcf048d6ee28 Co-authored-by: Tamas Domok --- .../scheduler/capacity/AbstractCSQueue.java | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 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/capacity/AbstractCSQueue.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java index 0abe7c266f..f97460c311 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java @@ -369,14 +369,15 @@ protected void setupQueueConfigs(Resource clusterResource, setupConfigurableCapacities(configuration); updateAbsoluteCapacities(); + updateCapacityConfigType(); + // Fetch minimum/maximum resource limits for this queue if // configured - capacityConfigType = CapacityConfigType.NONE; this.resourceTypes = new HashSet<>(); for (AbsoluteResourceType type : AbsoluteResourceType.values()) { resourceTypes.add(type.toString().toLowerCase()); } - updateConfigurableResourceRequirement(getQueuePath(), clusterResource); + updateConfigurableResourceLimits(clusterResource); // Setup queue's maximumAllocation respecting the global // and the queue settings @@ -601,12 +602,9 @@ protected boolean checkConfigTypeIsAbsoluteResource(String queuePath, queuePath, resourceTypes); } - protected void updateConfigurableResourceRequirement(String queuePath, - Resource clusterResource) { + protected void updateCapacityConfigType() { + this.capacityConfigType = CapacityConfigType.NONE; for (String label : configuredNodeLabels) { - Resource minResource = getMinimumAbsoluteResource(queuePath, label); - Resource maxResource = getMaximumAbsoluteResource(queuePath, label); - LOG.debug("capacityConfigType is '{}' for queue {}", capacityConfigType, getQueuePath()); @@ -621,39 +619,35 @@ protected void updateConfigurableResourceRequirement(String queuePath, } else { validateAbsoluteVsPercentageCapacityConfig(localType); } + } + } - // If min resource for a resource type is greater than its max resource, - // throw exception to handle such error configs. - if (!maxResource.equals(Resources.none()) && Resources.greaterThan( - resourceCalculator, clusterResource, minResource, maxResource)) { - throw new IllegalArgumentException("Min resource configuration " - + minResource + " is greater than its max value:" + maxResource - + " in queue:" + getQueuePath()); - } + protected void updateConfigurableResourceLimits(Resource clusterResource) { + for (String label : configuredNodeLabels) { + final Resource minResource = getMinimumAbsoluteResource(queuePath, label); + Resource maxResource = getMaximumAbsoluteResource(queuePath, label); - // If parent's max resource is lesser to a specific child's max - // resource, throw exception to handle such error configs. if (parent != null) { - Resource parentMaxRes = parent.getQueueResourceQuotas() - .getConfiguredMaxResource(label); - if (Resources.greaterThan(resourceCalculator, clusterResource, - parentMaxRes, Resources.none())) { - if (Resources.greaterThan(resourceCalculator, clusterResource, - maxResource, parentMaxRes)) { - throw new IllegalArgumentException("Max resource configuration " + final Resource parentMax = parent.getQueueResourceQuotas().getConfiguredMaxResource(label); + validateMinResourceIsNotGreaterThanMaxResource(maxResource, parentMax, clusterResource, + "Max resource configuration " + maxResource + " is greater than parents max value:" - + parentMaxRes + " in queue:" + getQueuePath()); - } + + parentMax + " in queue:" + getQueuePath()); - // If child's max resource is not set, but its parent max resource is - // set, we must set child max resource to its parent's. - if (maxResource.equals(Resources.none()) - && !minResource.equals(Resources.none())) { - maxResource = Resources.clone(parentMaxRes); - } + // If child's max resource is not set, but its parent max resource is + // set, we must set child max resource to its parent's. + if (maxResource.equals(Resources.none()) && + !minResource.equals(Resources.none()) && + !parentMax.equals(Resources.none())) { + maxResource = Resources.clone(parentMax); } } + validateMinResourceIsNotGreaterThanMaxResource(minResource, maxResource, clusterResource, + "Min resource configuration " + + minResource + " is greater than its max value:" + maxResource + + " in queue:" + getQueuePath()); + LOG.debug("Updating absolute resource configuration for queue:{} as" + " minResource={} and maxResource={}", getQueuePath(), minResource, maxResource); @@ -663,6 +657,16 @@ protected void updateConfigurableResourceRequirement(String queuePath, } } + private void validateMinResourceIsNotGreaterThanMaxResource(Resource minResource, + Resource maxResource, + Resource clusterResource, + String validationError) { + if (!maxResource.equals(Resources.none()) && Resources.greaterThan( + resourceCalculator, clusterResource, minResource, maxResource)) { + throw new IllegalArgumentException(validationError); + } + } + private void validateAbsoluteVsPercentageCapacityConfig( CapacityConfigType localType) { if (!queuePath.equals("root")