Skip to content

fix(startup): bound + parallelize check_and_cleanup_servers (#0043) — 0.21.134 - #1050

Merged
oeway merged 2 commits into
mainfrom
fix/0043-bound-server-check-startup
Aug 12, 2026
Merged

fix(startup): bound + parallelize check_and_cleanup_servers (#0043) — 0.21.134#1050
oeway merged 2 commits into
mainfrom
fix/0043-bound-server-check-startup

Conversation

@oeway

@oeway oeway commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Sibling of #15. #15 moved the orphaned-client reap off the readiness path and made it concurrent, but left the server-check phase — RedisStore.check_and_cleanup_servers, awaited inline in init() — with the same O(N × timeout) shape that CrashLooped the pod on a large post-crash Redis pile.

Root cause

  • list_servers() scans services:*|*:public/*:built-in@* — the built-in service of every public-workspace client, not just live hypha instances. After a crash/rollout a non-reset (prod, HYPHA_RESET_REDIS=false) Redis carries a pile of dead built-in registrations.
  • The old loop pinged each sequentially; a dead client never answers, so get_remote_service(..., {"timeout": 2}) burned the full timeout per server, and svc.ping("ping") itself had no explicit timeout — a half-open peer that resolves but never replies could hang boot forever.

So N dead registrations added O(N × 2s) (or unbounded) to the readiness path — exactly the #15 failure mode on a different phase.

Fix (readiness path stays bounded)

  • Concurrent probing, concurrency-capped (HYPHA_SERVER_CHECK_CONCURRENCY, default 50) → one pass is ~ceil(N/cap) × timeout, not N × timeout.
  • Each probe bounded: both the get_remote_service resolution and the ping round-trip get an explicit timeout (HYPHA_SERVER_CHECK_TIMEOUT, default 2).
  • Whole phase bounded by an overall deadline (HYPHA_SERVER_CHECK_DEADLINE, default 60); on deadline, log + continue startup (the background reaper trims the rest) rather than block readiness.
  • Contracts preserved: the duplicate-self-id guard still raises RuntimeError if a live server already holds this server's id; dead-server cleanup key patterns are byte-for-byte unchanged (server-owned services only — user services untouched).

Also bounds the ping in _cleanup_orphaned_client_services._probe with asyncio.wait_for (item 4): resolution was bounded there but the ping itself was not.

Tests

tests/test_server_check_bounded.py (real, docker-free, fakeredis; reproduce-before-fix): seed genuine dead built-in registrations in a non-reset Redis (prod-like — a reset boot flushes the pile and can never reproduce it), boot a real RedisStore, and assert boot is prompt and the dead registrations are cleaned.

  • Pre-fix: init() took 24.3s for 12 dead servers (12 × ~2s serial).
  • Post-fix: ~1s (concurrent, honoring HYPHA_SERVER_CHECK_TIMEOUT).
  • Overall-deadline test: per-probe timeout 30s but deadline 2s → phase returns in ~2s.
  • Clean-boot guard: happy path still registers this server.

Regression: test_orphan_reaper.py, test_login_registration_stale_marker.py, test_server_disconnection.py (13, incl. the two direct callers of check_and_cleanup_servers) all green.

🤖 Generated with Claude Code

….21.134

Sibling of #15. #15 moved the orphaned-CLIENT reap off the readiness
path and made it concurrent, but left the SERVER-check phase
(RedisStore.check_and_cleanup_servers, awaited inline in init()) with the
same O(N x timeout) shape that CrashLooped the pod:

  * list_servers() scans services:*|*:public/*:built-in@* — the built-in of
    EVERY public-workspace client, so after a crash/rollout a non-reset
    (prod) Redis carries a pile of DEAD built-in registrations.
  * The old loop pinged each SEQUENTIALLY; a dead client never answers, so
    get_remote_service(..., {"timeout": 2}) burned the full timeout per
    server, and svc.ping("ping") itself had NO explicit timeout — a
    half-open peer that resolves but never replies could hang boot forever.

Fix (readiness path stays bounded):
  * Probe servers CONCURRENTLY, concurrency-capped
    (HYPHA_SERVER_CHECK_CONCURRENCY, default 50) → one pass ~ceil(N/cap) x
    timeout, not N x timeout.
  * Bound EACH probe: both get_remote_service resolution AND the ping
    round-trip get an explicit timeout (HYPHA_SERVER_CHECK_TIMEOUT,
    default 2).
  * Bound the WHOLE phase with an overall deadline
    (HYPHA_SERVER_CHECK_DEADLINE, default 60); on deadline, log + continue
    startup (the background reaper trims the rest) rather than block
    readiness.
  * Preserve the duplicate-self-id guard (a LIVE server already holding
    this server's id still raises RuntimeError) and the exact dead-server
    cleanup key patterns (server-owned services only; user services are
    untouched, as before).

Also bound the ping in _cleanup_orphaned_client_services._probe with
asyncio.wait_for (item 4): resolution was bounded but the ping was not.

Tests (real, docker-free, fakeredis; reproduce-before-fix): seed genuine
dead built-in registrations in a non-reset Redis (prod-like — a reset boot
flushes the pile and can never reproduce it) and assert boot is prompt and
the dead registrations are cleaned. Pre-fix: init took 24.3s for 12 dead
servers (12 x ~2s serial); post-fix ~1s.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added bug Something isn't working documentation Improvements or additions to documentation labels Aug 12, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@oeway
oeway merged commit c240b66 into main Aug 12, 2026
11 of 12 checks passed
@oeway
oeway deleted the fix/0043-bound-server-check-startup branch August 12, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant