Repository navigation
fix(inference,webapp): bound inference requests and stop abandoned generations - #237
Merged
Merged
Conversation
…nerations Webapp - Give the jlens stream an 8s headers deadline in pass 1, so a wedged pod is skipped in seconds instead of holding the request until maxDuration. A host that went silent is remembered as a fallback for the queueing pass, so a quiet fleet still gets served rather than failing. - Cap the non-streaming inference calls at 60s, applied through the openapi-fetch client's own fetch so every generated call inherits it, plus the two hand-written fetches in activations-server and similarity-matrix-pred. - Give the steer endpoints failover across hosts, which they had none of: one dead pod returned an error with no retry. They fail over on hard failures only and wait out silence, because with no fail-fast flag a quiet steer pod is usually queueing behind a healthy generation, and hopping off would abandon a pod that was about to answer. - Drop ATTEMPT_TIMEOUT_MS entries for services that never read them and resolveTwoHosts, which now has no callers. The inference entry was especially misleading: inference bypasses computeFetch, so nothing bounded those requests at all. - Bump smol-toml 1.6.1 -> 1.8.0 in the lockfile, clearing a high-severity advisory. Dev-only, transitive through knip, and within its existing range. Inference - Convert the three http middlewares to pure ASGI. BaseHTTPMiddleware runs the app in a task group, so Request.is_disconnected() below it probes an already cancelled scope and always answers False. Client disconnect was therefore never detected on any endpoint, and an abandoned jlens run computed to completion holding a slot and its VRAM. The CUDA probe now also runs after the response body finishes rather than before it streams. - Check for disconnect between steer frames too, so failover cannot orphan a generation. - Let fail_if_busy short-circuit the VRAM budget as well as the slot, so no pass-1 path can block behind a 300s wait. - Flush the gzip stream per frame. Starlette's GZipMiddleware buffers, so the streaming endpoints did not stream: every frame arrived at once when the response closed. Costs 1.6% more bytes and 2% more CPU at realistic frame size. No wire contract change: openapi.json regenerates byte-identical, and failIfBusy was already in the committed spec. Tests: 30 new, covering the disconnect probe through each middleware layer, per-frame gzip emission, fail_if_busy on the budget, and host selection, deadlines and failover on both jlens and steer. Co-authored-by: Cursor <cursoragent@cursor.com>
Steer had no `fail_if_busy`, so a pod holding its headers was indistinguishable from a wedged one: usually it was just queueing behind a healthy generation. That left the webapp guessing, and it guessed by waiting. Inference - Add `fail_if_busy` to both steer request models, matching the lens field. - Thread it through `with_request_lock` to all three gates it guards -- the limiter slot, the SAE residency reservation and the VRAM budget. All three, because one gate left blocking holds the connection for the full timeout and hides the other two. Read off the request with `getattr`, so an endpoint opts in by declaring the field and endpoints that never did keep queueing. - Answer it with a 429 from an app-level exception handler. The decorator raises from outside the handler, so a handler's own `except` cannot see it. Same body as the lens endpoint returns, since one client reads both. Webapp - `streamWithFailover` now runs the same two passes as jlens: refuse-rather-than-queue across every host, then queue on the most promising one. Steer inherits the 8s deadline and drops its special case, and `lensPromptStream` collapses onto the shared helper rather than keeping its own copy. - The two non-streaming chat calls pass `failIfBusy: false` explicitly. They have no failover, so queueing is the only way for them to be served. Wire change, backward compatible: an old client omits the field and gets today's queueing behaviour. Regenerated `openapi.json` and `inference.d.ts` accordingly. Tests: 11 new. The decorator refusing at each gate, the default still queueing, an endpoint without the field unaffected, and on the webapp side that pass 1 sends the flag, that a silent host is dropped at the deadline, and that the queueing pass carries no deadline at all. Co-authored-by: Cursor <cursoragent@cursor.com>
…tream A headers deadline assumes headers arrive before the work does. For a reply that lands in one piece they arrive when the work is finished, so the deadline stops measuring the host's health and starts measuring how long the answer took -- aborting every request slower than it. Two callers were wrong: - `steerCompletion(stream: false)`, which `/api/steer` uses, would abort any completion longer than 8s, try the other hosts, and only succeed on the queueing pass -- having left a generation running on each host it gave up on. - `getAttentionForHead`, which is not a stream at all and only uses the streaming helper because the endpoint is missing from the typed client. It resolves a single host, so there was no failover to save it: an 8s abort was an outright failure where it used to succeed. Both now bound the call as a whole with INFERENCE_REQUEST_TIMEOUT_MS instead, which is what the other non-streaming calls got. Steer keeps `failIfBusy` on pass 1, so it still refuses and fails over quickly -- the fast refusal is what the deadline was standing in for, and the explicit protocol does the job better. `postInferenceStreaming` now defaults to no headers deadline, so the deadline is opt-in for callers that know their reply streams. Every current caller passes it explicitly. The old default made a wrong deadline the silent outcome of using the helper, which is how both of these happened. Tests: 3 new, pinning that a slow non-streamed completion is not abandoned, that it still asks pass 1 to refuse, and that the whole call stays bounded. Co-authored-by: Cursor <cursoragent@cursor.com>
hijohnnylin
marked this pull request as ready for review
September 11, 2026 04:45
…are rolled A pod that predates `fail_if_busy` on steer ignores the field, so a saturated one queues silently instead of refusing. The deadline cannot tell that apart from a wedged pod, so it fires, pass 1 walks every host, and each abandoned attempt leaves a generation running on a server that cannot yet notice the client left. One request becomes N+1 generations, doubled again for chat, exactly when the fleet is busy. Waiting gives up little. Against an upgraded pod the 429 does the real work: busy is answered in milliseconds and pass 1 moves on without the deadline being involved. The deadline only adds the wedged-pod case, and the route's `maxDuration` still bounds that. So `STEER_HEADERS_TIMEOUT_MS` is 0 for now, with the comment saying to set it to HEADERS_TIMEOUT_MS once every pod serving steer honours the flag. This lets the webapp deploy ahead of the pods: jlens gets its whole improvement, since `fail_if_busy` already works there for the request slot, and steer behaves as it does today. jlens is unchanged. Non-streaming steer and the attention call already carry no headers deadline. Tests: the steer deadline test becomes its opposite, plus one pinning that a busy pod is still left immediately, which is the part that does not depend on the deadline. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Nothing bounded an inference request.
ATTEMPT_TIMEOUT_MS[INFERENCE]incompute-host.tslookedlike it did, but inference deliberately bypasses
computeFetchto keep itsReadableStreamintact, so that entry was dead code. The real limits were undici's defaults — 300s for response
headers — against a route that dies at
maxDuration = 180. A wedged pod therefore took the wholerequest down at 180s and failover never happened, even with healthy pods sitting idle.
Fixing that alone would have made things worse, because of a second problem found on the way:
client disconnect was never detected on any endpoint. So this changes both sides.
Webapp
instead of holding the request. A host that merely went silent is remembered as a fallback for
the queueing pass, so a quiet fleet still gets served rather than failing.
fetchso every generated call inherits it, plus the two hand-written fetches in
activations-serverand
similarity-matrix-pred.retry. Steer now runs the same two passes as jlens — refuse-rather-than-queue across every host,
then queue on the most promising one — sharing one helper rather than keeping two copies of it.
fail_if_busyflag for steer, which is what makes the above safe. Without it a quiet steerpod is indistinguishable from a wedged one, since it is usually just queueing behind a healthy
generation, and a deadline would fire mostly on pods that were about to answer.
ATTEMPT_TIMEOUT_MSentries andresolveTwoHosts, which has no callersleft.
smol-toml1.6.1 → 1.8.0 in the lockfile, clearing a high-severity advisory.Dev-only, transitive through
knip, within its existing range.Inference
BaseHTTPMiddlewareruns the app in a taskgroup, so
Request.is_disconnected()below it probes an already-cancelled anyio scope and alwaysanswers
False. One layer is enough to break it and there were three. An abandoned jlens runtherefore computed the entire generation, holding its concurrency slot and VRAM reservation
while writing into a dead socket. The plumbing to stop was already correct on vLLM; only the
detection was broken. The CUDA probe now also runs after the response body finishes rather than
before it streams.
generation.
fail_if_busynow short-circuits the VRAM budget, not just the slot.is_busy()on thevLLM semaphore is only true at full saturation, so a partially loaded pod sailed past the busy
check and could then sit on the VRAM wait for minutes — indistinguishable from wedged. Every
pass-1 path is now non-blocking, which is what makes 8s a safe number.
GZipMiddlewarebuffers, so the streamingendpoints did not stream: a real uvicorn + httpx test showed all 20 frames arriving together at
1.01s, and 0.01s → 0.97s after the fix. Measured cost is 1.6% more bytes and 2% more CPU at
realistic frame size.
Compatibility
One wire change, backward compatible.
fail_if_busyis new on the two steer request models.An old client omits it and gets today's queueing behaviour, so an upgraded pod serves an old webapp
unchanged.
openapi.jsonandinference.d.tsare regenerated accordingly. Nothing else in thespec moves.
The webapp can deploy ahead of the pods.
STEER_HEADERS_TIMEOUT_MSis 0 for now, so steer'spass 1 does not hop off a quiet host. A pod predating
fail_if_busyon steer ignores the field andqueues silently rather than refusing, which is indistinguishable from wedged, so a deadline would
fire on a busy fleet and leave a generation running on every host it passed — N+1 generations for
one request, doubled again for chat. Little is given up: against an upgraded pod the 429 does the
real work, and the wedged-pod case the deadline adds is still bounded by the route's
maxDuration.Set it to
HEADERS_TIMEOUT_MSonce every pod serving steer honours the flag.jlens needs no such gate, because its
fail_if_busyalready works for the request slot on today'spods: a busy pod refuses in milliseconds and costs nothing. Only a pod with a free slot but no free
VRAM behaves differently, orphaning a few generations where it previously orphaned one, since the
memory wait does not honour the flag until the server change lands. That is self-limiting — each
orphan holds a slot, so the pod starts refusing rather than accepting more.
Non-streaming steer and the attention call carry no headers deadline at all, so neither amplifies.
Tests
41 new, covering the disconnect probe through each middleware layer, per-frame gzip emission,
fail_if_busyat each of the three admission gates, and host selection, deadlines and both failoverpasses on jlens and steer.
One of them caught a real bug in this branch: passing
undefinedfor the queueing pass's deadlinefell through
??to the 8s default, so the attempt meant to wait was still bounded.Gates run locally:
make python-lint,uv run pyright ., 709 inference tests,make openapi-check,npm run lint,npm run format:check, 133 webapp tests.Still open, not in this PR
failure as a success, including a vector download that writes a file containing the string
undefined. Deferred deliberately.maplibre-glXSS advisory onmain, unrelated to this branch.