Skip to content

Commit 5d4b3bb

Browse files
[serve] Gate rank-consistency check on replica membership changes (#64911)
## [serve] Gate rank-consistency check on replica membership changes ### Why The Ray Serve controller runs on the head node, continuously reconciling deployments and making autoscaling decisions. `check_rank_consistency_and_reassign_minimally` runs at the tail of every `DeploymentState.update()` — once per deployment per control-loop tick, which at measured loop rates is several times a second. The pass is O(N) in replicas with large constant factors: it materializes the active-key set and the rank-key set, takes set differences both ways, copies the entire `_ranks` dict, tallies per-rank counts across every active key, and sorts all rank values — then the manager above it regroups replicas per node. In steady state every one of those passes re-derives an identical answer. At 16K, py-spy showed it monopolizing the event loop and starving handle-report ingest for 20 s+. Ranks can only become inconsistent when replica membership changes, so gate the pass on the active replica-id set: run it when the set differs from the last checked one, skip when it does not. Transitions are unaffected — membership churn during rollouts and autoscaling changes the set, so the pass still runs exactly when it can matter. The guards that were already there (deployment not HEALTHY, or any STARTING replica) run first, before the replica list is materialized. `RAY_SERVE_FAIL_ON_RANK_ERROR` defaults off, so in production the pass can log an error and return a safe default. When that happens the membership is deliberately **not** cached and the check is retried next tick, rather than latching a membership the pass never validated. The error flag is read before the reassignment reconfigure, which calls back into the rank manager and resets it. The gate's own cost is the remaining O(N) here: building the id set measures 1.9 ms at 16,384 replicas, ~4.5% of the optimized loop. #64910, stacked on this PR, replaces the set comparison with a `ReplicaStateContainer` mutation-version integer, which removes it. ### What - Extract the pass into `_maybe_check_rank_consistency()`. - Gate it on the set of active replica ids: if the set is unchanged since the pass last ran, skip it. - Only record a membership as checked when the pass did not swallow an error. `RAY_SERVE_FAIL_ON_RANK_ERROR` is off by default, so the pass can catch an exception and return `[]`; caching that would mean a stable-but-inconsistent deployment never retries. `DeploymentRankManager` now records whether the last rank op errored. - The existing guards are preserved verbatim: only when the deployment is HEALTHY, and never while STARTING replicas exist (the node-migration case documented in the original comment). ### Why this is safe Rank consistency can only be violated by a membership change — replicas added, removed, or replaced — and every such change alters the active id set, so the pass still runs on exactly the ticks where it can find work. Rank release and removal from `_replicas` happen in the same `pop(states=[STOPPING])` branch, so the id set and the rank table cannot drift apart across ticks. The per-node and node-rank passes group off `_replica_to_node`, which is only mutated by rank assign/recover/release, so a fixed replica-id set implies a fixed node grouping. Set comparison is exact — there is no fingerprint collision to reason about. The residual cost is the O(N) set construction itself (~1.9 ms/tick at 16K replicas); the follow-up #64910 replaces it with an exact `ReplicaStateContainer` mutation counter, making the gate O(1). ### Results - Follow-up profile of the same 16K workload: the rank pass no longer appears in controller CPU samples; report ingest resumes (stale-report drops stop). - Composite (with the rest of the optimization stack): 16K steady-state control loop 352.6 ms → 42.9 ms. ### Testing - 6 new unit tests. Four over the gate itself: runs once then skips on stable membership; re-runs on membership change; a swallowed rank error is not cached and retries next tick; skips entirely (without recording a membership) while STARTING replicas exist. Two over a real `DeploymentRankManager(fail_on_rank_error=False)` — the production default — covering that a swallowed error is not cached, and that the error flag is read before `_reconfigure_replicas_with_new_ranks` can clear it. - Full `test_deployment_state.py` suite green (237 tests). <img width="1248" height="728" alt="perf_A6_loop_haproxy_on" src="https://github.com/user-attachments/assets/34758e31-6c04-46f9-958c-d6b34d3eae32" /> Signed-off-by: john.taylor <john.taylor@anyscale.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 286886c commit 5d4b3bb

2 files changed

Lines changed: 139 additions & 11 deletions

File tree

python/ray/serve/_private/deployment_state.py

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,6 +2934,7 @@ def __init__(
29342934
self._rank_manager = DeploymentRankManager(
29352935
fail_on_rank_error=RAY_SERVE_FAIL_ON_RANK_ERROR
29362936
)
2937+
self._last_rank_membership_ids: Optional[Set[str]] = None
29372938

29382939
self.replica_average_ongoing_requests: Dict[str, float] = {}
29392940

@@ -4972,24 +4973,41 @@ def check_and_update_replicas(self):
49724973
# if we delay the rank reassignment, the rank system will be in an invalid state
49734974
# for a longer period of time. Abrar made this decision because he is not confident
49744975
# about how rollouts work in the deployment state machine.
4975-
active_replicas = self._replicas.get()
4976+
self._maybe_check_rank_consistency()
4977+
4978+
def _maybe_check_rank_consistency(self) -> None:
4979+
"""Run the rank-consistency pass only when replica membership changed.
4980+
4981+
The pass is O(N) with heavy constants and used to run on every
4982+
control-loop tick in steady state -- at 10K+ replicas it monopolizes
4983+
the controller loop. Rank consistency can only be violated by
4984+
membership changes, so the active replica-id set gates it.
4985+
"""
4986+
# O(1) guards first -- `get()` below copies the whole replica list, and these
4987+
# two rule out the busiest ticks (rollouts, migrations) without paying for it.
49764988
if (
4977-
active_replicas
4978-
and self._curr_status_info.status == DeploymentStatus.HEALTHY
4989+
self._curr_status_info.status != DeploymentStatus.HEALTHY
49794990
# Skip consistency check if there are STARTING replicas. During node
49804991
# migration, new replicas are created in STARTING state (without ranks)
49814992
# after the status is set to HEALTHY. Running the consistency check
49824993
# with STARTING replicas causes "active keys without ranks" error.
4983-
and self._replicas.count(states=[ReplicaState.STARTING]) == 0
4994+
or self._replicas.count(states=[ReplicaState.STARTING]) != 0
49844995
):
4985-
replicas_to_reconfigure = (
4986-
self._rank_manager.check_rank_consistency_and_reassign_minimally(
4987-
active_replicas,
4988-
)
4996+
return
4997+
active_replicas = self._replicas.get()
4998+
if not active_replicas:
4999+
return
5000+
active_replica_ids = {r.replica_id.unique_id for r in active_replicas}
5001+
if active_replica_ids == self._last_rank_membership_ids:
5002+
return
5003+
replicas_to_reconfigure = (
5004+
self._rank_manager.check_rank_consistency_and_reassign_minimally(
5005+
active_replicas,
49895006
)
4990-
4991-
# Reconfigure replicas that had their ranks reassigned
4992-
self._reconfigure_replicas_with_new_ranks(replicas_to_reconfigure)
5007+
)
5008+
# Reconfigure replicas that had their ranks reassigned
5009+
self._reconfigure_replicas_with_new_ranks(replicas_to_reconfigure)
5010+
self._last_rank_membership_ids = active_replica_ids
49935011

49945012
def _handle_deployment_actor_failed_health_check(
49955013
self,

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10585,5 +10585,115 @@ def _make_app(name):
1058510585
assert dsm.is_ready_for_shutdown()
1058610586

1058710587

10588+
def _scale_to(dsm, ds, num_replicas, version="1", ticks=14):
10589+
"""Change membership via the public deploy path and settle back to HEALTHY."""
10590+
info, _ = deployment_info(num_replicas=num_replicas, version=version)
10591+
dsm.deploy(TEST_DEPLOYMENT_ID, info)
10592+
for _ in range(ticks):
10593+
dsm.update()
10594+
for replica in ds._replicas.get([ReplicaState.STARTING]):
10595+
replica._actor.set_ready()
10596+
if (
10597+
ds._curr_status_info.status == DeploymentStatus.HEALTHY
10598+
and ds._replicas.count(states=[ReplicaState.STARTING]) == 0
10599+
):
10600+
dsm.update()
10601+
return
10602+
raise AssertionError(
10603+
"deployment never settled: status=%s running=%d starting=%d"
10604+
% (
10605+
ds._curr_status_info.status,
10606+
ds._replicas.count(states=[ReplicaState.RUNNING]),
10607+
ds._replicas.count(states=[ReplicaState.STARTING]),
10608+
)
10609+
)
10610+
10611+
10612+
class CountingRankManager(ds_mod.DeploymentRankManager):
10613+
"""Real rank manager that records how often the consistency pass runs.
10614+
10615+
Injected at the same seam the fixture uses for actor wrappers, so the rank logic under
10616+
test stays real; only the call count is added.
10617+
"""
10618+
10619+
instances = []
10620+
10621+
def __init__(self, *args, **kwargs):
10622+
super().__init__(*args, **kwargs)
10623+
self.consistency_calls = 0
10624+
CountingRankManager.instances.append(self)
10625+
10626+
def check_rank_consistency_and_reassign_minimally(self, active_replicas):
10627+
self.consistency_calls += 1
10628+
return super().check_rank_consistency_and_reassign_minimally(active_replicas)
10629+
10630+
10631+
@pytest.fixture
10632+
def rank_gate_dsm(mock_deployment_state_manager, monkeypatch):
10633+
"""A running deployment whose rank manager counts consistency passes."""
10634+
CountingRankManager.instances = []
10635+
monkeypatch.setattr(ds_mod, "DeploymentRankManager", CountingRankManager)
10636+
10637+
create_dsm, timer, _, _ = mock_deployment_state_manager
10638+
dsm: DeploymentStateManager = create_dsm()
10639+
info, v1 = deployment_info(num_replicas=3, version="1")
10640+
assert dsm.deploy(TEST_DEPLOYMENT_ID, info)
10641+
ds = dsm._deployment_states[TEST_DEPLOYMENT_ID]
10642+
10643+
dsm.update()
10644+
for replica in ds._replicas.get():
10645+
replica._actor.set_ready()
10646+
dsm.update() # STARTING -> RUNNING
10647+
check_counts(ds, total=3, by_state=[(ReplicaState.RUNNING, 3, v1)])
10648+
10649+
# Settle: the deployment reaches HEALTHY a tick or two after the replicas do, and the
10650+
# gate legitimately runs once for this membership. Tests measure from after that.
10651+
for _ in range(8):
10652+
dsm.update()
10653+
assert ds._curr_status_info.status == DeploymentStatus.HEALTHY
10654+
assert (
10655+
ds._rank_manager.consistency_calls >= 1
10656+
), "gate never ran for a new membership"
10657+
return dsm, ds, ds._rank_manager, timer
10658+
10659+
10660+
class TestRankConsistencyMembershipGate:
10661+
"""The rank-consistency pass runs only when replica membership changes."""
10662+
10663+
def test_skips_while_membership_unchanged(self, rank_gate_dsm):
10664+
dsm, _, rank_manager, _ = rank_gate_dsm
10665+
after_startup = rank_manager.consistency_calls
10666+
for _ in range(5):
10667+
dsm.update()
10668+
assert rank_manager.consistency_calls == after_startup
10669+
10670+
def test_reruns_when_a_replica_leaves(self, rank_gate_dsm):
10671+
dsm, ds, rank_manager, timer = rank_gate_dsm
10672+
before = rank_manager.consistency_calls
10673+
10674+
# Membership change through the public path: scale up adds a new replica id.
10675+
_scale_to(dsm, ds, 4)
10676+
10677+
assert rank_manager.consistency_calls > before
10678+
10679+
def test_starting_replicas_skip_the_pass(
10680+
self, mock_deployment_state_manager, monkeypatch
10681+
):
10682+
"""A STARTING replica has no rank yet, so running the pass would raise
10683+
"active keys without ranks"; the guard must skip before that."""
10684+
CountingRankManager.instances = []
10685+
monkeypatch.setattr(ds_mod, "DeploymentRankManager", CountingRankManager)
10686+
create_dsm, _, _, _ = mock_deployment_state_manager
10687+
dsm: DeploymentStateManager = create_dsm()
10688+
info, _ = deployment_info(num_replicas=2, version="1")
10689+
assert dsm.deploy(TEST_DEPLOYMENT_ID, info)
10690+
ds = dsm._deployment_states[TEST_DEPLOYMENT_ID]
10691+
10692+
dsm.update() # replicas are STARTING, never marked ready
10693+
dsm.update()
10694+
check_counts(ds, total=2, by_state=[(ReplicaState.STARTING, 2, None)])
10695+
assert ds._rank_manager.consistency_calls == 0
10696+
10697+
1058810698
if __name__ == "__main__":
1058910699
sys.exit(pytest.main(["-v", "-s", __file__]))

0 commit comments

Comments
 (0)