wilfred-s commented on code in PR #971:
URL: https://github.com/apache/yunikorn-core/pull/971#discussion_r1895107706
##########
pkg/scheduler/ugm/queue_tracker.go:
##########
@@ -410,11 +410,7 @@ func (qt *QueueTracker) canRunApp(hierarchy []string,
applicationID string, trac
func (qt *QueueTracker) canBeRemoved() bool {
for _, childQT := range qt.childQueueTrackers {
// quick check to avoid further traversal
- if childQT.canBeRemovedInternal() {
- if !childQT.canBeRemoved() {
Review Comment:
The check works even for deeper trees. What is not supported is a recursive
delete so that is what it prevents from happening.
root -> parent1 .to. parent5 -> child1 .to. child5 -> leaf1 .to. leaf5
The .to. signifies a repeat of the queues. So we have 5 parents under the
root. Each with 5 child queues etc.
When we check for a removal we traverse the tree and would do a depth first
check. To remove parent1 it has to be empty and have no quota set, that is the
original call. Before we go all the way to the leaf at any point we check if
the child queues at the current level can be removed.
If any child has a quota set, even without any usage, we cannot remove
parent and would return false.
I think the check `len(qt.childQueueTrackers) == 0` in
`canBeRemovedInternal()` should not be there as well as it stops going down the
tree. I think @SP12893678 and @ryankert01 made the correct remarks to start of
the conversation. I think we need to fix that vunction and make it work
recursively as it should.
--
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]
