Skip to content

fix(flowcontrol): map pre-admission TTL/cancel to eviction statuses i… - #2162

Open
LukeAVanDrie wants to merge 3 commits into
llm-d:mainfrom
LukeAVanDrie:fix/fc-preadmission-outcome-mapping
Open

fix(flowcontrol): map pre-admission TTL/cancel to eviction statuses i…#2162
LukeAVanDrie wants to merge 3 commits into
llm-d:mainfrom
LukeAVanDrie:fix/fc-preadmission-outcome-mapping

Conversation

@LukeAVanDrie

@LukeAVanDrie LukeAVanDrie commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

Two outcome-mapping fixes in flow control admission.

  1. A TTL expiry or client disconnect that fires before an item reaches a queue (buffered in the enqueue channel, blocked in submission, or racing managedQueue.Add) finalizes as QueueOutcomeRejectedOther, which translateFlowControlOutcome mapped to 500. Once the item is queued, the same event maps to 503 with a dropped-reason header. These pre-admission windows are widest under overload, so a saturation stall with TTL expiries showed up as a spike of 500s that was really backpressure.

    The translator now recognizes ErrTTLExpired and ErrContextCancelled in the RejectedOther/EvictedOther arm (after the shutdown check, which keeps precedence) and delegates to the matching eviction mapping, so the two paths cannot drift apart. The same branches cover EvictedOther for symmetry; nothing currently emits it with these sentinels.

  2. tryDistribution finalized a ManagedQueue lookup failure for a leased flow as QueueOutcomeRejectedCapacity, reporting an internal invariant violation as 429 with the Saturated header. It now finalizes as RejectedOther: a 500 with no saturation header. This also keeps the RejectedCapacity label in flow_control_requests_total to genuine capacity rejections. The registry error is flattened with %v because wrapping it would let ErrPriorityBandNotFound flow back through the connection closure and trip the priority-0 fallback in withConnectionWithFallback.

Coordination with #2047: it changes the same function (queued TTL eviction becomes 429 when the pool has endpoints). Whichever lands second threads the ttlPoolEmpty parameter through the two delegating calls, and the pool-emptiness probe in Admit should also cover RejectedOther + ErrTTLExpired so pre-admission TTL gets the same 429/503 split.

Tests: new TestTranslateFlowControlOutcome rows for pre-admission TTL and cancel on both outcomes, plus a shutdown-precedence row. The controller lookup-failure test now also asserts that registry sentinels do not leak into the finalized error and that no fallback retry happens. Both packages pass with -race.

Which issue(s) this PR fixes:

Fixes #2097

Part of #1187.

Release note:

NONE

@github-actions github-actions Bot added kind/bug Categorizes issue or PR as related to a bug. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 24, 2026
…nstead of 500

A TTL expiry or client disconnect before an item reaches a queue
(buffered in the enqueue channel, blocked in SubmitOrBlock, or racing
managedQueue.Add) finalizes as QueueOutcomeRejectedOther, which the
translator mapped to 500. Once the item is queued, the same event maps
to 503 with a dropped-reason header. These pre-admission windows are
widest under overload, so a saturation stall with TTL expiries showed
up as a spike of 500s that was really backpressure.

The translator now recognizes ErrTTLExpired and ErrContextCancelled in
the RejectedOther/EvictedOther arm (after the shutdown check, which
keeps precedence) and delegates to the matching eviction mapping, so
the two paths cannot drift apart. The same branches cover EvictedOther
for symmetry; nothing currently emits it with these sentinels.

Separately, tryDistribution finalized a ManagedQueue lookup failure for
a leased flow as QueueOutcomeRejectedCapacity, reporting an internal
invariant violation as 429 with the Saturated header. It now finalizes
as RejectedOther: a 500 with no saturation header. This also keeps the
RejectedCapacity metric label to genuine capacity rejections. The
registry error is flattened with %v because wrapping it would let
ErrPriorityBandNotFound flow back through the connection closure and
trip the priority-0 fallback in withConnectionWithFallback.

Fixes llm-d#2097
Part of llm-d#1187

Signed-off-by: Luke Van Drie <lukevandrie@google.com>
@LukeAVanDrie
LukeAVanDrie force-pushed the fix/fc-preadmission-outcome-mapping branch from b62ff61 to 03f0d26 Compare July 24, 2026 06:40
@github-actions github-actions Bot added kind/bug Categorizes issue or PR as related to a bug. and removed kind/bug Categorizes issue or PR as related to a bug. labels Jul 24, 2026
@LukeAVanDrie

Copy link
Copy Markdown
Contributor Author

I would like to hold this until #2047 is merged upon which I will make the updates mentioned in the PR description.

@ahg-g

ahg-g commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

I would like to hold this until #2047 is merged upon which I will make the updates mentioned in the PR description.

merged now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Flow Control] [Graduation Blocker] Pre-admission TTL/cancel returns 500; registry lookup failure mislabeled as saturation (429)

2 participants