From 8bbf3afda048f40d30b41d458157262f1881ba1d Mon Sep 17 00:00:00 2001 From: Adryan Eka Vandra Date: Sat, 18 Apr 2026 02:17:37 +0700 Subject: [PATCH] perf(proxy): gate /debug/tasks stack-depth computation behind query param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task.get_stack(limit=32) walks coroutine frames synchronously and measurably stalls the event loop when called for 50+ relay tasks during a reconnect storm. The /debug/tasks snapshot does not need this by default — the perf-cheap fields (name, coro_qualname, age, done) are enough for the common case of "which tasks are alive". - collect_tasks: add with_stack_depth=False kwarg; default leaves stack_depth=None per entry. - /debug/tasks: read ?stack=true query parameter and forward it. Default response is cheap; opt-in explicitly when human-debugging one snapshot. - Test: verify default entries have stack_depth=None and that ?stack=true produces at least one integer depth. --- headroom/proxy/debug_introspection.py | 10 ++++++++- headroom/proxy/server.py | 14 ++++++++++-- tests/test_proxy_debug_endpoints.py | 31 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/headroom/proxy/debug_introspection.py b/headroom/proxy/debug_introspection.py index 80e284ed9..60254a8e2 100644 --- a/headroom/proxy/debug_introspection.py +++ b/headroom/proxy/debug_introspection.py @@ -113,6 +113,8 @@ def _age_for_named_task( def collect_tasks( ws_registry: WebSocketSessionRegistry | None = None, + *, + with_stack_depth: bool = False, ) -> list[dict[str, Any]]: """Enumerate ``asyncio.all_tasks()`` for /debug/tasks. @@ -121,6 +123,12 @@ def collect_tasks( and ``done``. Sorted by age descending with ``None`` ages sorted after known ages. System noise (``None`` tasks, tasks with no coroutine) is filtered out. + + ``stack_depth`` is only computed when ``with_stack_depth=True`` + because :meth:`asyncio.Task.get_stack` walks coroutine frames and + can noticeably stall the event loop during a storm with 50+ relay + tasks. The default returns ``stack_depth=None``; callers that need + it (a human debugging one snapshot) can pass ``with_stack_depth=True``. """ try: tasks = asyncio.all_tasks() @@ -145,7 +153,7 @@ def collect_tasks( "name": name, "coro_qualname": qualname, "age_seconds": age, - "stack_depth": _stack_depth(task), + "stack_depth": _stack_depth(task) if with_stack_depth else None, "done": bool(task.done()), } entries.append(entry) diff --git a/headroom/proxy/server.py b/headroom/proxy/server.py index 61f6e8765..5eb9dbd8e 100644 --- a/headroom/proxy/server.py +++ b/headroom/proxy/server.py @@ -1335,9 +1335,19 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: from headroom.proxy.loopback_guard import require_loopback as _require_loopback @app.get("/debug/tasks", dependencies=[Depends(_require_loopback)]) - async def debug_tasks(): + async def debug_tasks(stack: bool = False): + """Enumerate running asyncio tasks. + + Default is cheap — ``stack_depth`` is ``null`` in every entry so + a storm snapshot does not walk 50+ coroutine frames synchronously. + Pass ``?stack=true`` to compute ``stack_depth`` for each task + (useful for single-shot human debugging). + """ ws_registry = getattr(proxy, "ws_sessions", None) - return JSONResponse(status_code=200, content=_collect_tasks(ws_registry)) + return JSONResponse( + status_code=200, + content=_collect_tasks(ws_registry, with_stack_depth=stack), + ) @app.get("/debug/ws-sessions", dependencies=[Depends(_require_loopback)]) async def debug_ws_sessions(): diff --git a/tests/test_proxy_debug_endpoints.py b/tests/test_proxy_debug_endpoints.py index f7b1e1ec2..c64f4228e 100644 --- a/tests/test_proxy_debug_endpoints.py +++ b/tests/test_proxy_debug_endpoints.py @@ -256,6 +256,37 @@ def test_debug_tasks_returns_json_array_for_loopback(client): assert "coro_qualname" in entry +def test_debug_tasks_stack_depth_is_gated_behind_query(client): + """Default response must not compute stack_depth (P3 Fix 29 perf gate). + + ``?stack=true`` opts into the synchronous ``Task.get_stack`` walk; the + default stays cheap so snapshotting during a reconnect storm does + not stall the event loop. + """ + default = client.get("/debug/tasks") + assert default.status_code == 200 + for entry in default.json(): + assert entry["stack_depth"] is None, ( + f"default /debug/tasks must not compute stack_depth; " + f"got {entry['stack_depth']!r} for {entry.get('name')!r}" + ) + + with_stack = client.get("/debug/tasks?stack=true") + assert with_stack.status_code == 200 + entries = with_stack.json() + # At least one entry should have a computed depth (the TestClient + # itself runs under a task). Some entries may still be None if + # get_stack raised defensively — we only require that opting in + # produces at least one integer result. + integer_depths = [ + e["stack_depth"] for e in entries if isinstance(e["stack_depth"], int) + ] + assert integer_depths, ( + "expected at least one int stack_depth when ?stack=true; " + f"got entries={entries!r}" + ) + + def test_debug_warmup_reports_registry_slots(client): response = client.get("/debug/warmup") assert response.status_code == 200