tomicooler commented on a change in pull request #3500:
URL: https://github.com/apache/hadoop/pull/3500#discussion_r721973167
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/LeafQueue.java
##########
@@ -2330,25 +2330,27 @@ void
updateMaximumApplications(CapacitySchedulerConfiguration conf) {
int maxAppsForQueue = conf.getMaximumApplicationsPerQueue(getQueuePath());
int maxGlobalApplications = conf.getGlobalMaximumApplicationsPerQueue();
- if (maxGlobalApplications < 0) {
- maxGlobalApplications = conf.getMaximumSystemApplications();
- }
+ int maxSystemApplications = conf.getMaximumSystemApplications();
+ int baseMaxApplications = maxGlobalApplications > 0 ?
Review comment:
It is hard to see how absolute config type is handled and when the
maxSystemApplications is really used.
In the else branch (line 2342) we either have maxGlobalApplications <= 0 or
Absolute capacity config type, maxSystemApplications is used in the first case
maxGlobalApplications used in the later case.
Maybe something like this would be easier to read:
```
...
If (this.capacityConfigType == CapacityConfigType.ABSOLUTE_RESOURCE) {
maxLabel, maxAppsForQueue =
getUpdatedMaxAppsForQueueFromLabels(maxGlobalApplications);
} else {
If (maxGlobalApplications > 0) {
maxAppsForQueue = maxGlobalApplications;
} else {
maxLabel, maxAppsForQueue =
getUpdatedMaxAppsForQueueFromLabels(maxSystemApplications);
}
}
...
ImmutablePair<String, Integer> getUpdatedMaxAppsForQueueFromLabels(int base)
{
String maxLabel = RMNodeLabelsManager.NO_LABEL;
int maxAppsForQueue =
conf.getMaximumApplicationsPerQueue(getQueuePath());
for (String label : configuredNodeLabels) {
int maxApplicationsByLabel = (int) (base *
queueCapacities.getAbsoluteCapacity(label));
if (maxApplicationsByLabel > maxAppsForQueue) {
maxAppsForQueue = maxApplicationsByLabel;
maxLabel = label;
}
}
return new ImmutablePair<>(maxLabel, maxAppsForQueue);
}
```
It would be much simpler, but the maxLabel is needed for the log message
later.
--
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]
