Skip to content

Commit ecd3328

Browse files
Accumulate push-stall tally across ticks, not per dirty-set slice
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>
1 parent 143b44c commit ecd3328

2 files changed

Lines changed: 73 additions & 15 deletions

File tree

python/ray/serve/_private/deployment_state.py

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3113,6 +3113,7 @@ def __init__(
31133113
self._push_stall_since: Optional[float] = None
31143114
self._push_stall_stale: int = 0
31153115
self._push_stall_total: int = 0
3116+
self._push_stall_ticks: int = 0
31163117
# Default to the stock period so pushes are not misread as stale on
31173118
# deployments that have no target info yet (recovery, deletion).
31183119
self._push_stall_window_s: float = _push_freshness_window_s(
@@ -4975,15 +4976,7 @@ def _dirty_set_active_pairs(self):
49754976
self._outstanding_dirty_set = still
49764977
n = container.count_state(ReplicaState.RUNNING)
49774978
if n:
4978-
period = self._reconcile_sweep_period_s()
4979-
ticks = max(
4980-
1,
4981-
int(
4982-
CONTROLLER_HEALTH_CHECK_RECONCILIATION_FRACTION
4983-
* period
4984-
/ max(CONTROL_LOOP_INTERVAL_S, 1e-3)
4985-
),
4986-
)
4979+
ticks = self._reconcile_sweep_ticks()
49874980
slice_n = max(1, (n + ticks - 1) // ticks)
49884981
start = self._dirty_set_rr_cursor % n
49894982
sl = container.slice_state(ReplicaState.RUNNING, start, slice_n)
@@ -5017,10 +5010,42 @@ def _reconcile_sweep_period_s(self) -> float:
50175010
DEFAULT_HEALTH_CHECK_PERIOD_S, DEFAULT_REQUEST_ROUTING_STATS_PERIOD_S
50185011
)
50195012

5013+
def _reconcile_sweep_ticks(self) -> int:
5014+
"""Control-loop ticks the dirty-set takes to sweep the RUNNING bucket
5015+
once (see _dirty_set_active_pairs)."""
5016+
return max(
5017+
1,
5018+
int(
5019+
CONTROLLER_HEALTH_CHECK_RECONCILIATION_FRACTION
5020+
* self._reconcile_sweep_period_s()
5021+
/ max(CONTROL_LOOP_INTERVAL_S, 1e-3)
5022+
),
5023+
)
5024+
5025+
def _advance_push_stall_window(self) -> None:
5026+
"""Finalize the systemic-stall verdict once enough replicas have been
5027+
sampled across ticks (or a full sweep elapsed), then reset the tally.
5028+
5029+
Each tick only health-checks a dirty-set slice (~N/sweep_ticks), well
5030+
below the guard's min-tracked floor for all but the largest deployments;
5031+
accumulating across ticks lets the guard reach a deployment-representative
5032+
sample. A large deployment whose one-tick slice already exceeds the floor
5033+
still finalizes next tick, so engagement stays prompt where it matters.
5034+
"""
5035+
self._push_stall_ticks += 1
5036+
if (
5037+
self._push_stall_total >= HEALTH_PUSH_STALL_MIN_TRACKED
5038+
or self._push_stall_ticks >= self._reconcile_sweep_ticks()
5039+
):
5040+
self._finalize_push_stall_verdict()
5041+
self._push_stall_stale = 0
5042+
self._push_stall_total = 0
5043+
self._push_stall_ticks = 0
5044+
50205045
def _finalize_push_stall_verdict(self) -> None:
5021-
"""Set next sweep's probe-deferral from this sweep's staleness tally.
5046+
"""Set probe-deferral from the accumulated staleness tally.
50225047
5023-
Most of this deployment's pushed health going stale at once (by its own
5048+
Most of this deployment's sampled pushes going stale at once (by its own
50245049
window) reads as controller-side ingest lag, so fallback probes are
50255050
deferred -- bounded by HEALTH_PUSH_STALL_MAX_DEFER_S per episode so a
50265051
genuine mass outage still falls through to probes.
@@ -5071,9 +5096,7 @@ def check_and_update_replicas(self):
50715096
with state container from previous update() cycle to see if any state
50725097
transition happened.
50735098
"""
5074-
self._finalize_push_stall_verdict()
5075-
self._push_stall_stale = 0
5076-
self._push_stall_total = 0
5099+
self._advance_push_stall_window()
50775100
if self._target_state.info is not None:
50785101
self._push_stall_window_s = _push_freshness_window_s(
50795102
self._target_state.info.deployment_config.health_check_period_s

python/ray/serve/tests/unit/test_deployment_state.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9791,10 +9791,11 @@ def _shim(self, container):
97919791
s._outstanding_dirty_set = set()
97929792
s._dirty_set_rr_cursor = 0
97939793
s._target_state = object() # .info access raises -> default reconcile period
9794-
# Bind the real period helper so _dirty_set_active_pairs can call it on the shim.
9794+
# Bind the real period/ticks helpers so _dirty_set_active_pairs can call them.
97959795
s._reconcile_sweep_period_s = DeploymentState._reconcile_sweep_period_s.__get__(
97969796
s
97979797
)
9798+
s._reconcile_sweep_ticks = DeploymentState._reconcile_sweep_ticks.__get__(s)
97989799
return s
97999800

98009801
def test_covers_all_running_within_one_sweep(self):
@@ -10246,6 +10247,40 @@ def test_majority_stale_defers_next_sweep(self):
1024610247
ds._apply_pushed_health(replica)
1024710248
replica.defer_push_fallback_probe.assert_called_once()
1024810249

10250+
def test_accumulates_across_ticks_below_slice_threshold(self):
10251+
# Each tick samples only a small slice (< min tracked), but accumulation
10252+
# across ticks reaches the sample size, so systemic lag still engages.
10253+
ds = self._ds()
10254+
ds._push_stall_ticks = 0
10255+
ds._reconcile_sweep_ticks = lambda: 1000 # long sweep -> threshold path
10256+
for _ in range(8): # 8 * 10 = 80 >= HEALTH_PUSH_STALL_MIN_TRACKED
10257+
self._seed(ds, 10, 10)
10258+
self._sweep(ds, 10)
10259+
ds._advance_push_stall_window()
10260+
assert ds._push_probes_deferred
10261+
10262+
def test_tiny_deployment_never_engages_even_accumulated(self):
10263+
# A full sweep accumulates < min tracked -> finalize bails, never defers.
10264+
ds = self._ds()
10265+
ds._push_stall_ticks = 0
10266+
ds._reconcile_sweep_ticks = lambda: 3
10267+
for _ in range(3):
10268+
self._seed(ds, 10, 10)
10269+
self._sweep(ds, 10)
10270+
ds._advance_push_stall_window()
10271+
assert not ds._push_probes_deferred
10272+
10273+
def test_large_slice_engages_in_one_tick(self):
10274+
# A single tick whose slice already exceeds the floor finalizes next
10275+
# tick -- prompt engagement preserved for large deployments.
10276+
ds = self._ds()
10277+
ds._push_stall_ticks = 0
10278+
ds._reconcile_sweep_ticks = lambda: 50
10279+
self._seed(ds, 100, 80)
10280+
self._sweep(ds, 100)
10281+
ds._advance_push_stall_window() # total 100 >= floor -> finalized
10282+
assert ds._push_probes_deferred
10283+
1024910284
def test_below_min_tracked_never_defers(self):
1025010285
ds = self._ds()
1025110286
self._tally_and_verdict(ds, n=63, stale_n=63)

0 commit comments

Comments
 (0)