Keep nginx connections alive across agentic replay idle gaps - #283
Open
weireweire wants to merge 1 commit into
Open
Keep nginx connections alive across agentic replay idle gaps#283weireweire wants to merge 1 commit into
weireweire wants to merge 1 commit into
Conversation
Root cause: the generated nginx.conf sets no keepalive directives, so the client-facing keepalive_timeout falls back to nginx's default of 75s and the upstream is spoken to over HTTP/1.0 (a fresh TCP connection per request). Agentic replay reproduces recorded think-time, so a session's connection routinely sits idle for minutes between turns. nginx closes it at 75s while the benchmark client still holds it pooled; the client's next write on that socket surfaces as ClientOSError(32, 'Broken pipe') or ServerDisconnectedError. Nothing is logged server-side, because closing an idle keepalive connection is not an error from nginx's point of view. The failure is disproportionate for long replays: a single such error on a warmup request aborts the whole run, discarding hours of warmup on large topologies. Fix: render keepalive_timeout (new frontend.nginx_keepalive_timeout, default 3600s) for both the client-facing server and the upstream block, enable an upstream connection pool (keepalive 512), and add the proxy_http_version 1.1 / empty Connection header pair the pool requires. Validation: 12P4D DSV4-Pro FP4 agentic replay at concurrency 2048 (64 GPU, 22673 warmup requests). Three runs on the old template aborted mid-warmup on a single broken-pipe/server-disconnected error at 85-93% completion with zero prior errors; the run on the new template reached 99% of warmup with zero transport errors. New tests/test_nginx_conf.py covers the rendered directives and the timeout override. Pre-existing lint (127) and test (5) failures on main are unchanged.
weireweire
requested review from
alec-flowers,
csahithi,
ishandhanani and
nlevin-ui
as code owners
July 31, 2026 11:04
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
=======================================
Coverage ? 67.99%
=======================================
Files ? 70
Lines ? 9305
Branches ? 0
=======================================
Hits ? 6327
Misses ? 2978
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
Problem
The generated
nginx.confsets no keepalive directives, so two defaults apply:keepalive_timeoutfalls back to nginx's default of 75sAgentic replay reproduces recorded think-time, so a session's connection routinely sits idle for minutes between turns. nginx closes it at 75s while the benchmark client still holds it pooled, and the client's next write on that socket surfaces as
ClientOSError(32, 'Broken pipe')orServerDisconnectedError. Nothing is logged server-side — closing an idle keepalive connection is not an error from nginx's point of view, which makes this hard to attribute.The consequence is disproportionate for long replays: a single such error on a warmup request aborts the entire run, discarding hours of warmup on large topologies.
Fix
keepalive_timeoutfor both the client-facingserverand theupstreamblock, from a newfrontend.nginx_keepalive_timeout(default3600s)keepalive 512proxy_http_version 1.1/ emptyConnectionheader pair the pool requires — without them nginx keeps using HTTP/1.0 and the pool is a no-opSet
nginx_keepalive_timeout: "75s"to restore the previous behaviour.Validation
DeepSeek-V4-Pro FP4 agentic replay, 12 prefill x 4 decode (64 GPU), concurrency 2048, 22673 warmup requests:
The completed run also matches the one pre-fix run that survived, so the change
removes the transport errors without perturbing the measurement: 61581 vs 61033
tokens/s/GPU (0.9%), TTFT 37.3s vs 37.9s.
tests/test_nginx_conf.py(new) covers the rendered directives and the timeout override.Pre-existing
make lint(127) andmake test(5) failures onmainare unchanged by this PR.