mirror of
https://github.com/jamiepine/voicebox.git
synced 2026-10-03 00:55:14 -07:00
perf(mcp): move last_seen_at stamp off the request path
ClientIdMiddleware was running the SQLAlchemy SELECT/INSERT/UPDATE/COMMIT inline on the event loop after every /mcp/* and /speak request. SQLite serialises writes, so concurrent MCP traffic queued behind the stamp write — the response sat waiting on a side-effect that the client never needs in band, and SSE streams would stall briefly per request. The middleware now hands the stamp to asyncio.to_thread via a fire-and- forget create_task so the response returns immediately and the write runs on the default executor. A module-level set keeps strong refs to in-flight tasks (per asyncio docs) so the GC can't collect them mid- write. The fallback path runs the stamp inline if no loop is available (tests/oddball callers) rather than silently dropping it.
This commit is contained in:
@@ -7,6 +7,7 @@ ContextVar so tool implementations can read it without plumbing the request
|
|||||||
object through every service call.
|
object through every service call.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
import ipaddress
|
import ipaddress
|
||||||
import logging
|
import logging
|
||||||
from contextvars import ContextVar
|
from contextvars import ContextVar
|
||||||
@@ -20,6 +21,10 @@ from starlette.types import ASGIApp
|
|||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# Strong refs to in-flight stamp tasks so asyncio.create_task results
|
||||||
|
# don't get garbage-collected mid-flight (cf. asyncio.create_task docs).
|
||||||
|
_pending_stamps: set[asyncio.Task] = set()
|
||||||
|
|
||||||
CLIENT_ID_HEADER = "X-Voicebox-Client-Id"
|
CLIENT_ID_HEADER = "X-Voicebox-Client-Id"
|
||||||
|
|
||||||
# Tool handlers read this to apply per-client voice bindings.
|
# Tool handlers read this to apply per-client voice bindings.
|
||||||
@@ -86,10 +91,30 @@ class ClientIdMiddleware(BaseHTTPMiddleware):
|
|||||||
current_remote_addr.reset(addr_token)
|
current_remote_addr.reset(addr_token)
|
||||||
|
|
||||||
if client_id and _is_stamped_path(request.url.path):
|
if client_id and _is_stamped_path(request.url.path):
|
||||||
_stamp_last_seen(client_id)
|
_enqueue_stamp(client_id)
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|
||||||
|
def _enqueue_stamp(client_id: str) -> None:
|
||||||
|
"""Fire-and-forget the SQLite write so it doesn't block the response.
|
||||||
|
|
||||||
|
The stamp does sync SQLAlchemy I/O; running it inline on the event loop
|
||||||
|
serialises every MCP request behind the SQLite write and starves SSE
|
||||||
|
streams. ``asyncio.to_thread`` parks it on the default executor while
|
||||||
|
the response goes back to the caller.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
loop = asyncio.get_running_loop()
|
||||||
|
except RuntimeError:
|
||||||
|
# Middleware shouldn't run outside a loop, but if it ever does
|
||||||
|
# (tests, weird wsgi shim), do the write inline rather than drop it.
|
||||||
|
_stamp_last_seen(client_id)
|
||||||
|
return
|
||||||
|
task = loop.create_task(asyncio.to_thread(_stamp_last_seen, client_id))
|
||||||
|
_pending_stamps.add(task)
|
||||||
|
task.add_done_callback(_pending_stamps.discard)
|
||||||
|
|
||||||
|
|
||||||
def _is_stamped_path(path: str) -> bool:
|
def _is_stamped_path(path: str) -> bool:
|
||||||
# Require a path boundary so a future ``/speakers`` or ``/mcpfoo``
|
# Require a path boundary so a future ``/speakers`` or ``/mcpfoo``
|
||||||
# route doesn't silently inherit the stamp from ``/speak`` / ``/mcp``.
|
# route doesn't silently inherit the stamp from ``/speak`` / ``/mcp``.
|
||||||
|
|||||||
Reference in New Issue
Block a user