diff --git a/pkg/scheduler/queue/tenant_querier_assignment.go b/pkg/scheduler/queue/tenant_querier_assignment.go index d2293ee7243..2c87e8f5d73 100644 --- a/pkg/scheduler/queue/tenant_querier_assignment.go +++ b/pkg/scheduler/queue/tenant_querier_assignment.go @@ -323,13 +323,13 @@ func (tqa *tenantQuerierAssignments) shuffleTenantQueriers(tenantID TenantID, sc // // Note that because we use the shared tenantIDOrder and tenantOrderIndex to manage the queue, we functionally // ignore each Node's individual queueOrder and queuePosition. -func (tqa *tenantQuerierAssignments) dequeueGetNode(n *Node) (*Node, bool) { +func (tqa *tenantQuerierAssignments) dequeueSelectNode(dequeueFrom *Node) (*Node, bool) { // can't get a tenant if no querier set if tqa.currentQuerier == nil { return nil, true } - checkedAllNodes := n.childrenChecked == len(n.queueMap)+1 // must check local queue as well + checkedAllNodes := dequeueFrom.childrenChecked == len(dequeueFrom.queueMap)+1 // must check local queue as well // advance queue position for dequeue tqa.tenantOrderIndex++ @@ -338,8 +338,8 @@ func (tqa *tenantQuerierAssignments) dequeueGetNode(n *Node) (*Node, bool) { } // no children - if len(n.queueMap) == 0 || tqa.tenantOrderIndex == localQueueIndex { - return n, checkedAllNodes + if len(dequeueFrom.queueMap) == 0 || tqa.tenantOrderIndex == localQueueIndex { + return dequeueFrom, checkedAllNodes } checkIndex := tqa.tenantOrderIndex @@ -351,25 +351,25 @@ func (tqa *tenantQuerierAssignments) dequeueGetNode(n *Node) (*Node, bool) { tenantID := tqa.tenantIDOrder[checkIndex] tenantName := string(tenantID) - if _, ok := n.queueMap[tenantName]; !ok { + if _, ok := dequeueFrom.queueMap[tenantName]; !ok { // tenant not in _this_ node's children, move on checkIndex++ continue } // increment nodes checked even if not in tenant-querier map - n.childrenChecked++ - checkedAllNodes = n.childrenChecked == len(n.queueMap)+1 + dequeueFrom.childrenChecked++ + checkedAllNodes = dequeueFrom.childrenChecked == len(dequeueFrom.queueMap)+1 // if the tenant-querier set is nil, any querier can serve this tenant if tqa.tenantQuerierIDs[tenantID] == nil { tqa.tenantOrderIndex = checkIndex - return n.queueMap[tenantName], checkedAllNodes + return dequeueFrom.queueMap[tenantName], checkedAllNodes } if tenantQuerierSet, ok := tqa.tenantQuerierIDs[tenantID]; ok { if _, ok := tenantQuerierSet[*tqa.currentQuerier]; ok { tqa.tenantOrderIndex = checkIndex - return n.queueMap[tenantName], checkedAllNodes + return dequeueFrom.queueMap[tenantName], checkedAllNodes } } checkIndex++ @@ -379,14 +379,14 @@ func (tqa *tenantQuerierAssignments) dequeueGetNode(n *Node) (*Node, bool) { // dequeueUpdateState updates the node state to reflect the number of children that have been checked // for dequeueable elements, so that dequeueGetNode will stop checking if it has checked all possible nodes. -func (tqa *tenantQuerierAssignments) dequeueUpdateState(n *Node, v any, _ bool) { +func (tqa *tenantQuerierAssignments) dequeueUpdateState(dequeuedFrom *Node, v any, _ bool) { // we need to reset our checked nodes if we found a value to dequeue if v != nil { - n.childrenChecked = 0 + dequeuedFrom.childrenChecked = 0 return } - n.childrenChecked++ + dequeuedFrom.childrenChecked++ } // addChildNode adds a child to: diff --git a/pkg/scheduler/queue/tenant_queues.go b/pkg/scheduler/queue/tenant_queues.go index bd29893d857..10a7dded3a9 100644 --- a/pkg/scheduler/queue/tenant_queues.go +++ b/pkg/scheduler/queue/tenant_queues.go @@ -55,15 +55,15 @@ func newQueueBroker( // with query components at one level above tenants; if it is false, // tenant nodes will each maintain their own query component subtree. tree, err := NewTree( - tqas, // root - &roundRobinState{}, // tenant queues - &roundRobinState{}, // query components + tqas, // root; DequeueAlgorithm selects tenants + &roundRobinState{}, // tenant queues; DequeueAlgorithm selects query component + &roundRobinState{}, // query components; DequeueAlgorithm selects query from local queue ) if prioritizeQueryComponents { tree, err = NewTree( - &roundRobinState{}, // root - tqas, // query components - &roundRobinState{}, // tenant queues + &roundRobinState{}, // root; DequeueAlgorithm selects query component + tqas, // query components; DequeueAlgorithm selects tenant + &roundRobinState{}, // tenant queues; DequeueAlgorithm selects query from local queue ) } diff --git a/pkg/scheduler/queue/tree_queue.go b/pkg/scheduler/queue/tree_queue.go index ddeb39e154d..c1865844cb1 100644 --- a/pkg/scheduler/queue/tree_queue.go +++ b/pkg/scheduler/queue/tree_queue.go @@ -197,7 +197,7 @@ func (n *Node) dequeue() (QueuePath, any) { var dequeueNode *Node // continue until we've found a value or checked all nodes that need checking for v == nil && !checkedAllNodes { - dequeueNode, checkedAllNodes = n.dequeueAlgorithm.dequeueGetNode(n) + dequeueNode, checkedAllNodes = n.dequeueAlgorithm.dequeueSelectNode(n) var deletedNode bool // dequeuing from local queue if dequeueNode == n { @@ -218,8 +218,6 @@ func (n *Node) dequeue() (QueuePath, any) { childPath, v = dequeueNode.dequeue() // if the dequeue node is empty _after_ dequeuing, delete it from children if dequeueNode.IsEmpty() { - // removing an element sets our position one step forward; - // tell state to reset it to original queuePosition delete(n.queueMap, dequeueNode.Name()) deletedNode = n.dequeueAlgorithm.deleteChildNode(n, dequeueNode) } diff --git a/pkg/scheduler/queue/tree_queueing_algorithms.go b/pkg/scheduler/queue/tree_queueing_algorithms.go index 995885a173a..59854037310 100644 --- a/pkg/scheduler/queue/tree_queueing_algorithms.go +++ b/pkg/scheduler/queue/tree_queueing_algorithms.go @@ -6,10 +6,10 @@ package queue // at the layer-level -- every Node at the same depth in a TreeQueue shares the same DequeueAlgorithm, including state // that may be stored in a struct that implements DequeueAlgorithm. type DequeueAlgorithm interface { - addChildNode(*Node, *Node) - deleteChildNode(*Node, *Node) bool - dequeueGetNode(*Node) (*Node, bool) - dequeueUpdateState(*Node, any, bool) + addChildNode(parent, child *Node) + deleteChildNode(parent, child *Node) bool + dequeueSelectNode(dequeueFrom *Node) (*Node, bool) + dequeueUpdateState(dequeuedFrom *Node, v any, nodeDeleted bool) } // roundRobinState is the simplest type of DequeueAlgorithm; nodes which use this DequeueAlgorithm and are at @@ -54,14 +54,14 @@ func (rrs *roundRobinState) deleteChildNode(parent, child *Node) bool { // dequeueGetNode returns the node at the node's queuePosition. queuePosition represents the position of // the next node to dequeue from, and is incremented in dequeueUpdateState. -func (rrs *roundRobinState) dequeueGetNode(n *Node) (*Node, bool) { - checkedAllNodes := n.childrenChecked == len(n.queueOrder)+1 - if n.queuePosition == localQueueIndex { - return n, checkedAllNodes +func (rrs *roundRobinState) dequeueSelectNode(dequeueFrom *Node) (*Node, bool) { + checkedAllNodes := dequeueFrom.childrenChecked == len(dequeueFrom.queueOrder)+1 + if dequeueFrom.queuePosition == localQueueIndex { + return dequeueFrom, checkedAllNodes } - currentNodeName := n.queueOrder[n.queuePosition] - if node, ok := n.queueMap[currentNodeName]; ok { + currentNodeName := dequeueFrom.queueOrder[dequeueFrom.queuePosition] + if node, ok := dequeueFrom.queueMap[currentNodeName]; ok { return node, checkedAllNodes } return nil, checkedAllNodes @@ -70,16 +70,17 @@ func (rrs *roundRobinState) dequeueGetNode(n *Node) (*Node, bool) { // dequeueUpdateState increments queuePosition based on whether a child node was deleted during dequeue, // and updates childrenChecked to reflect the number of children checked so that the dequeueGetNode operation // will stop once it has checked all possible nodes. -func (rrs *roundRobinState) dequeueUpdateState(n *Node, v any, deletedNode bool) { +func (rrs *roundRobinState) dequeueUpdateState(dequeuedFrom *Node, v any, deletedNode bool) { if v != nil { - n.childrenChecked = 0 + dequeuedFrom.childrenChecked = 0 } else { - n.childrenChecked++ + dequeuedFrom.childrenChecked++ } + // removing an element sets our position one step forward, functionally incrementing position already if !deletedNode { - n.queuePosition++ + dequeuedFrom.queuePosition++ } - if n.queuePosition >= len(n.queueOrder) { - n.queuePosition = localQueueIndex + if dequeuedFrom.queuePosition >= len(dequeuedFrom.queueOrder) { + dequeuedFrom.queuePosition = localQueueIndex } }