fix(proxy): stop capping every generation at aiohttp's 300s default - #132
Merged
Conversation
The inference session was built as a bare aiohttp.ClientSession(), which inherits ClientTimeout(total=300). `total` spans the whole exchange — including every streamed chunk — so the gateway aborted any generation past five minutes no matter how healthy the upstream was. Reported by the evals team: benchmarks whose tasks carry a large max_gen_toks time out, and since lm-eval's openai backend doesn't stream, one request covers the entire completion. Pass the timeout explicitly instead, shaped to how each mode reports progress. Streaming gets sock_read and no total: chunks arrive continuously, so a gap means the upstream died while elapsed time means only that the answer is long. Non-streaming has no such signal — nothing comes back until the completion is finished — so a generous overall cap is all that's available. Both stay configurable, and sock_connect stays at 30s so an unreachable upstream still fails fast. Note for deploy: nginx caps reads at proxy-read-timeout 600 on the prod ingress, which non-streaming requests will now hit first. That needs raising alongside this. 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.
The bug
The inference proxy built its session as a bare
aiohttp.ClientSession():That inherits aiohttp's default
ClientTimeout(total=300).totalcovers the whole exchange, including every streamed chunk, so the gateway has been aborting any generation that runs past five minutes — no matter how healthy the upstream is. Nothing in the code says 300 anywhere; it's an inherited default, which is why it wasn't obvious.How it surfaced
The evals team hit it right after we lifted their rate limit: benchmarks whose tasks carry a large
max_gen_toksfail with request timeouts. Their harness (lm-eval via--backend openai,local-completions) does not stream, so a single HTTP request spans the entire completion. A long reasoning answer on a 70B model clears 300s easily.It isn't evals-specific, though — the same ceiling truncates any long chat answer, streaming included, since
totaldoesn't care that chunks are still flowing.The fix
Pass the timeout explicitly, shaped to how each mode reports progress:
sock_read, nototal. Chunks arrive continuously, so a gap means the upstream died, whereas elapsed time means only that the answer is long. Bounding the gap lets a legitimately long generation finish while still reaping dead connections.totalis the only bound available.Both are configurable (
UPSTREAM_TIMEOUT_SECONDS, default 3600;UPSTREAM_STREAM_STALL_SECONDS, default 300).sock_connectstays at 30s so an unreachable upstream still fails fast rather than hanging for an hour.Deploy dependency
The prod ingress sets
proxy-read-timeout: "600", which non-streaming requests will now hit before the new cap. That needs raising in the same window or this only moves the ceiling from 5 minutes to 10 — breithorn-poc/rob-poc PR to follow.Test plan
test_upstream_timeout.pycovers both modes, including a regression guard that neither path can inherittotal=300againmax_tokensthat runs >5 min returns a result instead of an errorMade with Cursor