fix(gunicorn): warn at startup when GUNICORN_WORKERS>1 breaks MCP sessions
MCP sessions live in a per-process in-memory dict on whatever worker
handled the initialize request (StreamableHTTPSessionManager._server_instances).
gunicorn round-robins HTTP requests across workers, so the same
mcp-session-id frequently lands on a different worker and returns
'Session not found' / 'Invalid or expired session ID'. fastapi-mcp
hardcodes stateless=False, and gunicorn has no built-in sticky-session
support, so there is no in-process workaround.
This change keeps the existing workers=2 default (the project does not
want to give up the modest parallelism it gives) but adds:
* A comment block above workers= explaining the constraint, the
workers=1 fix, and the nginx sticky-session escape hatch.
* An on_starting(server) hook that logs a WARNING whenever
workers > 1, naming the symptom and the fix so the misconfig is
loud at deploy time rather than silent at request time.
* Three regression tests in tests/unit/test_gunicorn_config.py
asserting the hook exists, is silent for workers=1, and warns
(with the expected text) for workers>1.
* A 'MCP session affinity' note in CLAUDE.md key conventions so
the constraint is recorded for future maintainers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -85,6 +85,7 @@ docs/superpowers/plans/ # implementation plans
|
||||
- **Exception handlers in `server.py`:** `KeyError` → 404, `ValueError` → 400, Pydantic validation → 422. Use the existing handlers — don't add try/except in route bodies.
|
||||
- **Tool functions raise `KeyError` for "unknown id" and `ValueError` for invalid state transitions** (e.g. confirming a CANCELLED pending). The handlers translate these to clean HTTP statuses.
|
||||
- **The two-step submit flow is core, not optional.** `prepare_submit_job` snapshots the connection's `master` / `deploy_mode` / `spark_conf` into the `PendingSubmission`; `confirm_submit_job` is the only place `spark-submit` is invoked. Editing a connection between prepare and confirm does **not** retarget the pending job.
|
||||
- **MCP session affinity: keep `GUNICORN_WORKERS=1` (or front with a sticky-session LB).** The `mcp` library stores each session in a per-process dict (`StreamableHTTPSessionManager._server_instances`); gunicorn round-robins requests across workers, so a multi-worker deploy returns "Session not found" / "Invalid or expired session ID" for the same `mcp-session-id` whenever it lands on a worker that didn't create it. `fastapi-mcp` hardcodes `stateless=False`, so there is no in-process workaround. The `on_starting` hook in `gunicorn.conf.py` logs a WARNING whenever `workers > 1` so the misconfig is loud, not silent. See the comment block above `workers =` in `gunicorn.conf.py` for full context and the nginx-sticky escape hatch.
|
||||
- **Test fixtures rebind module-level singletons** (`connections.store`, `submit.conn_store`, `submit.pending_store`) in `monkeypatch.setattr` because the tool modules captured the originals at import time. See `tests/integration/test_mcp_routes.py` for the pattern.
|
||||
- **86 tests pass** as of the last Stage 1 cleanup; run `uv run pytest` after any change.
|
||||
|
||||
|
||||
@@ -22,11 +22,52 @@ bind = os.environ.get("GUNICORN_BIND", "0.0.0.0:8000")
|
||||
# 2 is a sensible default for a small containerized MCP server: lets a slow
|
||||
# yarn logs request run in parallel with a status check. Scale up by
|
||||
# setting GUNICORN_WORKERS at deploy time.
|
||||
#
|
||||
# ⚠️ MCP SESSION AFFINITY CONSTRAINT ⚠️
|
||||
# ----------------------------------------
|
||||
# The official `mcp` library (used under fastapi-mcp) keeps each MCP
|
||||
# session's state in a per-process in-memory dict
|
||||
# (`StreamableHTTPSessionManager._server_instances`). gunicorn round-robins
|
||||
# HTTP requests across worker processes, so a client that does:
|
||||
# 1) POST /initialize → worker A creates session_id=xyz
|
||||
# 2) POST /tools/list → worker B has no record of xyz
|
||||
# 3) → "Session not found" / "Invalid or expired session ID"
|
||||
# gunicorn has no built-in sticky-session support, and fastapi-mcp hardcodes
|
||||
# `stateless=False`, so there is no in-process way to share sessions across
|
||||
# workers without an external store (Redis, etc.).
|
||||
#
|
||||
# Trade-off: `workers=1` eliminates the bug entirely. A single uvicorn-based
|
||||
# worker still handles many concurrent connections via the asyncio event
|
||||
# loop — the lost capacity is parallel CPU work, which this I/O-bound MCP
|
||||
# service rarely needs. If you ever need true multi-core parallelism, you
|
||||
# MUST front gunicorn with a sticky-session reverse proxy (nginx
|
||||
# `ip_hash` / `hash $mcp_session_id consistent`) AND give every session
|
||||
# a shared backing store — do NOT just bump GUNICORN_WORKERS.
|
||||
#
|
||||
# The `on_starting` hook below makes this footgun loud rather than silent.
|
||||
workers = int(os.environ.get("GUNICORN_WORKERS", "2"))
|
||||
worker_class = "uvicorn.workers.UvicornWorker"
|
||||
# 1 thread per worker is enough for ASGI handlers (no blocking I/O).
|
||||
threads = int(os.environ.get("GUNICORN_THREADS", "1"))
|
||||
|
||||
|
||||
def on_starting(server):
|
||||
"""Master-process hook: gunicorn calls this once before forking workers.
|
||||
|
||||
We use it to scream at anyone who raises GUNICORN_WORKERS above 1 without
|
||||
putting a sticky-session LB in front — see the workers= comment above for
|
||||
why MCP sessions cannot survive worker-local storage across processes.
|
||||
"""
|
||||
if workers > 1:
|
||||
server.log.warning(
|
||||
"GUNICORN_WORKERS=%d — MCP sessions are stored in per-process "
|
||||
"memory, so the same mcp-session-id will frequently land on a "
|
||||
"different worker and return 'Session not found'. Either set "
|
||||
"GUNICORN_WORKERS=1 or front this service with a sticky-session "
|
||||
"reverse proxy (see gunicorn.conf.py for details).",
|
||||
workers,
|
||||
)
|
||||
|
||||
# --- Lifecycle ---
|
||||
# Generous timeout because the slowest tool call here is yarn logs (which
|
||||
# can take 30+ seconds on a busy cluster). uvicorn standalone defaults to
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
# coding=utf-8
|
||||
"""
|
||||
@Time :2026/6/24
|
||||
@Author :tao.chen
|
||||
|
||||
Regression tests for gunicorn.conf.py.
|
||||
|
||||
The MCP session-affinity constraint is documented in gunicorn.conf.py: the
|
||||
mcp library stores each session in a per-process dict, so a multi-worker
|
||||
gunicorn deploy returns "Session not found" ~50% of the time. The fix in
|
||||
this config is an `on_starting` hook that warns loudly when workers > 1.
|
||||
|
||||
These tests assert that:
|
||||
1. The hook exists and is wired up.
|
||||
2. workers == 1 is silent (no false alarms in the default single-worker
|
||||
mode people use after reading the comment).
|
||||
3. workers > 1 emits a WARNING that mentions the symptom and the fix.
|
||||
|
||||
If anyone ever "cleans up" the hook, these tests fail — that's the point.
|
||||
"""
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
GUNICORN_CONF = REPO_ROOT / "gunicorn.conf.py"
|
||||
|
||||
|
||||
def _load_gunicorn_conf():
|
||||
"""Import gunicorn.conf.py as a module (not a package member).
|
||||
|
||||
The file lives at the repo root and is loaded by gunicorn via exec();
|
||||
we use importlib so the test can introspect `on_starting` and mutate
|
||||
`workers` between cases.
|
||||
"""
|
||||
spec = importlib.util.spec_from_file_location("gunicorn_conf_under_test", GUNICORN_CONF)
|
||||
assert spec is not None and spec.loader is not None, f"cannot load {GUNICORN_CONF}"
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
# Make sure repo root is on sys.path so the module's `import os` etc. work.
|
||||
if str(REPO_ROOT) not in sys.path:
|
||||
sys.path.insert(0, str(REPO_ROOT))
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
class _FakeServer:
|
||||
"""Minimal stand-in for gunicorn's `server` argument to on_starting.
|
||||
|
||||
gunicorn passes a `gunicorn.arbiter.Arbiter`-like object that exposes
|
||||
`server.log.warning(...)`. We only need the .log.warning sink.
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
self.warnings: list[str] = []
|
||||
|
||||
class _Log:
|
||||
def __init__(self, sink):
|
||||
self._sink = sink
|
||||
|
||||
def warning(self, fmt: str, *args) -> None: # noqa: A002 - matches gunicorn API
|
||||
self._sink.append(fmt % args if args else fmt)
|
||||
|
||||
@property
|
||||
def log(self) -> "_FakeServer._Log":
|
||||
return self._Log(self.warnings)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def gunicorn_conf():
|
||||
"""Load gunicorn.conf.py once per test, then reset `workers` to the
|
||||
project default so other tests in the suite are not affected by us
|
||||
mutating the module global."""
|
||||
mod = _load_gunicorn_conf()
|
||||
yield mod
|
||||
# Restore the env-driven default so a stale value doesn't leak out.
|
||||
import os
|
||||
mod.workers = int(os.environ.get("GUNICORN_WORKERS", "2"))
|
||||
|
||||
|
||||
def test_on_starting_hook_is_defined(gunicorn_conf):
|
||||
"""The hook must exist — the bug fix lives here."""
|
||||
assert callable(getattr(gunicorn_conf, "on_starting", None)), (
|
||||
"gunicorn.conf.py is missing the on_starting hook. The MCP session-"
|
||||
"affinity warning is the only thing that keeps a multi-worker deploy "
|
||||
"from silently returning 'Session not found' for half its requests."
|
||||
)
|
||||
|
||||
|
||||
def test_on_starting_silent_for_single_worker(gunicorn_conf):
|
||||
"""workers == 1 is the safe config; no warning should fire."""
|
||||
gunicorn_conf.workers = 1
|
||||
server = _FakeServer()
|
||||
gunicorn_conf.on_starting(server)
|
||||
assert server.warnings == [], (
|
||||
f"workers=1 should be silent (no MCP session affinity issue), got: {server.warnings}"
|
||||
)
|
||||
|
||||
|
||||
def test_on_starting_warns_for_multi_worker(gunicorn_conf):
|
||||
"""workers > 1 must produce a warning that names the symptom and the fix.
|
||||
|
||||
The point of the hook is to be loud at deploy time, not at request time.
|
||||
"""
|
||||
gunicorn_conf.workers = 4
|
||||
server = _FakeServer()
|
||||
gunicorn_conf.on_starting(server)
|
||||
assert len(server.warnings) == 1, (
|
||||
f"expected exactly one warning when workers=4, got {len(server.warnings)}: {server.warnings}"
|
||||
)
|
||||
msg = server.warnings[0]
|
||||
assert "GUNICORN_WORKERS=4" in msg, "warning should name the offending worker count"
|
||||
assert "Session not found" in msg, "warning should name the symptom users actually see"
|
||||
assert "GUNICORN_WORKERS=1" in msg, "warning should point at the easy fix"
|
||||
assert "sticky-session" in msg, "warning should mention the load-balancer escape hatch"
|
||||
Reference in New Issue
Block a user