Commit 8b6f025
committed
[serve] Push-based replica health: self-checks ride replica metric reports
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 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 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>1 parent e1eaf64 commit 8b6f025
10 files changed
Lines changed: 1224 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1061 | 1061 | | |
1062 | 1062 | | |
1063 | 1063 | | |
| 1064 | + | |
| 1065 | + | |
| 1066 | + | |
| 1067 | + | |
| 1068 | + | |
1064 | 1069 | | |
1065 | 1070 | | |
1066 | 1071 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| |||
254 | 255 | | |
255 | 256 | | |
256 | 257 | | |
| 258 | + | |
257 | 259 | | |
258 | 260 | | |
259 | 261 | | |
260 | 262 | | |
261 | 263 | | |
262 | 264 | | |
263 | 265 | | |
| 266 | + | |
264 | 267 | | |
265 | 268 | | |
266 | 269 | | |
| |||
391 | 394 | | |
392 | 395 | | |
393 | 396 | | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
394 | 405 | | |
395 | 406 | | |
396 | 407 | | |
397 | 408 | | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
398 | 421 | | |
399 | 422 | | |
400 | 423 | | |
| |||
619 | 642 | | |
620 | 643 | | |
621 | 644 | | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
622 | 653 | | |
623 | 654 | | |
624 | 655 | | |
| |||
Lines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
53 | 60 | | |
54 | 61 | | |
55 | 62 | | |
| |||
145 | 152 | | |
146 | 153 | | |
147 | 154 | | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
148 | 159 | | |
0 commit comments