K0K0V0K commented on code in PR #7607:
URL: https://github.com/apache/hadoop/pull/7607#discussion_r2053688000
##########
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:
##########
@@ -553,6 +553,11 @@ public boolean isEligibleForAutoQueueCreation() {
isAutoQueueCreationV2Enabled(getQueuePathObject());
}
+ public boolean isEligibleForLegacyAutoQueueCreation() {
Review Comment:
Some java doc can be use full here like isEligibleForAutoQueueCreation has.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/ProportionalCapacityPreemptionPolicy.java:
##########
@@ -430,12 +430,14 @@ private void cleanupStaledPreemptionCandidates(long
currentTime) {
}
private Set<String> getLeafQueueNames(TempQueuePerPartition q) {
- // Also exclude ParentQueues, which might be without children
- if (CollectionUtils.isEmpty(q.children)
- && !(q.parentQueue instanceof ManagedParentQueue)
- && (q.parentQueue == null
- || !q.parentQueue.isEligibleForAutoQueueCreation())) {
- return ImmutableSet.of(q.queueName);
+ // Only consider this a leaf queue if:
+ // It is a concrete leaf queue (not a childless parent)
+ if (CollectionUtils.isEmpty(q.children)) {
+ CSQueue queue = scheduler.getQueue(q.queueName);
+ if (queue instanceof LeafQueue || queue instanceof AutoCreatedLeafQueue)
{
Review Comment:
Maybe we can check for AbstractLeafQueue here instead of LeafQueue or
AutoCreatedLeafQueue ?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/TestProportionalCapacityPreemptionPolicy.java:
##########
@@ -1083,46 +1083,67 @@ public void testRefreshPreemptionProperties() throws
Exception {
}
@Test
- public void testLeafQueueNameExtraction() throws Exception {
- ProportionalCapacityPreemptionPolicy policy =
- buildPolicy(Q_DATA_FOR_IGNORE);
+ public void testLeafQueueNameExtractionWithFlexibleAQC() throws Exception {
+ runLeafQueueNameExtractionTest(true);
+ }
+
+ @Test
+ public void testLeafQueueNameExtractionWithLegacyAQC() throws Exception {
+ runLeafQueueNameExtractionTest(false);
+ }
+
+ private void runLeafQueueNameExtractionTest(boolean isFlexible) throws
Exception {
Review Comment:
NIT: maybe this runLeafQueueNameExtractionTest can be refactored to remove
the flag parameter.
https://martinfowler.com/bliki/FlagArgument.html
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
