Skip to content

Commit f7765ef

Browse files
Stop computing autoscaling aggregates at the source
Removing simple mode left the producers still computing and shipping the pre-aggregated scalars that nothing reads any more: the replica averaged its running-requests window on every report, and the router averaged queued and per-replica running requests. Drop that work and the three vestigial fields. HandleMetricReport.total_requests keeps its name and its four callers -- it is only used to report how much traffic a dropped handle was carrying -- and is recomputed from the raw series the report already carries. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: John Taylor <john.taylor@anyscale.com>
1 parent 2cad050 commit f7765ef

5 files changed

Lines changed: 13 additions & 149 deletions

File tree

python/ray/serve/_private/common.py

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -989,14 +989,9 @@ class HandleMetricReport:
989989
handle_source: Describes what kind of entity holds this
990990
deployment handle: a Serve proxy, a Serve replica, or
991991
unknown.
992-
aggregated_queued_requests: average number of queued requests at the
993-
handle over the past look_back_period_s seconds.
994992
queued_requests: list of values of queued requests at the
995993
handle over the past look_back_period_s seconds. This is a list because
996994
we take multiple measurements over time.
997-
aggregated_metrics: A map of metric name to the aggregated value over the past
998-
look_back_period_s seconds at the handle for each replica. Replica keys
999-
use ReplicaID.to_full_id_str() for efficient controller-side lookups.
1000995
metrics: A map of metric name to the list of values running at that handle for each replica
1001996
over the past look_back_period_s seconds. Replica keys use to_full_id_str().
1002997
This is a list because we take multiple measurements over time.
@@ -1007,21 +1002,24 @@ class HandleMetricReport:
10071002
handle_id: str
10081003
actor_id: str
10091004
handle_source: DeploymentHandleSource
1010-
aggregated_queued_requests: float
10111005
queued_requests: TimeSeries
1012-
aggregated_metrics: Dict[
1013-
str, Dict[str, float]
1014-
] # replica key = ReplicaID.to_full_id_str()
10151006
metrics: Dict[
10161007
str, Dict[str, TimeSeries]
10171008
] # replica key = ReplicaID.to_full_id_str()
10181009
timestamp: float
10191010

10201011
@property
10211012
def total_requests(self) -> float:
1022-
"""Total number of queued and running requests."""
1023-
return self.aggregated_queued_requests + sum(
1024-
self.aggregated_metrics.get(RUNNING_REQUESTS_KEY, {}).values()
1013+
"""Most recently observed queued + running requests across this handle's
1014+
replicas. Diagnostic only (reported when a handle's metrics are dropped), so
1015+
the latest sample is enough and no windowing is applied."""
1016+
1017+
def latest(series: TimeSeries) -> float:
1018+
return series[-1].value if series else 0.0
1019+
1020+
return latest(self.queued_requests) + sum(
1021+
latest(series)
1022+
for series in self.metrics.get(RUNNING_REQUESTS_KEY, {}).values()
10251023
)
10261024

10271025
@property
@@ -1045,16 +1043,13 @@ class ReplicaMetricReport:
10451043
10461044
Args:
10471045
replica_id: The replica ID of the replica.
1048-
aggregated_metrics: A map of metric name to the aggregated value over the past
1049-
look_back_period_s seconds at the replica.
10501046
metrics: A map of metric name to the list of values running at that replica
10511047
over the past look_back_period_s seconds. This is a list because
10521048
we take multiple measurements over time.
10531049
timestamp: The time at which this report was created.
10541050
"""
10551051

10561052
replica_id: ReplicaID
1057-
aggregated_metrics: Dict[str, float]
10581053
metrics: Dict[str, TimeSeries]
10591054
timestamp: float
10601055

python/ray/serve/_private/replica.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -962,21 +962,12 @@ def _push_autoscaling_metrics(self) -> None:
962962
look_back_period = self._autoscaling_config.look_back_period_s
963963
self._metrics_store.prune_keys_and_compact_data(time.time() - look_back_period)
964964

965-
new_aggregated_metrics = {}
966965
# The store keys are `Hashable`; this replica only ever records `str` keys.
967966
new_metrics = cast(Dict[str, TimeSeries], {**self._metrics_store.data})
968967

969-
if self.should_collect_ongoing_requests():
970-
# Keep the legacy window_avg ongoing requests in the merged metrics dict
971-
window_avg = (
972-
self._metrics_store.aggregate_avg([RUNNING_REQUESTS_KEY])[0] or 0.0
973-
)
974-
new_aggregated_metrics.update({RUNNING_REQUESTS_KEY: window_avg})
975-
976968
replica_metric_report = ReplicaMetricReport(
977969
replica_id=self._replica_id,
978970
timestamp=time.time(),
979-
aggregated_metrics=new_aggregated_metrics,
980971
metrics=new_metrics,
981972
)
982973
with self._metrics_push_lock:

python/ray/serve/_private/router.py

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -491,54 +491,26 @@ def _add_autoscaling_metrics_point(self):
491491
def _get_metrics_report(self) -> HandleMetricReport:
492492
timestamp = time.time()
493493
running_requests = dict()
494-
avg_running_requests = dict()
495494
autoscaling_config = self.autoscaling_config
496495
assert autoscaling_config is not None
497496
look_back_period = autoscaling_config.look_back_period_s
498497
self.metrics_store.prune_keys_and_compact_data(time.time() - look_back_period)
499-
avg_queued_requests = self.metrics_store.aggregate_avg([QUEUED_REQUESTS_KEY])[0]
500-
if avg_queued_requests is None:
501-
# If the queued requests timeseries is empty, we set the
502-
# average to the current number of queued requests.
503-
avg_queued_requests = self.num_queued_requests
504-
# If the queued requests timeseries is empty, we set the number of data points to 1.
505-
# This is to avoid division by zero.
506-
num_data_points = self.metrics_store.timeseries_count(QUEUED_REQUESTS_KEY) or 1
507498
queued_requests = self.metrics_store.data.get(
508499
QUEUED_REQUESTS_KEY, [TimeStampedValue(timestamp, self.num_queued_requests)]
509500
)
510501
if RAY_SERVE_COLLECT_AUTOSCALING_METRICS_ON_HANDLE and self.autoscaling_config:
511502
for replica_id, num_requests in self.num_requests_sent_to_replicas.items():
512-
# Calculate avg running requests.
513-
# NOTE (abrar): The number of data points from queued requests is often higher than
514-
# those from running requests. This is because replica metrics are only collected
515-
# once a replica is up, whereas queued request metrics are collected continuously
516-
# as long as the handle is alive. To approximate the true average of ongoing requests,
517-
# we should normalize by using the same number of data points for both queued and
518-
# running request time series.
519-
running_requests_sum = self.metrics_store.aggregate_sum([replica_id])[0]
520-
if running_requests_sum is None:
521-
# If the running requests timeseries is empty, we set the sum
522-
# to the current number of requests.
523-
running_requests_sum = num_requests
524-
replica_str = replica_id.to_full_id_str()
525-
avg_running_requests[replica_str] = (
526-
running_requests_sum / num_data_points
527-
)
528-
# Get running requests data
529-
running_requests[replica_str] = self.metrics_store.data.get(
503+
running_requests[
504+
replica_id.to_full_id_str()
505+
] = self.metrics_store.data.get(
530506
replica_id, [TimeStampedValue(timestamp, num_requests)]
531507
)
532508
handle_metric_report = HandleMetricReport(
533509
deployment_id=self._deployment_id,
534510
handle_id=self._handle_id,
535511
actor_id=self._self_actor_id,
536512
handle_source=self._handle_source,
537-
aggregated_queued_requests=avg_queued_requests,
538513
queued_requests=queued_requests,
539-
aggregated_metrics={
540-
RUNNING_REQUESTS_KEY: avg_running_requests,
541-
},
542514
metrics={
543515
RUNNING_REQUESTS_KEY: running_requests,
544516
},

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2725,13 +2725,6 @@ def test_autoscaling_with_mixed_deployment_types(
27252725
actor_id="actor_id",
27262726
handle_source=DeploymentHandleSource.UNKNOWN,
27272727
queued_requests=[TimeStampedValue(timestamp_offset, 0)],
2728-
aggregated_queued_requests=0,
2729-
aggregated_metrics={
2730-
RUNNING_REQUESTS_KEY: {
2731-
r1.to_full_id_str(): 3,
2732-
r2.to_full_id_str(): 3,
2733-
}
2734-
},
27352728
metrics={
27362729
RUNNING_REQUESTS_KEY: {
27372730
r1.to_full_id_str(): [TimeStampedValue(timestamp_offset, 3)],
@@ -2745,7 +2738,6 @@ def test_autoscaling_with_mixed_deployment_types(
27452738
for i in [1, 2]:
27462739
replica_report = ReplicaMetricReport(
27472740
replica_id=ReplicaID(unique_id=f"replica_{i}", deployment_id=d1_id),
2748-
aggregated_metrics={RUNNING_REQUESTS_KEY: 3},
27492741
metrics={
27502742
RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 3)]
27512743
},
@@ -2872,7 +2864,6 @@ def test_autoscale_multiple_apps_independent(
28722864
for replica_id in app1_d1_replicas + app1_d2_replicas:
28732865
replica_report = ReplicaMetricReport(
28742866
replica_id=replica_id,
2875-
aggregated_metrics={RUNNING_REQUESTS_KEY: 3},
28762867
metrics={RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 3)]},
28772868
timestamp=time.time(),
28782869
)
@@ -2882,7 +2873,6 @@ def test_autoscale_multiple_apps_independent(
28822873
for replica_id in app2_d1_replicas + app2_d2_replicas:
28832874
replica_report = ReplicaMetricReport(
28842875
replica_id=replica_id,
2885-
aggregated_metrics={RUNNING_REQUESTS_KEY: 0},
28862876
metrics={RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 0)]},
28872877
timestamp=time.time(),
28882878
)
@@ -2946,7 +2936,6 @@ def test_autoscale_with_partial_deployment_details(
29462936
for i in [1, 2]:
29472937
replica_report = ReplicaMetricReport(
29482938
replica_id=ReplicaID(unique_id=f"d1_replica_{i}", deployment_id=d1_id),
2949-
aggregated_metrics={RUNNING_REQUESTS_KEY: 3},
29502939
metrics={RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 3)]},
29512940
timestamp=time.time(),
29522941
)
@@ -3027,7 +3016,6 @@ def test_autoscale_single_deployment_in_app(self, mocked_application_state_manag
30273016
for i in [1, 2]:
30283017
replica_report = ReplicaMetricReport(
30293018
replica_id=ReplicaID(unique_id=f"replica_{i}", deployment_id=d1_id),
3030-
aggregated_metrics={RUNNING_REQUESTS_KEY: 4},
30313019
metrics={RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 4)]},
30323020
timestamp=time.time(),
30333021
)
@@ -3156,7 +3144,6 @@ def test_autoscale_many_deployments_in_app(self, mocked_application_state_manage
31563144
for replica in replicas:
31573145
replica_report = ReplicaMetricReport(
31583146
replica_id=replica,
3159-
aggregated_metrics={RUNNING_REQUESTS_KEY: load},
31603147
metrics={
31613148
RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, load)]
31623149
},
@@ -3231,7 +3218,6 @@ def test_autoscale_with_min_equals_max_replicas(
32313218
for i in range(3):
32323219
replica_report = ReplicaMetricReport(
32333220
replica_id=ReplicaID(unique_id=f"replica_{i}", deployment_id=d1_id),
3234-
aggregated_metrics={RUNNING_REQUESTS_KEY: 10},
32353221
metrics={
32363222
RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, 10)]
32373223
},
@@ -3368,13 +3354,6 @@ def _record_handle_metrics(
33683354
actor_id="actor_id",
33693355
handle_source=DeploymentHandleSource.UNKNOWN,
33703356
queued_requests=[TimeStampedValue(timestamp_offset, 0)],
3371-
aggregated_queued_requests=0,
3372-
aggregated_metrics={
3373-
RUNNING_REQUESTS_KEY: {
3374-
d1_r1.to_full_id_str(): d1_load,
3375-
d1_r2.to_full_id_str(): d1_load,
3376-
}
3377-
},
33783357
metrics={
33793358
RUNNING_REQUESTS_KEY: {
33803359
d1_r1.to_full_id_str(): [
@@ -3398,13 +3377,6 @@ def _record_handle_metrics(
33983377
actor_id="actor_id",
33993378
handle_source=DeploymentHandleSource.UNKNOWN,
34003379
queued_requests=[TimeStampedValue(timestamp_offset, 0)],
3401-
aggregated_queued_requests=0,
3402-
aggregated_metrics={
3403-
RUNNING_REQUESTS_KEY: {
3404-
d2_r3.to_full_id_str(): d2_load,
3405-
d2_r4.to_full_id_str(): d2_load,
3406-
}
3407-
},
34083380
metrics={
34093381
RUNNING_REQUESTS_KEY: {
34103382
d2_r3.to_full_id_str(): [
@@ -3427,7 +3399,6 @@ def _record_replica_metrics(
34273399
for i in [1, 2]:
34283400
replica_report = ReplicaMetricReport(
34293401
replica_id=ReplicaID(unique_id=f"replica_{i}", deployment_id=d1_id),
3430-
aggregated_metrics={RUNNING_REQUESTS_KEY: d1_load},
34313402
metrics={
34323403
RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, d1_load)]
34333404
},
@@ -3439,7 +3410,6 @@ def _record_replica_metrics(
34393410
for i in [3, 4]:
34403411
replica_report = ReplicaMetricReport(
34413412
replica_id=ReplicaID(unique_id=f"replica_{i}", deployment_id=d2_id),
3442-
aggregated_metrics={RUNNING_REQUESTS_KEY: d2_load},
34433413
metrics={
34443414
RUNNING_REQUESTS_KEY: [TimeStampedValue(timestamp_offset, d2_load)]
34453415
},

0 commit comments

Comments
 (0)