[ https://issues.apache.org/jira/browse/YARN-10759?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17344573#comment-17344573 ]
Peter Bacsko edited comment on YARN-10759 at 5/14/21, 12:54 PM: ---------------------------------------------------------------- Thanks [~gandras] for the patch. I just have one thing to note. I can see that {{allowZeroCapacitySum}} has been moved to {{AbstractCSQueue}}, although it's really something which is meant for {{ParentQueue}}. I assume this is because the new code is easier to read and no type checks and casts are necessary. Is that correct? I'm wondering if this can cause problems. Because right now, this logic only runs inside {{ParentQueue}}: {noformat} // We also allow children's percent sum = 0 under the following // conditions // - Parent uses weight mode // - Parent uses percent mode, and parent has // (capacity=0 OR allowZero) if (parentCapacityType == QueueCapacityType.PERCENT) { if ((Math.abs(queueCapacities.getCapacity(nodeLabel)) > PRECISION) && (!allowZeroCapacitySum)) { throw new IOException( "Illegal" + " capacity sum of " + childrenPctSum + " for children of queue " + queueName + " for label=" + nodeLabel + ". It is set to 0, but parent percent != 0, and " + "doesn't allow children capacity to set to 0"); } } } {noformat} But after this refactor, leaf queues will have this property too with it being set to "false". Although there are no unit test failures, we need to double check if this extra boolean flag on leafs can have any impact on the existing code. was (Author: pbacsko): Thanks [~gandras] for the patch. I just have one to note. I can see that {{allowZeroCapacitySum}} has been moved to {{AbstractCSQueue}}, although it's really something which is meant for {{ParentQueue}}. I assume this is because the new code is easier to read and no type checks and casts are necessary. Is that correct? I'm wondering if this can cause problems. Because right now, this logic only runs inside {{ParentQueue}}: {noformat} // We also allow children's percent sum = 0 under the following // conditions // - Parent uses weight mode // - Parent uses percent mode, and parent has // (capacity=0 OR allowZero) if (parentCapacityType == QueueCapacityType.PERCENT) { if ((Math.abs(queueCapacities.getCapacity(nodeLabel)) > PRECISION) && (!allowZeroCapacitySum)) { throw new IOException( "Illegal" + " capacity sum of " + childrenPctSum + " for children of queue " + queueName + " for label=" + nodeLabel + ". It is set to 0, but parent percent != 0, and " + "doesn't allow children capacity to set to 0"); } } } {noformat} But after this refactor, leaf queues will have this property too with it being set to "false". Although there are no unit test failures, we need to double check if this extra boolean flag on leafs can have any impact on the existing code. > Encapsulate queue config modes > ------------------------------ > > Key: YARN-10759 > URL: https://issues.apache.org/jira/browse/YARN-10759 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler > Reporter: Andras Gyori > Assignee: Andras Gyori > Priority: Major > Attachments: YARN-10759.001.patch, YARN-10759.002.patch, > YARN-10759.003.patch, YARN-10759.004.patch > > > Capacity Scheduler queues have three modes: > * relative/percentage > * weight > * absolute > Most of them have their own: > * validation logic > * config setting logic > * effective capacity calculation logic > These logics can be easily extracted and encapsulated in separate config mode > classes. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org