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 <tdomok@cloudera.com>
This commit is contained in:
Tamas Domok 2021-09-14 17:30:44 +02:00 committed by GitHub
parent 4f563ff1ba
commit 63c892278f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -369,14 +369,15 @@ protected void setupQueueConfigs(Resource clusterResource,
setupConfigurableCapacities(configuration); setupConfigurableCapacities(configuration);
updateAbsoluteCapacities(); updateAbsoluteCapacities();
updateCapacityConfigType();
// Fetch minimum/maximum resource limits for this queue if // Fetch minimum/maximum resource limits for this queue if
// configured // configured
capacityConfigType = CapacityConfigType.NONE;
this.resourceTypes = new HashSet<>(); this.resourceTypes = new HashSet<>();
for (AbsoluteResourceType type : AbsoluteResourceType.values()) { for (AbsoluteResourceType type : AbsoluteResourceType.values()) {
resourceTypes.add(type.toString().toLowerCase()); resourceTypes.add(type.toString().toLowerCase());
} }
updateConfigurableResourceRequirement(getQueuePath(), clusterResource); updateConfigurableResourceLimits(clusterResource);
// Setup queue's maximumAllocation respecting the global // Setup queue's maximumAllocation respecting the global
// and the queue settings // and the queue settings
@ -601,12 +602,9 @@ protected boolean checkConfigTypeIsAbsoluteResource(String queuePath,
queuePath, resourceTypes); queuePath, resourceTypes);
} }
protected void updateConfigurableResourceRequirement(String queuePath, protected void updateCapacityConfigType() {
Resource clusterResource) { this.capacityConfigType = CapacityConfigType.NONE;
for (String label : configuredNodeLabels) { for (String label : configuredNodeLabels) {
Resource minResource = getMinimumAbsoluteResource(queuePath, label);
Resource maxResource = getMaximumAbsoluteResource(queuePath, label);
LOG.debug("capacityConfigType is '{}' for queue {}", LOG.debug("capacityConfigType is '{}' for queue {}",
capacityConfigType, getQueuePath()); capacityConfigType, getQueuePath());
@ -621,39 +619,35 @@ protected void updateConfigurableResourceRequirement(String queuePath,
} else { } else {
validateAbsoluteVsPercentageCapacityConfig(localType); 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());
} }
// If parent's max resource is lesser to a specific child's max protected void updateConfigurableResourceLimits(Resource clusterResource) {
// resource, throw exception to handle such error configs. for (String label : configuredNodeLabels) {
final Resource minResource = getMinimumAbsoluteResource(queuePath, label);
Resource maxResource = getMaximumAbsoluteResource(queuePath, label);
if (parent != null) { if (parent != null) {
Resource parentMaxRes = parent.getQueueResourceQuotas() final Resource parentMax = parent.getQueueResourceQuotas().getConfiguredMaxResource(label);
.getConfiguredMaxResource(label); validateMinResourceIsNotGreaterThanMaxResource(maxResource, parentMax, clusterResource,
if (Resources.greaterThan(resourceCalculator, clusterResource, "Max resource configuration "
parentMaxRes, Resources.none())) {
if (Resources.greaterThan(resourceCalculator, clusterResource,
maxResource, parentMaxRes)) {
throw new IllegalArgumentException("Max resource configuration "
+ maxResource + " is greater than parents max value:" + 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 // 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. // set, we must set child max resource to its parent's.
if (maxResource.equals(Resources.none()) if (maxResource.equals(Resources.none()) &&
&& !minResource.equals(Resources.none())) { !minResource.equals(Resources.none()) &&
maxResource = Resources.clone(parentMaxRes); !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" LOG.debug("Updating absolute resource configuration for queue:{} as"
+ " minResource={} and maxResource={}", getQueuePath(), minResource, + " minResource={} and maxResource={}", getQueuePath(), minResource,
maxResource); 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( private void validateAbsoluteVsPercentageCapacityConfig(
CapacityConfigType localType) { CapacityConfigType localType) {
if (!queuePath.equals("root") if (!queuePath.equals("root")