chore(server): set uvicorn's keep-alive explicitly instead of inheriting 5s by wanghy73 · Pull Request #1561 · MemMachine/MemMachine · GitHub
Skip to content

chore(server): set uvicorn's keep-alive explicitly instead of inheriting 5s - #1561

Draft
wanghy73 wants to merge 1 commit into
MemMachine:speedkickfrom
wanghy73:fix/uvicorn-keepalive
Draft

chore(server): set uvicorn's keep-alive explicitly instead of inheriting 5s#1561
wanghy73 wants to merge 1 commit into
MemMachine:speedkickfrom
wanghy73:fix/uvicorn-keepalive

Conversation

@wanghy73

@wanghy73 wanghy73 commented Aug 31, 2026

Copy link
Copy Markdown

What

uvicorn defaults timeout_keep_alive to 5 seconds and app.py never set it. This sets it explicitly to 120 s, overridable via MEMMACHINE_KEEP_ALIVE_SECONDS.

Why

A server keep-alive shorter than its clients' idle timeouts is a known hazard. Every client that fronts this server pools idle connections for longer than 5 s — Go's transport for 90 s, httpx for 5 s. A client can reuse a connection the server has just closed, and because POST is not idempotent it will not retry: it waits out its own timeout instead.

Setting the value above the largest client idle timeout in play removes that hazard.

What this does NOT do

It does not fix the 30-second stalls seen on this deployment, and should not be merged as though it does. Raising the value removed them from one load arm and not from another on the same image:

arm before after
128 users, no think-time 242 stalls 0
100 users, 2 s / 5 s think-times 32 stalls 124

The change was verified live inside the running container in both cases (KEEP_ALIVE_SECONDS = 120, timeout_keep_alive present in start_http), so this is not a deployment mistake.

The two arms differ in think-times as well as in this setting, so the arm that showed zero may simply never have left a connection idle long enough to trigger the fault. The cause is unknown and the stall is still present at 0.4% of requests.

An earlier version of this PR claimed the first result as a confirmed fix. That was drawn from a single arm and was wrong.

The stall itself, for whoever picks it up

Still unexplained, and worth more than its rate suggests — each stall occupies a client slot for the full 30 s, which consumed roughly 47% of capacity in the 128-user arm.

  • Requests hang for exactly 30.00 s (spread under 35 ms across 71 samples), matching restyTimeout in MemMachine-Platform internal/memmachine/core/client.go
  • They never reach the application — over the same period the core completed 80,051 requests, all 200, none slower than 12,098 ms, zero above 30 s
  • Only POSTs are affected; /health GETs stay green at ~1 ms throughout
  • Neither end raises an error, which is why it went unnoticed

Note that AccessLogMiddleware logs after call_next returns, so a missing log line alone does not prove a request never arrived — the duration histogram is what rules out an internal stall.

Not measured

Holding connections open 24× longer keeps more sockets and file descriptors alive per idle client. Immaterial at the concurrency tested here; unknown at larger client counts.

ruff check and ruff format clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Nr9kacmpFVTTfkZRw6esxP

@wanghy73
wanghy73 marked this pull request as draft August 31, 2026 23:58
@wanghy73

Copy link
Copy Markdown
Author

…ing 5s

uvicorn defaults timeout_keep_alive to 5 seconds and app.py never set it,
which is shorter than the idle timeout of every client that fronts this
server: Go's transport pools idle connections for 90s, httpx for 5s. A
server keep-alive shorter than its clients' is a known hazard - a client can
reuse a connection the server has just closed, and because POST is not
idempotent it will not retry, so it waits out its own timeout instead.

Setting it explicitly, above the largest client idle timeout in play,
removes that hazard. MEMMACHINE_KEEP_ALIVE_SECONDS overrides it.

This does NOT fix the 30-second stalls observed on this deployment and must
not be merged as though it does. Raising the value removed them from one
load arm and not from another on the same image:

  128 users, no think-time      242 stalls -> 0
  100 users, 2s/5s think-times   32 stalls -> 124

The change was verified live in the running container for both, so that is
not a deployment mistake. The two arms differ in think-times as well as in
this setting, so the arm that showed zero may simply never have left a
connection idle long enough to trigger the fault. The cause is unknown, and
the stall is still present at 0.4% of requests.

An earlier version of this commit claimed the first result as a confirmed
fix. That was drawn from a single arm and was wrong.

Not measured: holding connections open 24x longer keeps more sockets and
file descriptors alive per idle client. Immaterial at the concurrency tested
here, unknown at larger client counts.

ruff check and ruff format clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nr9kacmpFVTTfkZRw6esxP
@wanghy73
wanghy73 force-pushed the fix/uvicorn-keepalive branch from 9b23fc8 to db6f87b Compare September 1, 2026 00:16
@wanghy73 wanghy73 changed the title fix(server): outlive the client's idle connection timeout chore(server): set uvicorn's keep-alive explicitly instead of inheriting 5s Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant