[serve] Carry replica self-health on handle metric reports - #64912
[serve] Carry replica self-health on handle metric reports#64912johntaylor-cell wants to merge 9 commits into
Conversation
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>
There was a problem hiding this comment.
Code Review
This pull request introduces a replica-pushed self-health mechanism in Ray Serve, allowing replicas to periodically evaluate and push their health status to the controller to bypass pull probes. It also optimizes rank-consistency checks by gating them on replica membership changes. The review feedback focuses on critical performance and reliability improvements: optimizing the pruning logic in ReplicaHealthPushRegistry to run periodically rather than on every record() call under high scale, falling back to a default health check period when target state info is unavailable to prevent false systemic stall verdicts, and bypassing heartbeat suppression when a replica is unhealthy to ensure immediate failure detection.
855a530 to
c39f9d0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit c39f9d0. Configure here.
c39f9d0 to
5065301
Compare
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>
…ersion A CPU profile of a controller managing 16K replicas showed the loop dominated by O(N) id-collection walks recomputed every tick (alive actor ids, active node ids, running replica ids, the autoscaler id-set rebuild) even though membership rarely changes in steady state. ReplicaStateContainer now bumps a mutation version on add / non-empty pop / non-empty remove, and the derived collections memoize on it at both DeploymentState and DeploymentStateManager level. Caching disarms while STARTING/RECOVERING replicas exist since their actor/node ids materialize in place. The autoscaler skips its id-set rebuild when handed the same (cached) list object, and the rank-consistency fingerprint becomes the version integer. Benchmark at 16K replicas: control loop 352.6ms -> 42.9ms. 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 (half the health-check period) and push the result: on the metric reports they already send when those are at least as frequent, 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. Signed-off-by: john.taylor <john.taylor@anyscale.com>
Review: the over-threshold prune re-ran an O(N) dict rebuild on every record() once all entries were fresh; now at most once per 30s. The stall window defaults to the stock health-check period so deployments without target info (recovery, deletion) cannot misread every push as stale. Signed-off-by: john.taylor <john.taylor@anyscale.com>
Review: suppression assumed a recent report/response carried current health, but a status flip to unhealthy after the last carry would be delayed a full period. Gate suppression on healthy so unhealthy reaches the controller promptly; unhealthy is rare, so the extra heartbeat is negligible. Signed-off-by: john.taylor <john.taylor@anyscale.com>
Review: the dirty-set health-checks only a slice (~N/sweep_ticks) each tick, but the stall guard compared its 64-sample floor against that one slice -- so it only engaged above ~3200 replicas per deployment. Accumulate the tally across ticks and finalize once enough replicas are sampled or a full sweep elapses; a large deployment whose one-tick slice already exceeds the floor still finalizes next tick, so engagement stays prompt where it matters. Extract _reconcile_sweep_ticks (shared with the dirty set). Known follow-up: many small deployments can still each stay under the per-deployment floor while the controller lags overall. Signed-off-by: john.taylor <john.taylor@anyscale.com>
P2 of the push-health stack. Response admission piggybacks the replica self-check on the queue-length info; routers keep the newest entry per replica (age-pruned) and fold it -- plus the host replica own health via a process-local provider -- into the handle metric reports they already send. Handle-owning replicas and replicas behind active handles need no heartbeat RPC; consumption stamps handle-carried recency so the P1 heartbeat stays suppressed. Signed-off-by: john.taylor <john.taylor@anyscale.com>
…sses carriage suppression Review (cursor + gemini): self_health_snapshot stamped handle-carried recency even when _health_kwargs sent nothing (checked_at unset), suppressing a fresh replica first heartbeat; now stamps only when health is actually carried. Also gate the response/handle-report suppression clauses on healthy so an unhealthy flip is never delayed a full period. Signed-off-by: john.taylor <john.taylor@anyscale.com>
5065301 to
e9e3177
Compare
|
Abandoning |

[serve] Carry replica self-health on handle metric reports
Second PR of the push-health stack (stacked on #64913).
Why
With P1, replicas that push autoscaling metric reports get health delivery for
free, and everyone else heartbeats. But the most common serving topology —
replicas called through deployment handles — already has another message flowing
to the controller on a fixed cadence: the handle metric report each router
sends for autoscaling. This PR folds replica health into those reports, removing
the P1 heartbeat for two classes of replica:
already return;
health rides the handle reports their in-process router already sends.
No new messages, no new RPCs — strictly more payload on existing ones.
What
ReplicaQueueLengthInfo(therejection-protocol admission message) carries the replica's latest self-check
(healthy, checked_at, consecutive_failures) on both accept and reject.
(out-of-order responses resolved by
checked_at; the recording wall-clock iskept alongside so age decisions never compare across machine clocks), prunes
entries for departed replicas on membership updates, and drops entries not
refreshed within 30 s — the controller already has those, and re-sending them
bloats every report at high fan-in.
routers living inside that replica merge the host's own health into their
handle reports. The merge is exception-isolated — health can never break the
metrics path.
P1 introduced; from there the existing freshness/fallback logic applies
unchanged.
Suppression (no double delivery, no lost heartbeats)
(
reports_carry_healthgate) — one channel per fact.P1 heartbeat; if handle reports stop (traffic ceases, handle closed), the
stamp ages out and the heartbeat resumes on its own. Fail-open in both
directions.
Testing
(newest-wins, age-prune, membership prune, provider exception isolation);
controller forwarding into the registry.
test_deployment_state.py+test_metrics_utils.py+test_autoscaling_policy.pygreen on this tier (375 tests).
Benchmarks
Full-stack numbers are in #64909's table (8K/16K, HAProxy off: 52.1/42.9 ms
loops, ≤0.55% health-deadline misses). This PR's isolated contribution
(P1-only vs P1+P2 arms on the same harness) is being measured and lands here
before review.