Skip to content

Commit e60d10b

Browse files
Document the aggregation semantics of the built-in autoscaling metrics
Review flagged that AutoscalingContext advertised total_queued_requests as "Number of requests currently queued" while _get_queued_requests now aggregates the merged handle timeseries with the deployment's aggregation_function, so min or max reports the window trough or peak instead of the current depth. Fix the docs rather than the aggregation. total_running_requests is total_num_requests - total_queued_requests, and the note above it already flags that non-additive functions make the difference approximate; that identity only holds while both sides share the function. Aggregating queued with mean while the total uses max would yield max_total - mean_queued, which approximates nothing and can go negative. So: state the shared window and function on the class, where it reaches the rendered API page; drop "currently" from the field comments; name the coupling at the subtraction so the two sides do not get aggregated differently by accident; and stop _get_queued_requests describing itself as a sum, which it never was. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: John Taylor <john.taylor@anyscale.com>
1 parent 00cd3f3 commit e60d10b

2 files changed

Lines changed: 11 additions & 5 deletions

File tree

python/ray/serve/_private/autoscaling_state.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,8 @@ def _get_queued_requests(self) -> float:
652652
"""Calculate the total number of queued requests across all handles.
653653
654654
Returns:
655-
Sum of queued requests at all handles, aggregated from handle timeseries.
655+
The merged instantaneous total of every handle's queued-requests
656+
timeseries, aggregated over the window by `aggregation_function`.
656657
"""
657658
return self._merge_and_aggregate_timeseries(
658659
self._collect_handle_queued_requests()

python/ray/serve/config.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ class AutoscalingContext:
6161
Note: The aggregated_metrics and raw_metrics fields support lazy evaluation.
6262
You can pass callables that will be evaluated only when accessed, with results
6363
cached for subsequent accesses.
64+
65+
Note: total_num_requests and total_queued_requests are both aggregated over
66+
`look_back_period_s` using the deployment's `aggregation_function`, so under `min`
67+
or `max` they report the window trough or peak rather than the current value.
6468
"""
6569

6670
def __init__(
@@ -112,10 +116,10 @@ def __init__(
112116

113117
# Built-in metrics
114118
self._total_num_requests_value = (
115-
total_num_requests #: Total number of requests across all replicas.
119+
total_num_requests #: Ongoing (running + queued) requests.
116120
)
117121
self._total_queued_requests_value = (
118-
total_queued_requests #: Number of requests currently queued.
122+
total_queued_requests #: Requests queued at handles.
119123
)
120124

121125
# Custom metrics - store potentially lazy callables privately
@@ -172,8 +176,9 @@ def total_queued_requests(self) -> float:
172176

173177
@property
174178
def total_running_requests(self) -> float:
175-
# NOTE: for non-additive aggregation functions, total_running_requests is not
176-
# accurate, consider this is an approximation.
179+
# Exact only because both sides share the deployment's `aggregation_function`;
180+
# for non-additive ones (min/max) this difference is an approximation. Keep them
181+
# on the same function -- aggregating either side differently breaks it outright.
177182
return self.total_num_requests - self.total_queued_requests
178183

179184
@property

0 commit comments

Comments
 (0)