Skip to content

Commit 1dfbb97

Browse files
committed
fix(flowcontrol): call up() after down() in maxMinHeap.Remove
Remove swaps position i with the last element then calls down() but not up(). When the last element comes from a different subtree, the ancestor relationship can break: Before: After Remove(index=3, time=1): 0 0 / \ / \ 5 50 5 50 / \ \ / \ 1 4 40 40 4 Last element (index 5, time=40) swaps into index 3. down(3): no children, no-op. Node 1 (min-level, time=5) must be lowest-priority in {5, 40, 4}. time=40 has lower priority than time=5 -- heap property violated. Add the missing up(i) call after down(i). If down moved the element, up is a no-op at the new position (and vice versa). Signed-off-by: RishabhSaini <rishabhsaini01@gmail.com>
1 parent 28b36f1 commit 1dfbb97

1 file changed

Lines changed: 5 additions & 2 deletions

File tree

pkg/epp/flowcontrol/framework/plugins/queue/maxminheap.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,12 @@ func (h *maxMinHeap) Remove(handle flowcontrol.QueueItemHandle) (flowcontrol.Que
285285
delete(h.handles, handle)
286286
h.byteSize.Add(^item.OriginalRequest().ByteSize() + 1) // Atomic subtraction
287287

288-
// The swapped item at index i might violate the heap property, so we must restore it
289-
// by bubbling the item down the heap.
288+
// The swapped item at index i might violate the heap property in either direction:
289+
// - downward: it is less extreme than its descendants -> down() fixes this.
290+
// - upward: it is more extreme than its ancestors -> up() fixes this.
291+
// Both calls are needed because the replacement can come from a different subtree.
290292
h.down(i)
293+
h.up(i)
291294
} else {
292295
// It's the last item, just remove it.
293296
h.items = h.items[:n]

0 commit comments

Comments
 (0)