fix(witan): stop dropping in-flight requests 2s into a shutdown - #241
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a configurable Uvicorn graceful-shutdown timeout to prevent interrupted in-flight Witan requests.
Changes:
- Defaults HTTP shutdown grace to 120 seconds.
- Adds forwarding and stdio-isolation tests.
- Prepares release metadata, which now requires rebasing to 0.15.0.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates the workspace package version. |
mcp/servers/witan/witan/cli/__init__.py |
Configures graceful HTTP shutdown. |
mcp/servers/witan/tests/test_cli.py |
Tests timeout forwarding and stdio behavior. |
mcp/servers/witan/pyproject.toml |
Updates package release metadata. |
mcp/servers/witan/CHANGELOG.md |
Documents the shutdown fix. |
Suppressed comments (1)
mcp/servers/witan/pyproject.toml:99
- This bumpversion value now collides with the 0.14.0 release merged in PR #240. Rebase and advance it to 0.15.0 together with the project version, changelog, and lockfile so future bumps start from the actual latest release.
current_version = "0.14.0"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
FastMCP builds its uvicorn config with a hardcoded timeout_graceful_shutdown of 2 seconds, so on SIGTERM the server stopped accepting connections, gave running requests two seconds, and dropped the rest. A witan write has been measured at 27s under load, so every one in flight was severed by a deploy, an eviction or a node drain — and a severed write is exactly the indeterminate outcome the caller cannot safely retry. serve() now passes uvicorn_config through, defaulting to 120s to match the request budget the deployment enforces at its gateway, and settable via --shutdown-grace-seconds / WITAN_MCP_SHUTDOWN_GRACE_SECONDS. This could not be fixed from the deployment side. ol-infrastructure sets terminationGracePeriodSeconds: 150 so the kubelet waits, but uvicorn declined to use it — the pod-side setting bought time nothing spent, while looking like the whole fix. Found while investigating FastMCP's concurrency knobs, not from a failure report. Verified against the real library rather than the test double: with uvicorn.Config instrumented, 120.0 arrives where fastmcp's 2 would have been. stdio runs pass no uvicorn config at all and are unaffected. Releases witan-council 0.14.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JYb9sMjetD9Nxjf24Aw3m5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JYb9sMjetD9Nxjf24Aw3m5
blarghmatey
force-pushed
the
witan-graceful-shutdown
branch
from
August 16, 2026 21:20
2b19756 to
9885b0d
Compare
Review caught that the serve flag table enumerates every public option and env var, and this PR added one without updating it. The table is the canonical reference, so a missing row is a stale doc rather than an omission nobody sees. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JYb9sMjetD9Nxjf24Aw3m5
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.
What are the relevant tickets?
N/A — no GitHub issue. The missing half of the shutdown safety asserted in
ol-infrastructure#5448;
that PR's comment has been corrected in
#5449.
Description (What does it do?)
witan servenow passesuvicorn_config={"timeout_graceful_shutdown": …},defaulting to 120s, settable via
--shutdown-grace-seconds/WITAN_MCP_SHUTDOWN_GRACE_SECONDS.witan-council0.15.0.FastMCP hardcodes a 2-second graceful shutdown. From
run_http_async(fastmcp 4.0.0b2):
So on SIGTERM uvicorn stops accepting connections, gives in-flight requests two
seconds, and drops the rest. A witan write has been measured at 27.1s under
16 concurrent writers, so every one in flight was severed by a deploy, an
eviction or a node drain — and a severed write is precisely the indeterminate
outcome a caller cannot safely retry, which is the failure class this deployment
has spent weeks removing elsewhere.
★ Why the deployment could not fix this alone
ol-infrastructure sets
terminationGracePeriodSeconds: 150so the kubelet iswilling to wait. uvicorn declined to use it. The pod-side setting bought
time that nothing spent, while looking like the entire fix — the comment there
asserted, in as many words, that "uvicorn shuts down gracefully on SIGTERM: it
stops accepting connections and waits for in-flight requests rather than
dropping them." That was wrong as configured. Both halves are required, and the
half that looks sufficient is the one that isn't.
Found while investigating FastMCP's concurrency knobs, not from a failure
report — so the window it closes has been open the whole time the 150s grace
period has been in place.
How can this be tested?
Three new tests cover the override, its tunability, and that stdio passes no
uvicorn config at all.
The committed tests use the existing
_FakeMCPdouble, so they prove thekwarg is forwarded and nothing more. If FastMCP rejected or ignored it, witan
would either fail to start or silently keep the 2s default — neither of which a
fake can catch. So that was verified separately against the real library, with
uvicorn.Configinstrumented to capture its kwargs:Additional Context
Why 120s. It matches the request budget the deployment enforces at APISIX
(
WITAN_REQUEST_TIMEOUT), on the principle that shutdown should be willing towait as long as a request was allowed to take. Anything past that budget is
being cut off upstream anyway. The deployment's 150s termination grace sits
above it, so the kubelet remains the outer bound.
Local runs are unaffected. stdio never reaches this code path, and a local
HTTP run has nothing in flight worth waiting for — so the default is safe
everywhere rather than only in the cluster.
Rebased onto #240, which merged first and took
witan-council0.14.0. Thisis now 0.15.0. The CHANGELOG conflict was resolved by keeping #240's 0.14.0
section intact and adding 0.15.0 above it — verified the
SessionLifefloor-bumpentry and the
witan-core>=0.21pin both survived. Full suite re-run after therebase: 820 passed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JYb9sMjetD9Nxjf24Aw3m5