fix(serve): cache /api/status profile-gateway topology scan (GIL stalls starve desktop boot) - #71396
fix(serve): cache /api/status profile-gateway topology scan (GIL stalls starve desktop boot)#71396lost9999 wants to merge 1 commit into
Conversation
/api/status is the desktop's boot liveness probe (polled ~1/s) but since NousResearch#60537 every call ran a full topology scan — per-profile yaml.safe_load (pure-Python loader), psutil process probes, realpath walks — in the default executor. On multi-profile installs concurrent polls pile up and hold the GIL 14-16s, starving the event loop: the WS sidecar cannot flush gateway.ready, the desktop times out into the next stall, and boot escalates to the 'Hermes couldn't start' overlay (NousResearch#60800). Memoize the scan behind a 10s TTL with a collapse lock so concurrent polls share one scan. Topology only changes on gateway start/stop, so a <=10s stale badge is an acceptable trade for not starving the loop. The cache also keys on the collector's identity: tests monkeypatch _collect_profile_gateway_topology per case, and the identity check keeps them hermetic (a swapped collector is a miss) without a reset hook. py-spy captures during a failing boot land in _profile_platform_ports -> yaml.safe_load on executor threads (7 profiles, Windows). After: one cold-start scan, zero recurring stalls, desktop boots.
|
CI slice 7 caught Fixed by keying the cache on the collector's identity as well as the TTL — a swapped collector is a cache miss, so monkeypatching tests stay hermetic with no reset hook, and production behavior is unchanged (the identity is constant there). Added |
a3d94d7 to
e497f51
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the expensive topology collector and covering TTL, expiry, concurrent collapse, and collector replacement.
Problems
- The new comments describe current Desktop boot as repeatedly polling
/api/status. Current Desktop boot instead calls/api/healthatapps/desktop/electron/backend-health.ts:131-135; it reaches/api/statusonly for a backend without the health route (apps/desktop/electron/backend-health.ts:151-160). Please reframe this as a dashboard/status-consumer optimization. tests/test_web_server_status_topology_cache.pytests the helper directly, but not the route integration. The production boundary remainshermes_cli/web_server.py:3239-3243; add two HTTP/api/statuscalls with a mocked collector and assert one scan within the TTL.
The underlying current-main issue remains: every status request directly schedules _collect_profile_gateway_topology at hermes_cli/web_server.py:3239-3243, while the dashboard sidebar polls that endpoint in web/src/hooks/useSidebarStatus.ts:14-22.
Automated hermes-sweeper review.
| return {"profiles": profile_names, "gateway_mode": mode, "gateways": gateways} | ||
|
|
||
|
|
||
| # /api/status is polled ~1/s by the desktop app while it waits for the backend |
There was a problem hiding this comment.
Current Desktop boot now probes /api/health (apps/desktop/electron/backend-health.ts:131-135) and reaches /api/status only for a missing health route. Please reframe this comment around dashboard or other ongoing status consumers.
|
Merged via #76969 — thank you @lost9999. Your commit was cherry-picked, so you remain the author in git history. One honest note recorded in the salvage PR: since your branch, 23cb26c moved desktop boot readiness to /api/health, so the boot-time GIL-starvation symptom your PR described no longer occurs on current desktops. We merged it anyway because the value is real beyond boot — /api/status is still polled continuously (web sidebar 10s, statusbar 60s, Portal, multi-window), and your TTL cache + collapse lock deduplicates the genuinely expensive topology scan for all of them. The double-checked locking and the fn-identity test-hermeticity trick were both correct as written; nothing was changed. |
What does this PR do?
/api/statusis the liveness probe the desktop app polls (~1/s) while waiting for the backend to become ready (the dashboard badge polls it too). Since #60537, every call runs a full profile-gateway topology scan — for each profile home:yaml.safe_loadofconfig.yaml(pure-Python loader), psutil process-table probes, and realpath walks — inside the default executor.On multi-profile installs those scans pile up under the poll rate and hold the GIL for 14–16 s at a stretch, starving the event loop. The WS sidecar then cannot flush
gateway.ready(ws ready frame send failed), the desktop client times out and re-dials into the next stall, and after enough attempts boot escalates to the "Hermes couldn't start" overlay.This PR memoizes the scan behind a 10 s TTL cache with a collapse lock, so concurrent polls share one scan. Topology only changes when gateways start/stop, so a ≤10 s stale badge is an acceptable trade for not starving the loop.
Measured on the affected machine (Windows 11, 7 profiles, 3 MCP servers):
event loop stalled … (GIL pressure suspected)py-spy stack captures during a real failing boot land squarely in the scan on executor threads:
Related Issue
Fixes the dominant, measured stall source in #60800 on multi-profile installs (that issue's own hypothesis section had not located this path). Perf regression introduced by #60537.
Type of Change
Changes Made
hermes_cli/web_server.py: add_collect_profile_gateway_topology_cached()(10 s TTL,threading.Lockcollapse so concurrent executor calls share one scan); the/api/statushandler now calls the cached wrapper. Scan logic itself unchanged.tests/test_web_server_status_topology_cache.py(new): cache hit within TTL, rescan after TTL expiry, and 8-thread concurrent-poll collapse to a single scan.How to Test
pytest tests/test_web_server_status_topology_cache.py tests/test_web_server.py -qlogs/errors.log— repeating pairs ofevent loop stalled … GIL pressure suspected+ws ready frame send failedappear while the boot overlay shows.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — ran the targeted suites (test_web_server_status_topology_cache.py,test_web_server.py) on the affected production install; deferring the full suite to CIDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/A (no config keys)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Athreading/time), no platform-specific codeScreenshots / Logs
Failing boot (before),
logs/errors.log:After the patch (same machine, 110 s observation): a single cold-start stall, zero recurrence, desktop boots to a working UI.