Skip to content

[Flow Control] [Graduation Blocker] Recover posture and invariant guards for flow-control-owned goroutines #2098

Description

@LukeAVanDrie

Edited 2026-07-23: reworked after the queue plugin surface was removed; the invariant panics are eliminated by de-duplicating queue statistics.

The panic at managedqueue.go:186 only guards agreement between two copies of the same state: the queue already tracks its own byteSize, its length is the heap slice's length, and managedQueue mirrors both in atomics. Delete the mirror and there is no invariant left to check. Go doesn't trap negative arithmetic, so the choice was always an explicit panic or no duplicated state; we can now have the latter.

PR 1 (internal):

  • priorityqueue stamps each item's byte size on the heapItem at Add and debits the stored value on removal. Today it re-reads ByteSize() at :188/:209; callers implement that interface, so an unstable value can corrupt the counter.
  • managedQueue drops its len/byteSize atomics, delegates reads to the queue, and propagates aggregate deltas measured before/after each mutation. Aggregates become sums of true per-queue histories. Nothing can go negative, no check anywhere, panic deleted.
  • The *FlowItem downcasts in dispatchItem (processor.go:453) and the sweep predicate (:485) become comma-ok, like evictAll already is.
  • utilruntime.HandleCrash (log with component context, then repanic) at the top of the four flow-control-owned goroutines, as a backstop for unknown bugs. Incidentally covers plugin panics on those goroutines regardless of where [Framework] Define a panic-isolation posture for plugin extension points #2105 lands.
  • Rider (or its own small PR): orderedPriorityLevels becomes copy-on-write behind an atomic.Pointer[[]int], making AllOrderedPriorityLevels() a pointer load that returns a shared read-only slice. Removes the per-dispatch-cycle copy. An iterator doesn't help here: yielding under RLock deadlocks against a queued writer (the loop body re-acquires RLock), and yielding from a snapshot allocates the same copy.
  • doc.go gets a short error-handling-and-statistics section; a forbidigo rule bans panic( in the package, no allowlist.

PR 2 (SDK-breaking, consider before graduation):

Replace ComputeLimit(ctx, saturation, priorities) []float64 with a per-band call inside the dispatch loop:

Limit(ctx, saturation, priority, activePriorities) float64

One value in, one out: nothing to misalign (ceilings[i], processor.go:376), zero allocation on a 1ms path, and bands below the HoL block never compute. activePriorities is the shard's shared read-only slice from PR 1, so passing it is free; the read-only contract is documented on the interface. If we prefer a batch call, map[int]float64 works but allocates per cycle.

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIndicates an issue or PR lacks a triage label and requires one.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions