[Defilan]

What

A request whose ModelPool member never became resident within swapBudget now always gets 503 with Retry-After, whichever of the two timers fires first. Before this change the client got 502 all_backends_failed whenever the background swap's deadline beat the request's hold, which well-behaved clients read as an upstream outage rather than as something to retry.

Why

Since #2135 the background swap (startSwap, resolveSwapDeadline) and the request's hold (dispatchWithFallback, resolveSwapBudget) are bounded by the same duration, so both timers expire at the same instant. When the hold won, Acquire returned ErrHoldBudgetExceeded and the proxy answered 503 + Retry-After. When the swap won, it stored a plain errors.New in swapErr, the proxy did not recognise it, and the request fell through to the generic 502 all_backends_failed. That nondeterminism is also why TestProxyPoolActivationTimeout503RetryAfter failed intermittently on main and on unrelated PRs.

docs/operations/gpu-sharing.md already documented the intended contract: "If the budget elapses before the target is resident, the held request fails with 503 and a Retry-After". The code was the outlier, so this makes the code match the documented and RFC 9110 (15.6.4, 10.2.3) behavior.

Fixes #2167

How

Four commits on fix/modelpool-swap-past-swapbudget-races-2167.

Commit	What it does	Files
3bba92b2d fix(router): answer a swap abandoned at swapBudget with 503 + Retry-After	Adds ErrSwapDeadlineExceeded, wraps the abandoned-swap error with it, and maps it alongside ErrHoldBudgetExceeded to the existing 503 + Retry-After branch	internal/router/activator.go, internal/router/proxy.go, internal/router/activator_test.go, internal/router/proxy_test.go, internal/router/proxy_2167_test.go (new), docs/operations/gpu-sharing.md
fbf35f3e8 test(router): make the abandoned-swap audit assertion bite	Review round 1. Requires an audit line attributable to the abandoned-swap request itself	internal/router/proxy_2167_test.go
5ec5d88b7 test(router): correct the slog timestamp comment	Fixes a false claim about the timestamp format in a test helper comment	internal/router/proxy_2167_test.go
1f80b0fb0 test(router): assert the abandoned swap returns ErrSwapDeadlineExceeded	Asserts the sentinel directly at the activator level, tightens the error text to "member X not resident after Y", and lists swap failures among Acquire's errors	internal/router/activator.go, internal/router/activator_test.go

@Tanguille found and fixed the same race independently in #2184. The activator-level errors.Is assertion, the error wording and the Acquire doc line in 1f80b0fb0 come from that PR. Thanks!

The design decisions, each with the source that settled it and the test that pins it:

• The abandoned-swap error is a sentinel rather than reusing ErrHoldBudgetExceeded, so a swap deadline stays distinguishable from a hold expiry and the existing classification in Activator is untouched.
• Both timers keep their current duration. fix(modelpool): stop swap thrash on reversals, idle-probe blindness, and abandoned loads (#2130) #2135's coupling is deliberate and the issue asks for order-independence, not a new winner, so neither the hold nor the swap deadline was reordered or given a grace period.
• A swap that fails for a non-deadline reason, for example the controller rejecting the activation, keeps its 502 and its reason="error" metric. That is an error, not a transient capacity condition.
• The audit token for this case stays pool_activation_timeout, shared with the hold path, and Retry-After keeps its existing value.
• Pools with no swapBudget are unchanged: the hold is the response-header timeout while the swap bound stays twice the default, so the hold always ends that request first.
• The abandoned swap still scales its target back down and still counts llmkube_modelpool_swap_failures_total{reason="deadline"}, and a later request still starts a fresh load.

The new test file internal/router/proxy_2167_test.go reproduces the reported topology (a member that never becomes Ready) and forces the abandonment to happen first, so it does not depend on timer ordering. On base it fails with status = 502, want 503. TestProxyPoolActivationTimeout503RetryAfter passed 500 runs under -race -cpu=1,2.

Two things worth naming for a reviewer, neither changed here. With more than one backend in a route, dispatchWithFallback returns the last backend's error, so a mixed route reports that backend's result rather than the pool one; the same is true on base for the hold class. And a failClosed rule whose swap is abandoned now reaches the pool 503 branch before the fail_closed_runtime branch, so its audit token and llmkube_router_fail_closed_total accounting move while the status stays 503 and no data egresses.

AI assistance

Assisted-by: DeepSeek-V4.1-Flash (deepseek-ai/DeepSeek-V4.1-Flash, vllm runtime, self-hosted with LLMKube on 3x NVIDIA GB10; roles: planning, code, tests and commit messages, executing review: base-vs-head probes and mutation checks)
Assisted-by: qwen38-flash-next (unsloth/Qwen3.8-Flash-Next-GGUF UD-Q3_K_XL, generic (llmkube-gufo-rocm-gfx1151) runtime, self-hosted with LLMKube on AMD Strix Halo (Ryzen AI Max, Radeon iGPU); roles: read-only review)

Assisted-by: Claude Opus 5.5 (Claude Code; roles: comparing this PR with #2184, the 1f80b0fb0 follow-up commit and this PR text update)

Verified by the author: Supervised the build and read diffs

Checklist

• Tests added/updated
• make test passes locally
• make lint passes locally
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance (if any) is disclosed above, per CONTRIBUTING.md
• Documentation updated (if user-facing change)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!