[serve] Push-based replica health: replicas self-check and report over existing channels by johntaylor-cell · Pull Request #64913 · ray-project/ray

johntaylor-cell · 2026-07-21T20:44:42Z

[serve] Push-based replica health: replicas self-check and report over existing channels

Why

Pull health checking makes the controller the initiator: every health-check
period it schedules an actor call per replica, awaits the results, and tracks
the in-flight refs. Actor-call submission alone is ~0.5 ms of GIL-bound work, so
at 10K+ replicas the controller spends whole ticks just asking — and under
load the checks arrive late, which reads as replica failure when it is really
controller lag.

This PR inverts the direction. Replicas already know their own health — they run
the user's check_health locally — and most of them already send the controller
a periodic message (the autoscaling metric report). So: replicas self-check on
their own clock and the result rides messages that already flow. For a
metric-reporting replica the incremental cost of health is three fields on an
existing report; only non-reporting replicas fall back to a lightweight
heartbeat RPC (deduplicated, at most one in flight).

What

Replica side

A periodic self-health task evaluates the user health check at half the
configured period (so results are always fresh within one period despite
scheduling jitter) and latches at the unhealthy threshold — the user check
stops re-running, unhealthy keeps being reported; parity with pull probes.
Remote check_health calls serve the cached result while the task is active:
concurrent probes can no longer re-run a user health check.
ReplicaMetricReport carries (healthy, checked_at, consecutive_failures);
when the metric cadence is slower than the health period, the heartbeat fills
the gap so freshness never lapses.

Controller side

A push registry keeps the newest entry per replica (out-of-order arrivals
resolved by checked_at) and exposes self-check interval stats
(p50/p99/max) on the controller health metrics.
During the reconcile sweep, pushed entries are handed to the replica wrapper:
a fresh push substitutes for scheduling a pull probe; the pull path remains,
untouched, as the fallback for replicas whose push has gone stale (1.5× the
health-check period).

The stall guard
When most of a deployment's pushes go stale simultaneously, the cause is
controller-side ingest lag, not mass replica failure — and probing thousands of
"stale" replicas at once drowns the already-lagging controller (we reproduced
this spiral at 16K). If ≥50% of ≥64 tracked replicas are stale in one sweep, the
sweep defers fallback probes, bounded at 120 s per episode so a genuine mass
outage still falls through to probes. Actor death detection (GCS) is unaffected
by the guard and remains immediate.

Behavior changes

Health-check freshness becomes replica-driven; detection latency for a hung
replica is bounded by the freshness window + probe time (and by the 120 s
episode cap while a stall verdict is active).
No API or config changes; existing knobs (health_check_period_s,
health_check_timeout_s, unhealthy threshold) keep their meanings.

Testing

Registry: newest-wins on out-of-order reports, gap statistics.
Pusher: half-period scheduling, unhealthy latch, heartbeat suppression when
reports carry health, in-flight heartbeat dedup.
Controller: pushed-health handoff to wrappers, probe-result precedence when a
probe resolves in the same tick (push stashed, consumed next tick), stall
verdict engage/disengage and probe deferral.
test_deployment_state.py + test_metrics_utils.py + test_autoscaling_policy.py
green on this tier (364 tests).

Benchmarks

Full-stack results (this PR + # + #): 8K replicas — health
freshness misses its deadline 23 times in 1.09M updates (pull baseline: 93.5%
missed); 16K — first clean completion on this harness, 0.55% misses. This PR's
isolated contribution (base vs P1-only arms, same harness) is being measured and
lands here before review.

gemini-code-assist

Code Review

This pull request introduces a replica-pushed self-health mechanism in Ray Serve, allowing replicas to push their health status to the controller to avoid pull probes and mitigate probe storms during controller-side ingest lag. It also optimizes rank consistency checks and memoizes derived ID collections to reduce O(N) overhead on control-loop ticks. The review feedback highlights critical improvement opportunities: rate-limiting the pruning of the health push registry to prevent CPU thrashing and GIL bottlenecks under high load, and initializing the push stall window with a safe default to avoid false stall alarms when target state info is unavailable.

gemini-code-assist

Code Review

This pull request introduces a replica-pushed self-health mechanism in Ray Serve to optimize health checking and reduce controller load, alongside performance optimizations like memoized id-collection walks and gating rank-consistency checks on membership changes. The review feedback highlights critical improvements: resolving a potential CPU and log-flooding loop in rank consistency checks upon deterministic errors, correcting a return type mismatch in _take_fresh_pushed_health, preventing metric desensitization in ReplicaHealthPushRegistry by avoiding indefinite accumulation of gap statistics, and enhancing debuggability by explicitly logging asyncio.TimeoutError during replica health checks.

The per-tick rank-consistency pass is O(N) with heavy constants; at 10K+ replicas it monopolizes the controller event loop in steady state (py-spy: it starved handle-report ingest for 20s+ at 16K). Rank consistency can only be violated by membership changes, so gate the pass on a fingerprint of the active replica set. Signed-off-by: john.taylor <john.taylor@anyscale.com>

Review (gemini): compare the set of active replica ids instead of an xor fingerprint -- no hash-collision risk, drops the reduce/xor imports, and set comparison is C-optimized. Review (cursor): when fail_on_rank_error is off the pass can swallow an error and return []; do not cache that membership as checked, or a stable deployment never retries. DeploymentRankManager now records whether the last op errored; cache only on success. Signed-off-by: john.taylor <john.taylor@anyscale.com>

…reconfigure - Expose last_rank_op_errored as a read-only property; DeploymentState no longer reaches into the rank manager private. - Snapshot the flag BEFORE _reconfigure_replicas_with_new_ranks, which calls get_replica_rank() and resets it. Safe today only because safe_default is [] and the reconfigure early-returns on an empty list -- an accident of the default, not a design. - Hoist the O(1) status/STARTING guards above the O(N) _replicas.get() copy (0.23ms at 16384), previously built and discarded on the busiest ticks. - Two tests over a real DeploymentRankManager(fail_on_rank_error=False), the production default: a swallowed error is not cached, and the flag is read before the reconfigure clears it. Both fail if the manager-side fix is reverted; the prior test mocked the manager and asserted only its own write. - Drop a redundant local import; note that never-caching a persistently erroring deployment is deliberate. Signed-off-by: john.taylor <john.taylor@anyscale.com>

Review: the tests built a DeploymentState via __new__ (bypassing __init__) and injected Mocks plus private attributes (_replicas, _rank_manager, _last_rank_membership_ids). That couples them to internal field names and stubs out the logic under test. They now follow the pattern used elsewhere in this file: a real DeploymentStateManager from the mock_deployment_state_manager fixture, driven through deploy()/update() with MockReplicaActorWrapper, and a single injected collaborator -- a DeploymentRankManager subclass that counts consistency passes and delegates to the real implementation, patched at the same seam the fixture uses for the actor wrappers. Real rank logic, observable call counts, no private writes. Membership changes are triggered by a scale-up redeploy rather than a health check failure: a failed health check leaves the deployment UNHEALTHY with a replica stuck in STOPPING, and the gate requires HEALTHY, so those assertions were unsatisfiable. 235 tests pass; disabling the gate fails test_skips_while_membership_unchanged. Signed-off-by: john.taylor <john.taylor@anyscale.com>

Review: an errored pass is deliberately not cached so it gets retried, but a deterministic failure then re-runs an O(N) pass and logger.error()s on every control-loop tick, indefinitely. Caching the membership on error would stop that but is worse: the gate would latch a membership the pass never validated, so a real inconsistency would never be rechecked or repaired. Instead back off, and only while the membership still matches the one that failed -- a genuine membership change bypasses the backoff and runs immediately, so recovery is never delayed by a stale failure. _RANK_ERROR_RETRY_S is a module constant, not a new env knob. Tests assert both halves: the same membership is not re-run on subsequent ticks, and is re-run once the backoff elapses (driven via the fixture MockTimer). Signed-off-by: john.taylor <john.taylor@anyscale.com>

… fingerprint The per-tick id walks (alive-actor-ids, active-node-ids, running-ids, the autoscaler id-set rebuild) recompute identical results on every steady-state tick. Memoize each on a ReplicaStateContainer membership fingerprint, and disarm while STARTING or RECOVERING replicas exist, since their actor_id/actor_node_id materialize in place without touching the container. The fingerprint is the summed per-replica hash of the container's {replica id -> state} content rather than a mutation counter, because the health and migration passes pop a whole bucket and re-add every replica to the state it came from. A counter reads that churn as a membership change, and one node draining anywhere in the cluster is enough to trigger it: at 4 replicas it bumps 5 times per otherwise-idle tick, which invalidates all three memos and re-runs the O(N) rank-consistency pass on every tick. Summing per-replica terms makes pop-and-re-add cancel exactly, while a real arrival, departure or state transition still changes it. The same fingerprint replaces the id-set key of the rank-consistency gate and of its error backoff, so both keep an O(1) key without losing the content-based semantics that made them hold under churn. Manager-level getters return frozensets so the memo cannot be mutated by callers (with set() at the one mutating caller), and AutoscalingContext.running_replicas is copied to a list at the public boundary. The gate tests also stop depending on RAY_SERVE_FAIL_ON_RANK_ERROR. The backoff test injects a failure that _execute_with_error_handling swallows only when that flag is off; four CI variants of this file set it to 1, where the error propagated out of update() instead of exercising the retry path. The fake rank manager now pins the swallow path, which is the production default and the only mode where that backoff exists. Signed-off-by: john.taylor <john.taylor@anyscale.com> Co-Authored-By: Claude <noreply@anthropic.com>

gemini-code-assist on ray-project#64913: - gap_stats() accumulated over the controller lifetime, so the reported p50/p99/max went insensitive to current lag exactly when the numbers are worth reading. Hold the histogram in two windows and report the last one or two; `count` stays cumulative so it still reads as a counter. - _take_fresh_pushed_health() returns (healthy, consecutive_failures), not a bool -- fix the annotation. - A timed-out self-check was swallowed silently: log the cause. wait_for enforces its timeout by cancelling the check, and CancelledError is not an Exception, so the cached result also stayed healthy -- a fallback pull probe would then answer healthy for a replica whose check is wedged, contradicting the push that just reported the timeout. Mark it unhealthy on cancellation. The rank-consistency retry comment is fixed by the rebase: that pass is A6 code, and A6 already carries the retry backoff. Signed-off-by: john.taylor <john.taylor@anyscale.com>

…ports P1 of the push-health stack. Replicas run their own health check on a periodic task (twice per health-check period) and push the result: on the metric reports they already send when those are frequent enough to keep the controller's view fresh, or on a lightweight heartbeat otherwise. The controller keeps a push registry; pull probes become the fallback for stale pushes, guarded by a per-deployment systemic-stall verdict (mass staleness reads as controller ingest lag and defers probes, capped per episode) so an overloaded controller cannot trigger a probe storm. Review fixes folded in: - Registry pruning is rate-limited to once per 30s; the over-threshold prune used to re-run an O(N) dict rebuild on every record() once all entries were fresh. The stall window defaults to the stock health-check period so deployments without target info (recovery, deletion) cannot misread every push as stale. - An unhealthy self-check is never suppressed: suppression assumed a recent report carried current health, but a flip to unhealthy after the last carry would be delayed a full period. The in-flight heartbeat dedupe honours the same rule: turning unhealthy jumps the queue rather than waiting out a lag episode, but only until an unhealthy heartbeat is actually in flight. Keying that on what the outstanding push carries, rather than on whether a bypass happened, is what bounds it -- the payload is absolute, so a second unhealthy heartbeat adds nothing and a flapping replica could otherwise queue many. - The push-stall tally accumulates across ticks rather than per dirty-set slice, which only engaged the guard above ~3200 replicas per deployment. Extract _reconcile_sweep_ticks (shared with the dirty set). Known follow-up: many small deployments can each stay under the per-deployment floor while the controller lags overall. - Gap stats live in two windows instead of accumulating over the controller's lifetime, where the reported p50/p99/max went insensitive to current lag exactly when the numbers are worth reading; count stays cumulative. - A timed-out self-check logs its cause, and marks itself unhealthy on cancellation: wait_for enforces the timeout by cancelling the check, and CancelledError is not an Exception, so the cached result stayed healthy and a fallback pull probe would answer healthy for a wedged check. - A probe still in flight when a newer push is applied no longer overwrites it on resolving: its result predates the push, so it is dropped instead of flapping the failure count across pull-to-push transitions. ACTOR_CRASHED is exempt -- a crash is authoritative and a dead replica pushes nothing. - Probe deferral no longer borrows _last_push_consume_time. That field now means strictly "a push was applied"; deferral carries its own deadline, supplied by the deployment as the stall episode's cap. Before, the gate honoured the borrowed stamp for a further freshness window past the cap (+1.5x the period, so 570s against a documented 120s at a 300s period), and the drop guard above read a deferral as a superseding observation and destroyed every probe result that resolved during an episode -- exactly when a probe is the only signal. The deadline is re-stamped on every visit, including with 0, so an episode that ends early retracts rather than leaving a replica sitting out a cap that no longer applies; and it is staggered per replica, because one deadline shared by a whole deployment opens every gate on the same tick, which is the storm the deferral exists to avoid. The offset only shortens the hold. - The failure count advances once per configured period, not once per eval. Evals run twice per period for freshness, but the controller weighs the count against a threshold calibrated to the period, so counting each one replaced a replica in half the wall clock the configuration asks for. - The probe gate is anchored to when the newest push ARRIVED rather than to when the reconcile loop consumed it (the field is now _last_applied_push_received_at to say so), because a slow reconcile otherwise stretches the window past the 1.5x period it documents -- on the very ticks the controller is behind. - The drop guard is symmetric. A push that arrived before a probe was even started is likewise strictly older, so it can no longer sit in the stash and overwrite that probe's result on the next tick. A push that arrived while the probe was in flight still wins, which is the sibling guard's whole point. - The stall tally counts each replica once per window. The dirty set re-visits a replica with an in-flight probe every tick, and those are precisely the stale-push ones, so the undeduped tally could read a small failed cohort as fleet-wide ingest lag and defer probes for everyone. The MIN_TRACKED floor now means 64 distinct replicas, matching what it always claimed to mean. - A replica with no registry entry is no longer deferred: probes are its only health signal and it cannot sway the verdict either way. - health_check_failures_counter follows the observation actually acted on. Only the probe paths set the failure flag, so the counter had gone silent for push-detected failures while still counting probe results the controller discarded -- inverted precisely when push health is doing its job. - Heartbeat suppression compares the metric-report interval against the configured health-check period, not the half-period evaluation cadence -- under the stock config (10s period, 10s metric interval) the latter suppressed nothing, so every reporting replica heartbeated anyway. Signed-off-by: john.taylor <john.taylor@anyscale.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit f628c22. Configure here.}

cursor · 2026-07-27T23:53:05Z

+                    self._last_self_health_error or "Replica self health check failed."
+                )
+            return
+        await self._run_user_health_check()


Recovery fails on cached health

High Severity

After the self-health task starts, check_health serves the cached result instead of re-running the user check. Controller recovery still calls initialize_and_get_metadata, which always awaits check_health and treats failure as init failure. A single recent failed self-check leaves _healthy false, so recovery can fail a live replica that a fresh check would pass and replace it unnecessarily.

Additional Locations (2)

python/ray/serve/_private/replica.py#L1998-L2005

python/ray/serve/_private/deployment_state.py#L1577-L1581

^{Reviewed by Cursor Bugbot for commit f628c22. Configure here.}

johntaylor-cell requested a review from a team as a code owner July 21, 2026 20:44

gemini-code-assist Bot reviewed Jul 21, 2026

View reviewed changes

Comment thread python/ray/serve/_private/deployment_state.py

Comment thread python/ray/serve/_private/deployment_state.py Outdated

Comment thread python/ray/serve/_private/deployment_state.py Outdated

johntaylor-cell force-pushed the serve-push-core branch from 026fa1d to ecd3328 Compare July 21, 2026 23:02

ray-gardener Bot added the serve Ray Serve Related Issue label Jul 22, 2026

This was referenced Jul 22, 2026

[serve] Push health: interleave health frames on open response streams #64909

Closed

[serve] Carry replica self-health on handle metric reports #64912

Closed

johntaylor-cell marked this pull request as draft July 26, 2026 21:56

johntaylor-cell self-assigned this Jul 26, 2026

johntaylor-cell added the performance label Jul 26, 2026

johntaylor-cell marked this pull request as ready for review July 27, 2026 18:16

johntaylor-cell added the go add ONLY when ready to merge, run all tests label Jul 27, 2026

gemini-code-assist Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread python/ray/serve/_private/deployment_state.py Outdated

Comment thread python/ray/serve/_private/deployment_state.py Outdated

Comment thread python/ray/serve/_private/deployment_state.py

Comment thread python/ray/serve/_private/replica.py

johntaylor-cell and others added 6 commits July 27, 2026 20:00

johntaylor-cell force-pushed the serve-push-core branch from ecd3328 to 16618aa Compare July 27, 2026 20:08