Keep status dependency checks from blocking probes#1085
Conversation
wayyoungboy
left a comment
There was a problem hiding this comment.
P2: the timeout currently only releases the coroutine waiting for the probe; it does not stop the synchronous probe already running in the default executor. After _cached_probe() stores the timeout result and releases the dependency lock, a later status poll after the cache TTL can start another worker even if the previous probe is still hung. In a real stuck dependency initialization, repeated /system/status polling can eventually exhaust default-executor workers and degrade unrelated asyncio.to_thread users.
Please keep same-dependency coalescing active until the underlying worker exits, or run these probes through a bounded/isolateable executor so a stuck dependency check cannot consume an unbounded number of shared workers.
|
Updated this PR to address the dependency probe timeout review finding:
Validation:
|
|
LGTM |
Summary:
Root cause:
/api/v1/system/statusperforms dependency checks for dashboard status data. In a single-worker deployment, a slow synchronous dependency probe can occupy the event loop long enough for lightweight health probes to time out.Validation:
pytest tests/unit/server/test_health_check.py tests/unit/server/test_settings.py(6 passed).pytest tests/unit/server(79 passed, 2 warnings from existing dependency deprecations).python -m compileall src/server/utils/health_check.py src/server/config.py.git diff --check.scripts/branch_name_check.sh --repo ../api/v1/system/healthrequest stayed responsive while/api/v1/system/statusreturned a degraded dependency result.