feat(flowcontrol): demand-driven in-flight eviction#2093
Draft
LukeAVanDrie wants to merge 5 commits into
Draft
Conversation
…he tracker RequestEvictor gains the confirmation signal for reclamation pacing (an at-most-once listener fired when an evicted request's stream terminates), a victim-priority peek, and a strict per-decision victim priority bound on EvictN so multi-revocation decisions cannot churn the demand band itself. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
… eviction Implements the controller from docs/flow-control-eviction.md (PR llm-d#2061): on HoL blocking, scan blocked bands for queued demand strictly above the victim head, size revocations as a saturation deficit against a mean-footprint credit with pending-reclaim debits, and pace by confirmation (stream termination) plus a grace covering the sensor's visibility lag, with a timeout so a hung stream cannot wedge the gate. Bands with unattainable ceilings are excluded from demand. Adds the flowControl.enableEviction API field and its translation. New metrics: revocations issued and terminal outcomes, reclaim target, pending reclaim, confirmation latency. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
The confirmation grace is derived at wiring time from the selected saturation detector (dispatch-tick scale for the concurrency detector; refresh interval + staleness + engine reclaim budget for scraped sensors); remaining controller parameters are internal. The Director tracks dispatches and stream termination; the ext_proc server receives the evict-channel lookup. Signed-off-by: Luke Van Drie <lukevandrie@google.com> Co-authored-by: Rishabh Saini <rishabhsaini01@gmail.com> Signed-off-by: Luke Van Drie <lukevandrie@google.com>
Signed-off-by: Luke Van Drie <lukevandrie@google.com>
Operators tuning holdback and priorities need to see which band's demand drives destruction; the terminal-outcome counter stays unlabeled because it measures actuator health, not band economics. Adds an emission test covering the eviction metrics. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
10 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Implements the reclamation controller from the in-flight eviction design (#2061,
docs/flow-control-eviction.md): flow control can now evict in-flight sheddable requests (priority < 0) when higher-priority requests are queued behind pool saturation, converting the priority-inversion window from unbounded natural churn time to a bounded revocation round-trip.The merged victim filter/ordering policies and the ext_proc eviction channel are used as-is; the merged RequestEvictor is reused with small tracker extensions (commit 1), and the wiring is adapted from draft PR #1360; the wiring commit carries a Co-authored-by trailer for @RishabhSaini. Maintainers, please retain it in the squash-merge message. This PR adds the piece #1119 left open, deciding when and how much to evict:
flowControl.enableEviction(default false). The grace is derived at wiring time from the selected saturation detector (dispatch-tick scale for the concurrency detector; refresh interval + staleness threshold + an engine reclaim budget for scraped sensors); remaining parameters are internal. The design doc's Configuration section carries the rationale.Review guide: the diff is +1365/-38 across 22 files, but the decision logic is one new file,
pkg/epp/flowcontrol/controller/internal/reclamation.go(~330 lines, half of it the invariants as comments); review it against the design doc's Trigger/Sizing/Pacing sections. The rest splits into tracker extensions on the mergedRequestEvictor(commit 1), metrics (mechanical), and wiring (commit 3, adapted from #1360). Tests are ~45% of the diff. The three commits are ordered to be read in sequence.A closed-loop benchmark reproducing the design doc's Validation results is published alongside this PR as a companion draft (necessarily based on
main, since GitHub cannot base a PR on a fork branch; its reviewable diff is its final commit). Per the design doc, an end-to-end measurement of issue-to-abort latency is part of the acceptance criteria; eviction remains experimental (within the experimental flow control layer) until that measurement exists.Which issue(s) this PR fixes:
Part of #1119 (partial eviction and retry orchestration are deferred per the design doc's non-goals; close the issue here if we consider the core ask delivered)
Release note:
Test plan (new tests; functionality verified):
Not covered here (tracked as acceptance criteria in the design doc): live e2e measurement of the ImmediateResponse-to-engine-abort chain.
Known edge, tolerated with a comment at the site: re-tracking a victim (bound hit or failed eviction) races a concurrent completion in a sub-microsecond window; the re-inserted dead entry costs at most one spurious ConfirmationTimeout stall on a later decision.