fix: multi-replica HA — converge cross-pod subscriptions (#1053) + confirm death before reaping (#1052) - #1056
Open
oeway wants to merge 2 commits into
Open
fix: multi-replica HA — converge cross-pod subscriptions (#1053) + confirm death before reaping (#1052)#1056oeway wants to merge 2 commits into
oeway wants to merge 2 commits into
Conversation
….136) RedisEventBus.subscribe_to_client_events recorded a targeted psubscribe pattern in `_subscribed_patterns` optimistically — BEFORE the psubscribe was confirmed — and on a psubscribe timeout/error left the pattern recorded-but-unwired. The `if pattern not in self._subscribed_patterns` guard then turned every later subscribe into a no-op, so the pattern was a permanent cross-pod black hole: targeted RPC delivered via the Redis `targeted:<ws>/<client_id>:*` channel was silently dropped forever. This was masked for same-pod callers by the local short-circuit in `emit`, and it is the root cause of the #1052 orphan-reaper false-reap (a live client fails a single cross-pod ping precisely because of this gap, then the #15 reaper deletes its services:* keys). Fix: separate the DESIRED set (`_subscribed_patterns`, what we want wired) from the CONFIRMED set (`_confirmed_patterns`, actually wired to the live pubsub). The membership guard is now keyed on the confirmed set, so a desired-but-unconfirmed pattern is retried instead of no-op'd, via three paths: (a) a subsequent subscribe call, (b) reconnect re-wire that never discards a desired pattern on transient failure, and (c) a new background reconciler loop (`_reconcile_loop`, HYPHA_SUBSCRIPTION_RECONCILE_INTERVAL, default 5s) — required because subscribe_to_client_events is called only ONCE per client at register time, so register-time-timeout convergence cannot rely on a later subscribe call. On reconnect the circuit breaker is cleared BEFORE re-wiring targeted patterns so a stale-open breaker cannot refuse to wire them on the fresh healthy pubsub. Tests (tests/test_cross_pod_subscription_convergence.py, Docker-free, two RedisEventBus instances sharing one fakeredis = two pods on one Redis, reproduce-before-fix): register-time psubscribe timeout is reconciled via retry; the reconciler re-wires without a second subscribe call; a reconnect re-wire preserves every desired pattern and never discards a transiently failing one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… a client The #15 continuous orphan reaper deleted ALL of a client's services:* keys after a SINGLE failed cross-pod ping. The cross-pod ping is best-effort (Redis pub/sub has no buffering), so a single dropped message during a reconnect / subscription-convergence window (the #1053 window) made the reaper delete a LIVE client's registration — a silent, permanent outage until manual restart. Defense-in-depth on top of the #1053 root-cause fix: track per-client CONSECUTIVE probe failures across reaper passes (_orphan_probe_failures) and only reap after HYPHA_ORPHAN_REAP_MIN_FAILURES (default 3) consecutive failures. A client that answers any probe has its counter reset; a client that drops out of the candidate set has its counter pruned. At the default 300s interval, 3 failures span ~10 min — far longer than any transient unreachability window — so only a genuinely dead client is reaped. Tests: tests/test_orphan_reaper_confirmation.py (single dropped ping does not reap; N consecutive failures do reap; a recovered probe resets the counter; a disappeared candidate is pruned). Existing single-pass reaper tests set MIN_FAILURES=1 to preserve their concurrency/startup-cleanup focus. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two tightly-coupled multi-replica (N≥2 pods) HA fixes. #1052 is a direct consequence of the #1053 window, so they ship together as one coherent fix (0.21.136).
#1053 — cross-pod targeted subscriptions never converge (root cause)
RedisEventBus.subscribe_to_client_eventsoptimistically recorded a targeted psubscribe pattern (targeted:<ws>/<client_id>:*) in_subscribed_patternsbefore confirming the psubscribe. On a timeout/error the pattern stayed recorded-but-unwired, and theif pattern not in self._subscribed_patternsguard turned every future attempt into a no-op → a permanent cross-pod black hole for that client's targeted messages. Masked for same-pod callers by the local short-circuit inemit.Fix: separate the DESIRED set (
_subscribed_patterns) from the CONFIRMED set (_confirmed_patterns, actually wired to the live pubsub); guard on CONFIRMED. Three convergence paths:subscribe_to_client_eventscall retries the unconfirmed pattern;_rewire_desired_patterns, never discards on failure) — and clears the circuit breaker before re-wiring so a stale-open breaker can't refuse to wire on a fresh healthy pubsub;_reconcile_loop,HYPHA_SUBSCRIPTION_RECONCILE_INTERVAL, default 5s) retriesdesired − confirmed— required becausesubscribe_to_client_eventsis called only once per client at register.Tests:
tests/test_cross_pod_subscription_convergence.py(reproduce-before-fix: a dropped targeted psubscribe is later reconciled; the reconcile loop re-wires without a second subscribe call; reconnect re-wire preserves desired and never discards).#1052 — orphan reaper deletes live clients after a single dropped ping (defense-in-depth)
The #15 continuous reaper deleted all of a client's
services:*keys after a single failed cross-pod ping. The ping is best-effort (Redis pub/sub has no buffering), so one dropped message during the #1053 convergence window made the reaper delete a live client's registration — a silent, permanent outage until manual restart.Fix: track per-client consecutive probe failures across reaper passes and only reap after
HYPHA_ORPHAN_REAP_MIN_FAILURES(default 3) consecutive failures. Any successful probe resets the counter; a client that drops out of the candidate set is pruned. At the default 300s interval, 3 failures span ~10 min ≫ the convergence/reconcile window, so only genuinely-dead clients are reaped.Tests:
tests/test_orphan_reaper_confirmation.py(single dropped ping does not reap; N consecutive failures do reap; a recovered probe resets the counter; a disappeared candidate is pruned). The two existing single-pass reaper tests setMIN_FAILURES=1to preserve their concurrency/startup-cleanup focus.New env knobs
HYPHA_SUBSCRIPTION_RECONCILE_INTERVAL(default5.0s)HYPHA_ORPHAN_REAP_MIN_FAILURES(default3)Blast radius / review note
Core event-bus subscription path — affects every deployment (single- and multi-pod). Docker-free tests pass locally; multi-replica integration tests run in CI. Do not merge without maintainer (@oeway) sign-off despite green CI, given the all-deployment blast radius.
🤖 Generated with Claude Code