epp: hold flow-control queue under saturation via in-flight credit

[ruocco]

The flow controller's dispatch gate (dispatchCycle: saturation >= usageLimit) did not hold back non-sheddable (priority-0) traffic under sustained load: the backend's internal waiting queue grew to hundreds while the EPP flow-control queue stayed near-empty, so the backlog accumulated inside the model server (ordered by the model server) instead of being held in the EPP queue where the fairness and ordering policies apply.

Root cause: the saturation signal is computed from endpoint metrics scraped on a poller (RefreshMetricsInterval, default 50ms), while dispatchCycle runs on a 1ms ticker and dispatches one item per cycle. Between two scrapes the controller issues many dispatches against a stale-low WaitingQueueSize, overshooting the gate before the next scrape reflects them. The utilization detector also aggregated per-endpoint scores with an unweighted average, letting one saturated endpoint be diluted by idle ones.

Fix:

• Add SaturationDetector.SaturationWithInFlight(ctx, endpoints, inFlight), with the contract Saturation(...) == SaturationWithInFlight(..., 0). The controller passes the count of gate-passing dispatches not yet reflected in scraped metrics; the detector attributes them to the pool's load so the gate holds within one scrape window. Implemented for the utilization and concurrency detectors; all mock implementations updated.
• Processor tracks inFlightCredit (incremented per dispatch, reset when a fresher metrics sample lands or the pool is empty) and feeds it to the detector. State is touched only by the single Run goroutine, so no synchronization is added.
• The utilization detector now aggregates endpoint scores with max (roofline at the pool level) instead of an unweighted average, so a single saturated endpoint is no longer diluted by idle ones.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Release note (write NONE if no user-facing change):

The utilization saturation detector now compensates the flow-control gate for scrape lag using the in-flight request count from a configured `inflight-load-producer`, holding backpressure within a single scrape window instead of overshooting between polls. Gate behavior changes only when a producer is configured; without one, the detector relies solely on scraped metrics as before.

[animeshtrivedi]

cc: @praveingk @vMaroon

[LukeAVanDrie]

@ruocco, have you tried using the concurrency-detector instead? That seems better suited for your use case without making any overhauls to the flow control runtime

[ruocco]

@ruocco, have you tried using the concurrency-detector instead? That seems better suited for your use case without making any overhauls to the flow control runtime

For a very narrow and specific use case, meaning fixing a deployment configuration the traces, I can find a number that approximates this behaviour. It will still sometimes overshoot and sometimes undershoot.

But if we want to have any meaningful impact on production environment, where the deployment and workload can vary, the only way is to have a reliable dynamic way of moving the queue away from vllm and into llm-d.

[ruocco]

I've updated the PR, I'm now using the existing inflight-load-producer to account for both dispatched requests before the poll and completed ones.

Without inflight-load-producer the previous behaviour still stands, i.e. the last queue depth polled value is used and there is no compensation for the requests that are dispatched in between the polls.

@animeshtrivedi FYI

[LukeAVanDrie]

Sorry for the delay on this, I had some reservations on your first commit which have largely been resolved in the second. Deriving the credit from inflight-load-producer gives it per-request identity, makes the accounting much safer, and neither the controller nor the plugin interface needs to change anymore.

One related update: #1991 merged since your last push, adding a hybrid mode to the concurrency detector (per-endpoint max(requestRatio, tokenRatio), averaged across endpoints). It addresses the dimension-mismatch half of your objection, since whichever axis the workload stresses binds first, so it may be a usable interim config for your deployment. The limits are still static, though, so it doesn't remove the calibration burden this PR targets.

One observation on where this sits architecturally. The PR fuses the two detectors' data sources, with the scrape as primary truth and the EPP-tracked count as a correction. The capacity-ledger proposal in #2061 (docs/flow-control-capacity-ledger.md) ends at the inverse: EPP-tracked accounting becomes primary and the scrape becomes a reconciliation input. Its migration path includes an intermediate step that grows out of exactly these components, inflight-load-producer plus the concurrency detector: track in-flight KV tokens and slots per endpoint, releasing prompt tokens at first token and the rest at end of stream. Your formula stops right at that step's boundary.

The credit corrects the queue term, but unobserved dispatches also consume KV as soon as prefill lands, and compensating that axis needs the per-request token estimates the producer already tracks. Leaving it out here is reasonable, and inverting the roles later stays a localized change. If you have time, I would appreciate your review on #2061.

Two changes I would like to see before this merges:

1. Drop the average -> max change from this PR. The credit corrects each endpoint's score before aggregation, so it fixes the scrape-lag defect under either aggregation; the PR doesn't need max to deliver its fix. Max has consequences that need their own design. #1991 also just standardized the other shape on the concurrency detector (roofline per endpoint, average across the pool), so pool-level max here would go against that precedent. I understand the dilution defect you're targeting, but I think the fix has to keep staleness handling separate from aggregation and account for routing. Can we create an issue for this and discuss there?
2. P/D disaggregation breaks the credit as-is. With the default producer config, prefill-profile request counters are held until end-of-stream, so a prefill endpoint shows phantom credit for the full decode duration of every request whose prefill it served.

Follow-up, non-blocking: Filter() still reads raw scraped metrics, so the thundering-herd defect documented in the README (a routing problem) gets none of this benefit while the gate does. The same credit could serve the per-endpoint filter in a separate PR.

Please also add a release note (gate behavior changes when a producer is configured) and file or link an issue for tracking.

[LukeAVanDrie]

With max aggregation the staleness sentinel of 1.0 becomes the pool saturation. One endpoint with missing or stale metrics closes the gate for the whole pool, and also trips the Saturation() >= 1.0 shedding logic for the legacy admission controller. A new endpoint joining before its first scrape hits this during scale-up under load. Under average a stale endpoint cost 1/N. With your change, it takes out the pool. Max also over-throttles on fresh metrics: the scheduler (and this detector's own Filter) route away from the hottest endpoint, so one hot pod gating nine idle ones defeats work conservation.

The credit doesn't depend on this change, since it corrects each endpoint's score before aggregation. I'd revert to average in this PR.

If addressing the skew is still required we can fix that separately. E.g., by excluding staleness from the aggregate while fresh endpoints exist and possibly adapting the aggregate to account for routing. This change also alters Saturation() semantics for consumers outside flow control and for every dashboard on the pool-saturation metric, so on its own it needs a release note and discussion.

[LukeAVanDrie]

In an aggregated pool this formula is sound.

• Requests in transit to the engine belong in the credit (that is the compensation you want).
• Completions since the scrape net out, since they decrement the count while the stale Waiting + Running still includes them.
• Requests that finished generating but haven't hit EndOfStream yet add a small standing bias of roughly completion rate times drain latency, which errs toward holding the gate and is backstopped by the state janitor.

Because every increment has a matching decrement event, nothing should accumulate.

P/D disaggregation is where the assumption breaks. With the default addEstimatedOutputTokens=false, the producer releases only token counters at StartOfStream; the request counter for every profile, including prefill, is held until EndOfStream. A prefill endpoint finishes its physical work at TTFT and its scraped Waiting/Running drop, but its InFlightLoad.Requests stays elevated for the whole decode. Its credit is then roughly the number of requests currently decoding whose prefill it served.

This bias scales with pool throughput instead of a small scalar constant, saturates the qRatio at the default threshold of 5, and with your max aggregation approach can hold the pool gate closed under sustained decode load.

I'd fix this in the producer rather than here (and maybe as an independent PR). Release the prefill profile's request counter at StartOfStream in both config paths. The addEstimatedOutputTokens=true path already does this via release(), and prefill is physically complete at first token. It would also correct the concurrency detector's view of prefill pods. The alternative is documenting that the credit assumes aggregated (non-P/D) pools.

^ I am happy to create an issue for this and get other opinions here.

[LukeAVanDrie]

Nit: I think some additional content would help here. The subtraction is only sound when the metrics mapping defines runningRequestsSpec (defaults exist for vLLM and SGLang); a custom mapping without it leaves RunningRequestsSize at 0 and the credit over-counts by the running count.

And InFlightRequests is per EPP replica while Waiting + Running is engine-global, so with multiple EPP replicas the credit floors at 0 and the compensation silently disappears. That is the same single-writer assumption the concurrency detector makes, but it should be stated, especially as there has been an influx in multi-replica saturation detector issues resulting from batch gateway benchmarking.

[LukeAVanDrie]

@asharkhan3101 Could I get your perspective on this PR (and my capacity ledger design in #2061 if you have time)? I want to make sure the detectors are converging on the same north star for capacity management. One concrete tension: #1991 settled on per-endpoint roofline aggregated by pool average, while this PR moves the utilization detector to a pool-level max we should pick one shape deliberately rather than diverge by accident.

These are tricky to review because their major effects are on workload dynamics that aren't apparent from code inspection. The eviction proposal in #2061 already commits to a closed-loop simulation harness (synthetic pool, configurable sensor lag, burst shapes) for the reclamation controller. I'm wondering whether we should generalize that into a traffic-flow simulation for saturation detection broadly, so changes like this one can be evaluated during development instead of against production traces.

[github-actions]

🚨 Unsigned commits detected! Please sign your commits.

For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation.

[ruocco]

@LukeAVanDrie Thanks for the thorough review.

• Reverted the average->max aggregation change. The credit corrects each endpoint's score before aggregation, so the scrape-lag fix stands under average; I've reverted the doc comment, README, and tests. Filed Utilization detector: reconsider pool aggregation (average vs. max/routing-aware) #2173 to discuss max (and routing-aware) aggregation on its own, given the semantic change for external consumers and the feat(concurrency-detector): Add hybrid mode to concurrency detector #1991 precedent.
• P/D disaggregation. Documented the assumption in the README and the Saturation doc comment (credit assumes aggregated / non-P/D pools). Filed inflight-load-producer: release prefill request counter at start-of-stream #2174 for the producer-side fix (release the prefill request counter at start-of-stream).
• README nits. Documented the runningRequestsSpec dependency and the per-EPP-replica / engine-global single-writer assumption.
• Release note added to the PR body.
• Filter() follow-up noted as non-blocking; agree it belongs in its own PR.