Skip to content

Commit 25182fc

Browse files
[serve] Cache per-tick replica id walks behind a container membership 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. Gang deployments health-check through that same pop-and-re-add path on every tick, so for them a counter never holds at all. Summing per-replica terms makes pop-and-re-add cancel exactly, while a real arrival, departure or state transition still changes it. The same key replaces the id-set key of the rank-consistency gate, which keeps that gate O(1) per tick without losing the content-based semantics that let it hold under churn. It changes strictly more often than the id set it replaces -- it includes state -- so the gate can never skip a pass the id set would have run. It is paired with the exact replica count, because unlike an id-set comparison it is a hash: memberships of the same size collide with probability ~2^-64, and while a stale memo self-corrects on the next real change, a skipped rank pass would not. Caching disarms while STARTING, UPDATING or RECOVERING replicas exist: the startup check calls check_ready() for all three, and that rewrites actor_id/node_id in place without a membership change. One delta from the gate as it stands in the base PR: the empty-membership early return now caches before returning. 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 autoscaler's identity skip is safe because _running_replicas and _cached_running_replica_strs are only ever written together and both start empty, so the pair cannot desync -- note that re-registering a deployment reuses the existing DeploymentAutoscalingState rather than presenting a new list object. Signed-off-by: john.taylor <john.taylor@anyscale.com> Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 05770ef commit 25182fc

4 files changed

Lines changed: 473 additions & 41 deletions

File tree

python/ray/serve/_private/autoscaling_state.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,18 @@
33
import math
44
import time
55
from collections import defaultdict
6-
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Set, Tuple, Union
6+
from typing import (
7+
TYPE_CHECKING,
8+
Any,
9+
Callable,
10+
Dict,
11+
List,
12+
Optional,
13+
Sequence,
14+
Set,
15+
Tuple,
16+
Union,
17+
)
718

819
from ray.serve._private.common import (
920
RUNNING_REQUESTS_KEY,
@@ -93,7 +104,7 @@ def __init__(self, deployment_id: DeploymentID):
93104
# user defined policy returns a dictionary of state that is persisted between autoscaling decisions
94105
# content of the dictionary is determined by the user defined policy
95106
self._policy_state: Optional[Dict[str, Any]] = None
96-
self._running_replicas: List[ReplicaID] = []
107+
self._running_replicas: Sequence[ReplicaID] = []
97108
self._cached_running_replica_strs: Set[str] = set()
98109
self._target_capacity: Optional[float] = None
99110
self._target_capacity_direction: Optional[TargetCapacityDirection] = None
@@ -208,8 +219,12 @@ def get_num_replicas_upper_bound(self) -> int:
208219
self._target_capacity,
209220
)
210221

211-
def update_running_replica_ids(self, running_replicas: List[ReplicaID]):
222+
def update_running_replica_ids(self, running_replicas: Sequence[ReplicaID]):
212223
"""Update cached set of running replica IDs for this deployment."""
224+
if running_replicas is self._running_replicas:
225+
# Identity means DeploymentState handed back its memoized tuple, so the
226+
# running-replica ids are unchanged and the derived str set still matches.
227+
return
213228
self._running_replicas = running_replicas
214229
self._cached_running_replica_strs = {
215230
r.to_full_id_str() for r in running_replicas
@@ -395,7 +410,9 @@ def get_autoscaling_context(
395410
app_name=self._deployment_id.app_name,
396411
current_num_replicas=len(self._running_replicas),
397412
target_num_replicas=curr_target_num_replicas,
398-
running_replicas=self._running_replicas,
413+
# Copy: _running_replicas holds the memoized tuple from
414+
# get_running_replica_ids(), and this is a stable public List[ReplicaID].
415+
running_replicas=list(self._running_replicas),
399416
total_num_requests=self.get_total_num_requests,
400417
capacity_adjusted_min_replicas=self.get_num_replicas_lower_bound(),
401418
capacity_adjusted_max_replicas=self.get_num_replicas_upper_bound(),
@@ -1013,7 +1030,7 @@ def get_decision_num_replicas(
10131030
}
10141031

10151032
def update_running_replica_ids(
1016-
self, deployment_id: DeploymentID, running_replicas: List[ReplicaID]
1033+
self, deployment_id: DeploymentID, running_replicas: Sequence[ReplicaID]
10171034
):
10181035
self._deployment_autoscaling_states[deployment_id].update_running_replica_ids(
10191036
running_replicas
@@ -1183,7 +1200,7 @@ def should_autoscale_deployment(self, deployment_id: DeploymentID):
11831200
)
11841201

11851202
def update_running_replica_ids(
1186-
self, deployment_id: DeploymentID, running_replicas: List[ReplicaID]
1203+
self, deployment_id: DeploymentID, running_replicas: Sequence[ReplicaID]
11871204
):
11881205
app_state = self._app_autoscaling_states.get(deployment_id.app_name)
11891206
if app_state:

python/ray/serve/_private/controller.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,9 @@ def _update_proxy_nodes(self):
533533
Controller decides where proxy actors should run
534534
(head node and nodes with deployment replicas).
535535
"""
536-
new_proxy_nodes = self.deployment_state_manager.get_active_node_ids()
536+
# Copy: get_active_node_ids() returns a shared frozenset, and `frozenset - set`
537+
# stays frozen, so the .add() below would raise without this.
538+
new_proxy_nodes = set(self.deployment_state_manager.get_active_node_ids())
537539
new_proxy_nodes = new_proxy_nodes - set(
538540
self.cluster_node_info_cache.get_draining_nodes()
539541
)

0 commit comments

Comments
 (0)