[serve] Cache per-tick replica id walks behind a container mutation version - #64910
[serve] Cache per-tick replica id walks behind a container mutation version#64910johntaylor-cell wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces performance optimizations to Ray Serve's deployment state management by caching and memoizing expensive per-tick id-collection walks (such as alive replica actor IDs, running replica IDs, and active node IDs) using a mutation version tracker on the replica container. It also optimizes the rank-consistency check to run only when replica membership changes. The reviewer feedback focuses on ensuring backward compatibility during rolling upgrades or controller recoveries by defensively using getattr to access newly introduced attributes on deserialized objects, and preventing state corruption by returning immutable collections (frozenset and tuple) from cached methods.
2c9f04a to
b5ff9be
Compare
0073e32 to
a110642
Compare
d169329 to
abe8f6a
Compare
a846bb0 to
78aa66d
Compare
78aa66d to
c7f6a48
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 c7f6a48. Configure here.
66cf6ce to
6933a3b
Compare
The rank-consistency pass is O(N) with heavy constants and used to run on every control-loop tick; at 10K+ replicas it monopolizes the controller loop. Rank consistency can only be violated by a membership change, so gate the pass on the active replica-id set and skip it while that set is unchanged. Two O(1) guards run first -- status must be HEALTHY and there must be no STARTING replicas -- which rule out the busiest ticks before paying for the replica-list copy. A pass whose error was swallowed must not be cached as validated, or an unvalidated membership would never be rechecked: fail_on_rank_error is off in production, so the pass can log and return a safe default. Such a membership stays uncached and is retried, rate-limited while it still matches the one that failed, and a real membership change bypasses the rate limit and runs immediately. The errored flag is snapshotted before the reconfigure, which calls back into the rank manager and resets it. An errored pass also drops the membership that was last validated, not just its own. The equality guard runs ahead of the error backoff, so a membership validated before a failed pass would otherwise be skipped for good once membership returned to it, and an upscale followed by a downscale is enough to return to it. That matters because the pass reassigns global ranks before it checks per-node ones: a failure part way through can leave ranks it already moved while the swallowed error reports no replicas to reconfigure, so what an earlier pass validated no longer holds. The gate's tests inject a counting rank manager at the same seam the fixture already uses for actor wrappers, so the rank logic under test stays real and only the call count is added. That fake pins the swallow path, because _execute_with_error_handling re-raises when fail_on_rank_error is on and four CI variants of this file set it to 1. Signed-off-by: john.taylor <john.taylor@anyscale.com> Co-Authored-By: Claude <noreply@anthropic.com>
6933a3b to
e9917d9
Compare
… 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 and of its error
backoff, so both keep an O(1) key without losing the content-based semantics that made
them 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, and unlike an id-set comparison it is a hash rather
than an exact equality, so memberships of the same size collide with probability ~2^-64.
A stale memo self-corrects on the next real change; a skipped rank pass would not, which
is why the count is carried alongside.
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.
Two deltas from the gate as it stands in the base PR: the empty-membership early return
now caches before returning, and the fingerprint is what an errored pass clears.
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>
e9917d9 to
0465c10
Compare

Stacked on #64911 (the rank-consistency
membership gate; this PR replaces its per-tick id-set rebuild with the container key
below).
Why
A 200 Hz CPU profile of a controller managing 16K replicas showed the control loop
dominated not by any single pass but by five separate O(N) id-collection walks
recomputed every tick, while metric-report ingest (<1% of samples) starved behind
them:
DeploymentState.get_alive_replica_actor_ids— called every tick forstale-handle-metric pruning
DeploymentState.get_active_node_ids— every tick via_update_proxy_nodesDeploymentState.get_running_replica_ids— every tick for autoscaler registrationto_full_id_str()× N, every tick)#64911 — an id-set rebuild over N
replicas, after copying the full replica list
Each walk drags N replica objects through Python property chains to recompute a
result that only changes when membership changes — and in steady state membership
doesn't change for hours.
What
ReplicaStateContainergains a membership key: the summed per-replica hash of its{replica id -> state}content, maintained incrementally onadd/pop/remove, andreturned paired with the exact replica count. It is a hash of the content rather than a
counter of mutations because the health and migration passes pop a whole bucket and
re-add every replica to the state it came from: gang deployments take that path on
every tick, and every other deployment takes it whenever any node in the cluster is
draining, which is 5 mutations per otherwise-idle tick at 4 replicas. A counter reads
that churn as a membership change and gives back none of the win. Summing per-replica
terms makes pop-and-re-add cancel exactly, while a real arrival, departure or state
transition still changes it.
of the same size collide with probability ~2^-64. The pairing with the exact count
removes every collision where cardinality differs. The asymmetry worth knowing is that
a stale memo self-corrects on the next real membership change, whereas a skipped rank
pass would not, which is why the count is carried and why the fingerprint is pinned to
a full recompute in the tests.
DeploymentStateandDeploymentStateManagerlevel, through one shared_memoized_walkhelper.startup check calls
check_ready()for all three, and that rewritesactor_id/node_idin place without a container mutation. During ramp, rollout andrecovery every getter recomputes exactly as before; the cache engages only at steady
state, which is where the waste was.
already processed. That is safe because
_running_replicasand_cached_running_replica_strsare only ever written together and both start empty, sothe pair cannot desync — note that re-registering a deployment reuses the existing
DeploymentAutoscalingStaterather than presenting a new list object.error backoff both become this key — no walk at all, and the gate's check now sits
before the
get()list copy instead of after it.Two deliberate deltas from the gate as it stands in #64911: the empty-membership early
return now caches before returning, and the errored-pass path clears the key.
Cached collections are shared objects, so the getters return them frozen
(
FrozenSet/Tuple) and the one caller that needs a mutable set copies at its ownboundary; every other caller only reads them.
Results
Isolated microbench of the three
DeploymentStatewalks at 16K replicas:3.23 ms → 0.008 ms per tick (real
DeploymentReplicaproperty chains costmore than the benchmark stand-ins, so live savings are larger).
On the full harness (16K replicas, single deployment, pinned, steady state,
36 samples): control loop 352.6 ms → 42.9 ms, deployment-state update
253.8 ms → 17.1 ms, 1.36 → 4.99 loops/s — and the first run on this
harness to place all 16,384 replicas and complete cleanly; without this change
the controller starves its own report ingest at that scale. (Composite numbers
measured with the push-health stack on top; isolated this-PR-vs-base cells on the
same harness are running and will replace this table before review.)
Testing
nor does popping a bucket and re-adding every replica to the state it came from, while
a transition, an arrival and a departure each do. A separate test recomputes the key
from every bucket and asserts it matches after a mixed add/pop/remove/re-bucket
sequence, so a future bucket write that forgets to maintain it fails loudly.
caching; autoscaler identity-skip (same object skipped, new object rebuilt).
gate both hold across ticks with a node draining elsewhere in the cluster, and across
steady-state ticks of a gang deployment. Each of these witnesses the churn through
bucket-list identity so none can pass vacuously, and each is verified to fail if the
key's per-replica terms are made non-cancelling.
before a failure is rechecked rather than skipped forever.
test_deployment_state.py+test_autoscaling_policy.py: 287 tests, green under thedefault env and all four CI env variants (
_with_pack_scheduling,_metr_disab,_metr_agg_at_controller,_metr_agg_at_controller_and_replicas).