pbacsko commented on code in PR #926:
URL: https://github.com/apache/yunikorn-core/pull/926#discussion_r1705292115
##########
pkg/scheduler/ugm/queue_tracker.go:
##########
@@ -403,24 +403,20 @@ func (qt *QueueTracker) canRunApp(hierarchy []string,
applicationID string, trac
// it decides the removal. It returns false the moment it sees any unexpected
values for any queue in any levels.
// Note: Lock free call. The RLock of the linked tracker (UserTracker and
GroupTracker) should be held before calling this function.
func (qt *QueueTracker) canBeRemoved() bool {
- for _, childQT := range qt.childQueueTrackers {
- // quick check to avoid further traversal
- if childQT.canBeRemovedInternal() {
+ if len(qt.childQueueTrackers) > 0 {
+ for _, childQT := range qt.childQueueTrackers {
+ // quick check to avoid further traversal
if !childQT.canBeRemoved() {
return false
}
- } else {
- return false
}
+ return true // removable because all children are removable
}
- // reached leaf queues, no more to traverse
- return qt.canBeRemovedInternal()
-}
-func (qt *QueueTracker) canBeRemovedInternal() bool {
- if len(qt.runningApplications) == 0 &&
resources.IsZero(qt.resourceUsage) && len(qt.childQueueTrackers) == 0 &&
- qt.maxRunningApps == 0 && resources.IsZero(qt.maxResources) {
- return true
- }
- return false
+ // reached a leaf queue
+ return len(qt.runningApplications) == 0 &&
+ resources.IsZero(qt.resourceUsage) &&
+ len(qt.childQueueTrackers) == 0 &&
+ qt.maxRunningApps == 0 &&
+ resources.IsZero(qt.maxResources)
Review Comment:
Even in the old code, `canBeRemovedInternal()` only returns true if
`len(qt.childQueueTrackers) == 0`. So you need to descend down the hierarchy.
You can't return earlier, therefore the optimization is not really there.
--
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]
