What happened:
- A request whose TTL expires or whose client disconnects BEFORE the item is admitted to a queue gets HTTP 500.
inferOutcome (pkg/epp/flowcontrol/controller/internal/item.go:174-198) maps TTL/cancel on a not-yet-queued item to QueueOutcomeRejectedOther, and translateFlowControlOutcome maps non-shutdown RejectedOther to 500 Internal (pkg/epp/requestcontrol/admission.go:238-243). "Not yet queued" includes items buffered in the enqueue channel and callers blocked in SubmitOrBlock -- exactly the overload conditions where TTL expiries peak. The same client behavior an instant later (item queued) yields EvictedTTL/EvictedContextCancelled and
- There is also a narrow race with the same result:
Finalize infers queued-ness from the item handle (item.go:132), which managedQueue.Add sets with no lock shared with the finalizing goroutine, so a TTL firing mid-Add lands in the same 500 path.
tryDistribution finalizes a ManagedQueue lookup failure for a leased flow (logged as "Invariant violation") as QueueOutcomeRejectedCapacity (controller/controller.go:353-359), surfacing an internal error as 429 with the Saturated dropped-reason header.
What you expected to happen:
Client-caused terminations (TTL, disconnect) map to the same status pre-admission as post-admission -- never 500. Note #2046 (PR #2047) proposes changing the post-admission queued-TTL mapping itself to 429 when the pool has endpoints; whichever way that lands, the pre-admission path must follow it. Internal invariant violations return 500 without a saturation header.
How to reproduce it (as minimally and precisely as possible):
Unit level: translateFlowControlOutcome(QueueOutcomeRejectedOther, fmt.Errorf("%w: %w", ErrRejected, ErrTTLExpired)) returns Internal (500). End to end: fill the enqueue channel (default buffer 100) so submissions block, then cancel a waiting request; the response is 500.
Anything else we need to know?:
Part of #1187; blocks enabling the gate by default. Under a saturation stall plus TTL storm, operators see a wall of 500s (and 500-based alerts) for what is actually backpressure.
Proposed fix: in the RejectedOther/EvictedOther case of the translator, branch on errors.Is(err, ErrTTLExpired) and errors.Is(err, ErrContextCancelled) before the Internal fallback, mapping each to the SAME status + dropped-reason header as its post-admission equivalent (EvictedTTL / EvictedContextCancelled) -- i.e. delegate to that mapping rather than hardcoding, so this stays correct whichever way #2046 / PR #2047 resolves the queued-TTL 429-vs-503 question. Change the tryDistribution lookup-failure finalization to QueueOutcomeRejectedOther with a non-sentinel error. The existing TestTranslateFlowControlOutcome table covers only RejectedOther+shutdown and +generic; add rows for the new branches first. Coordinate with #2046/#2047 (same function, overlapping tests).
Release note: pre-admission TTL/cancel no longer returns 500.
Environment:
- Kubernetes version: n/a (code analysis)
- llm-d-router version: main @
6cc1dac7
- Cloud provider or hardware configuration: n/a
- Install tools: n/a
- Others: n/a
What happened:
inferOutcome(pkg/epp/flowcontrol/controller/internal/item.go:174-198) maps TTL/cancel on a not-yet-queued item toQueueOutcomeRejectedOther, andtranslateFlowControlOutcomemaps non-shutdownRejectedOtherto 500 Internal (pkg/epp/requestcontrol/admission.go:238-243). "Not yet queued" includes items buffered in the enqueue channel and callers blocked inSubmitOrBlock-- exactly the overload conditions where TTL expiries peak. The same client behavior an instant later (item queued) yieldsEvictedTTL/EvictedContextCancelledandFinalizeinfers queued-ness from the item handle (item.go:132), whichmanagedQueue.Addsets with no lock shared with the finalizing goroutine, so a TTL firing mid-Add lands in the same 500 path.tryDistributionfinalizes a ManagedQueue lookup failure for a leased flow (logged as "Invariant violation") asQueueOutcomeRejectedCapacity(controller/controller.go:353-359), surfacing an internal error as 429 with theSaturateddropped-reason header.What you expected to happen:
Client-caused terminations (TTL, disconnect) map to the same status pre-admission as post-admission -- never 500. Note #2046 (PR #2047) proposes changing the post-admission queued-TTL mapping itself to 429 when the pool has endpoints; whichever way that lands, the pre-admission path must follow it. Internal invariant violations return 500 without a saturation header.
How to reproduce it (as minimally and precisely as possible):
Unit level:
translateFlowControlOutcome(QueueOutcomeRejectedOther, fmt.Errorf("%w: %w", ErrRejected, ErrTTLExpired))returns Internal (500). End to end: fill the enqueue channel (default buffer 100) so submissions block, then cancel a waiting request; the response is 500.Anything else we need to know?:
Part of #1187; blocks enabling the gate by default. Under a saturation stall plus TTL storm, operators see a wall of 500s (and 500-based alerts) for what is actually backpressure.
Proposed fix: in the
RejectedOther/EvictedOthercase of the translator, branch onerrors.Is(err, ErrTTLExpired)anderrors.Is(err, ErrContextCancelled)before the Internal fallback, mapping each to the SAME status + dropped-reason header as its post-admission equivalent (EvictedTTL/EvictedContextCancelled) -- i.e. delegate to that mapping rather than hardcoding, so this stays correct whichever way #2046 / PR #2047 resolves the queued-TTL 429-vs-503 question. Change thetryDistributionlookup-failure finalization toQueueOutcomeRejectedOtherwith a non-sentinel error. The existingTestTranslateFlowControlOutcometable covers only RejectedOther+shutdown and +generic; add rows for the new branches first. Coordinate with #2046/#2047 (same function, overlapping tests).Release note: pre-admission TTL/cancel no longer returns 500.
Environment:
6cc1dac7