Skip to content

Commit 574ca25

Browse files
Remove the replica columnar path
should_encode_columnar returns False for every ReplicaMetricReport by design -- columnar array merge over many small per-replica sources is slower than the object path in the controllers decision loop -- so no producer ever emits a columnar replica frame and the entire receiving side was unreachable. Drop the replica encoder and decoders, the controllers replica columnar branch, the per-replica array stores and their dedup-at-write, and the two test files that covered only that path. Handle reports are now the only columnar producers, so encode() takes a HandleMetricReport. This also removes the columnar arms of _get_aggregated_custom_metrics and _get_raw_custom_metrics, which diverged from the object path on a present-but-empty series: the object branch reported 0.0 while the columnar branch omitted the replica entirely, so a custom policy indexing by replica id saw a KeyError on one path only. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: John Taylor <john.taylor@anyscale.com>
1 parent c595856 commit 574ca25

9 files changed

Lines changed: 7 additions & 709 deletions

python/ray/serve/_private/autoscaling_metrics_codec.py

Lines changed: 4 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import logging
2323
import struct
2424
import zlib
25-
from typing import Any, Dict, Union
25+
from typing import Any, Dict
2626

2727
try:
2828
import numpy as np
@@ -33,8 +33,6 @@
3333
RUNNING_REQUESTS_KEY,
3434
DeploymentID,
3535
HandleMetricReport,
36-
ReplicaID,
37-
ReplicaMetricReport,
3836
)
3937
from ray.serve._private.constants import (
4038
RAY_SERVE_COLUMNAR_METRICS_MIN_REPLICAS,
@@ -103,15 +101,11 @@ def _flatten_series(series_list):
103101
# ---------------------------------------------------------------------------
104102
# encode
105103
# ---------------------------------------------------------------------------
106-
def encode(report: Union[HandleMetricReport, ReplicaMetricReport]) -> bytes:
107-
if isinstance(report, HandleMetricReport):
108-
return _encode_handle(report)
109-
return _encode_replica(report)
104+
def encode(report: HandleMetricReport) -> bytes:
105+
return _encode_handle(report)
110106

111107

112-
def should_encode_columnar(
113-
report: Union[HandleMetricReport, ReplicaMetricReport]
114-
) -> bool:
108+
def should_encode_columnar(report: HandleMetricReport) -> bool:
115109
"""Whether a producer should serialize this report columnar (vs Python objects).
116110
117111
Format is chosen by report TYPE and self-identifies on the wire (see is_columnar),
@@ -230,41 +224,6 @@ def _encode_handle(rep: HandleMetricReport) -> bytes:
230224
return _frame(header, blob)
231225

232226

233-
def _encode_replica(rep: ReplicaMetricReport) -> bytes:
234-
metric_names = list(rep.metrics.keys())
235-
mi = {m: i for i, m in enumerate(metric_names)}
236-
entries, series_list = [], []
237-
for m in metric_names:
238-
series = rep.metrics[m]
239-
entries.append((mi[m], 0, len(series)))
240-
series_list.append(series)
241-
ts, val, _ = _flatten_series(series_list)
242-
off = 0
243-
for i, (a, _o, n) in enumerate(entries):
244-
entries[i] = (a, off, n)
245-
off += n
246-
arrays = {
247-
"entries": (
248-
np.array(entries, dtype="<i8") if entries else np.zeros((0, 3), "<i8")
249-
).reshape(-1, 3),
250-
"ts": np.array(ts, dtype="<f8"),
251-
"val": np.array(val, dtype="<f8"),
252-
}
253-
descriptors, blob = _pack(arrays)
254-
header = {
255-
"type": "replica",
256-
"replica_unique_id": rep.replica_id.unique_id,
257-
"deployment": [
258-
rep.replica_id.deployment_id.name,
259-
rep.replica_id.deployment_id.app_name,
260-
],
261-
"timestamp": rep.timestamp,
262-
"metric_names": metric_names,
263-
"arrays": descriptors,
264-
}
265-
return _frame(header, blob)
266-
267-
268227
def _frame(header: dict, blob: bytes) -> bytes:
269228
hb = json.dumps(header).encode()
270229
# level=1: metric reports are serialized on the producer hot path (per
@@ -319,46 +278,6 @@ def decode(buf: bytes) -> Dict[str, Any]:
319278
# ---------------------------------------------------------------------------
320279
# round-trip self-test (TimeStampedValue.value is compare=False, so compare deeply)
321280
# ---------------------------------------------------------------------------
322-
def _series_eq(a, b):
323-
return len(a) == len(b) and all(
324-
x.timestamp == y.timestamp and x.value == y.value for x, y in zip(a, b)
325-
)
326-
327-
328-
def decode_replica_running_requests(payload, metric_name=RUNNING_REQUESTS_KEY):
329-
"""For a REPLICA columnar payload, return (replica_id, ts_arr, val_arr, timestamp)
330-
for the given metric -- zero-copy arrays, no per-point objects."""
331-
view = decode(payload)
332-
h = view["header"]
333-
dep = DeploymentID(h["deployment"][0], h["deployment"][1])
334-
replica_id = ReplicaID(h["replica_unique_id"], dep)
335-
ts_arr, val_arr = view["ts"][:0], view["val"][:0]
336-
names = h["metric_names"]
337-
if metric_name in names:
338-
mi = names.index(metric_name)
339-
for row in view["entries"]:
340-
if int(row[0]) == mi:
341-
off, n = int(row[1]), int(row[2])
342-
ts_arr, val_arr = view["ts"][off : off + n], view["val"][off : off + n]
343-
break
344-
return replica_id, ts_arr, val_arr, h["timestamp"]
345-
346-
347-
def decode_replica_all_metrics(payload):
348-
"""For a REPLICA columnar payload, return (replica_id, {metric_name: (ts, val)},
349-
timestamp) for ALL metrics -- zero-copy arrays, no per-point objects. Carries
350-
custom autoscaling metrics through the columnar path, not just running_requests."""
351-
view = decode(payload)
352-
h = view["header"]
353-
dep = DeploymentID(h["deployment"][0], h["deployment"][1])
354-
replica_id = ReplicaID(h["replica_unique_id"], dep)
355-
names = h["metric_names"]
356-
ts_all, val_all = view["ts"], view["val"]
357-
metric_arrays = {}
358-
for row in view["entries"]:
359-
mi, off, n = int(row[0]), int(row[1]), int(row[2])
360-
metric_arrays[names[mi]] = (ts_all[off : off + n], val_all[off : off + n])
361-
return replica_id, metric_arrays, h["timestamp"]
362281

363282

364283
def decode_handle_flat(payload):

python/ray/serve/_private/autoscaling_state.py

Lines changed: 2 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
ReplicaMetricReport,
2626
TargetCapacityDirection,
2727
TimeSeries,
28-
TimeStampedValue,
2928
)
3029
from ray.serve._private.constants import (
3130
RAY_SERVE_MIN_HANDLE_METRICS_TIMEOUT_S,
@@ -88,10 +87,8 @@ def __init__(self, deployment_id: DeploymentID):
8887
self._replica_metrics: Dict[ReplicaID, ReplicaMetricReport] = dict()
8988
# Columnar per-replica running-requests arrays (wire-detected; producers
9089
# choose the format via should_encode_columnar).
91-
self._replica_running_arrays: Dict[ReplicaID, tuple] = dict()
9290
# Non-running columnar metrics per replica (custom autoscaling metrics):
9391
# replica_id -> {metric_name: (ts_arr, val_arr)}.
94-
self._replica_custom_arrays: Dict[ReplicaID, Dict[str, tuple]] = dict()
9592
# Unified per-replica "last accepted report timestamp" across BOTH wire formats.
9693
# Gates the object AND columnar ingest paths so a delayed report in either format
9794
# can't overwrite fresher data the other wrote. Cleared only on replica stop --
@@ -214,8 +211,6 @@ def register(self, info: DeploymentInfo, curr_target_num_replicas: int) -> int:
214211
def on_replica_stopped(self, replica_id: ReplicaID):
215212
if replica_id in self._replica_metrics:
216213
del self._replica_metrics[replica_id]
217-
self._replica_running_arrays.pop(replica_id, None)
218-
self._replica_custom_arrays.pop(replica_id, None)
219214
self._replica_report_ts.pop(replica_id, None)
220215

221216
def get_num_replicas_lower_bound(self) -> int:
@@ -293,68 +288,18 @@ def record_request_metrics_for_replica(
293288
self._replica_report_ts[replica_id] = send_timestamp
294289
# dedup-at-write: this source now reports via cloudpickle; drop any
295290
# columnar entries so the stores never double-count it.
296-
self._replica_running_arrays.pop(replica_id, None)
297-
self._replica_custom_arrays.pop(replica_id, None)
298-
299-
def record_columnar_metrics_for_replica(
300-
self, replica_id, metric_arrays, timestamp
301-
) -> None:
302-
"""Store columnar per-metric arrays for a replica (no per-point objects).
303-
running_requests feeds the hot-path store; any other metrics feed the custom
304-
store used by custom autoscaling policies (the columnar decode is lossless)."""
305-
prev_ts = self._replica_report_ts.get(replica_id)
306-
if prev_ts is not None and timestamp <= prev_ts:
307-
return
308-
self._replica_report_ts[replica_id] = timestamp
309-
running = metric_arrays.get(RUNNING_REQUESTS_KEY)
310-
if running is not None:
311-
self._replica_running_arrays[replica_id] = (
312-
running[0],
313-
running[1],
314-
timestamp,
315-
)
316-
else:
317-
# A newer report that omits running_requests must drop the stale running
318-
# timeseries -- the object path replaces the whole report, so missing
319-
# running stops contributing there too.
320-
self._replica_running_arrays.pop(replica_id, None)
321-
custom = {m: a for m, a in metric_arrays.items() if m != RUNNING_REQUESTS_KEY}
322-
if custom:
323-
self._replica_custom_arrays[replica_id] = custom
324-
else:
325-
self._replica_custom_arrays.pop(replica_id, None)
326-
# dedup-at-write: drop any cloudpickle entry for this source.
327-
self._replica_metrics.pop(replica_id, None)
328291

329292
def _columnar_aggregate_total_requests(self) -> float:
330293
"""Aggregate-mode total over pure-columnar stores: replica (direct-ingress)
331294
running arrays when a RUNNING replica reported, else handle running arrays,
332295
plus queued -- one fused numpy merge (no per-replica Python objects)."""
333-
# Gate on whether a RUNNING replica actually reported, NOT on the store being
334-
# non-empty: a lingering stopped-replica array (before on_replica_stopped
335-
# clears it) must fall through to handle-running exactly like the object
336-
# path, else handle-collected running is dropped (total reads queued-only).
337-
replica_segments = self._replica_columnar_segments()
338-
if replica_segments:
339-
return self._aggregate_segments(
340-
replica_segments + self._queued_columnar_segments()
341-
)
342296
if not self._handle_arrays:
343297
return 0.0
344298
return self._aggregate_segments(
345299
self._handle_running_columnar_segments(self._cached_running_replica_strs)
346300
+ self._queued_columnar_segments()
347301
)
348302

349-
def _replica_columnar_segments(self):
350-
"""RUNNING replicas' columnar running-request arrays as (ts, val) segments."""
351-
segs = []
352-
for replica_id in self._running_replicas:
353-
a = self._replica_running_arrays.get(replica_id)
354-
if a is not None and a[0].size:
355-
segs.append((a[0], a[1]))
356-
return segs
357-
358303
def _queued_columnar_segments(self):
359304
"""Columnar per-handle queued arrays as (ts, val) segments."""
360305
return [
@@ -811,7 +756,7 @@ def _calculate_total_requests_aggregate_mode(self) -> float:
811756
Total number of requests (average running + queued) calculated from
812757
timeseries data aggregation.
813758
"""
814-
has_columnar = bool(self._replica_running_arrays or self._handle_arrays)
759+
has_columnar = bool(self._handle_arrays)
815760
has_object = bool(self._replica_metrics or self._handle_requests)
816761
# Homogeneous fleets keep their native fast path. Columnar arrays are used
817762
# whenever present -- the controller wire-detects the format from the frame
@@ -862,8 +807,7 @@ def _mixed_aggregate_total_requests(self) -> float:
862807
are converted to small arrays. Empty object series are dropped so they
863808
cannot flip metrics_collected_on_replicas and suppress handle-side running
864809
(mirrors the columnar empty-skip). Disjoint by dedup-at-write."""
865-
segments = self._replica_columnar_segments()
866-
segments += self._series_segments(self._collect_replica_running_requests())
810+
segments = self._series_segments(self._collect_replica_running_requests())
867811
metrics_collected_on_replicas = bool(segments)
868812
if not metrics_collected_on_replicas:
869813
segments += self._handle_running_columnar_segments(
@@ -931,8 +875,6 @@ def _get_aggregated_custom_metrics(self) -> Dict[str, Dict[ReplicaID, float]]:
931875
Dict mapping metric name to dict of replica ID to aggregated metric value.
932876
"""
933877
aggregated_metrics: Dict[str, Dict[ReplicaID, float]] = defaultdict(dict)
934-
now = time.time()
935-
agg = self._config.aggregation_function
936878
for replica_id in self._running_replicas:
937879
# A replica is in the object store OR the columnar stores (dedup-at-write).
938880
replica_metric_report = self._replica_metrics.get(replica_id)
@@ -942,18 +884,6 @@ def _get_aggregated_custom_metrics(self) -> Dict[str, Dict[ReplicaID, float]]:
942884
replica_id
943885
] = self._merge_and_aggregate_timeseries([timeseries])
944886
continue
945-
running = self._replica_running_arrays.get(replica_id)
946-
if running is not None and running[0].size:
947-
aggregated_metrics[RUNNING_REQUESTS_KEY][
948-
replica_id
949-
] = self._aggregate_single_array(running[0], running[1], now, agg)
950-
custom = self._replica_custom_arrays.get(replica_id)
951-
if custom:
952-
for metric_name, (ts, val) in custom.items():
953-
if ts.size:
954-
aggregated_metrics[metric_name][
955-
replica_id
956-
] = self._aggregate_single_array(ts, val, now, agg)
957887
return dict(aggregated_metrics)
958888

959889
def _get_raw_custom_metrics(
@@ -971,19 +901,6 @@ def _get_raw_custom_metrics(
971901
for metric_name, timeseries in replica_metric_report.metrics.items():
972902
raw_metrics[metric_name][replica_id] = timeseries
973903
continue
974-
running = self._replica_running_arrays.get(replica_id)
975-
if running is not None and running[0].size:
976-
raw_metrics[RUNNING_REQUESTS_KEY][replica_id] = [
977-
TimeStampedValue(float(running[0][k]), float(running[1][k]))
978-
for k in range(running[0].size)
979-
]
980-
custom = self._replica_custom_arrays.get(replica_id)
981-
if custom:
982-
for metric_name, (ts, val) in custom.items():
983-
raw_metrics[metric_name][replica_id] = [
984-
TimeStampedValue(float(ts[k]), float(val[k]))
985-
for k in range(ts.size)
986-
]
987904
return dict(raw_metrics)
988905

989906

@@ -1236,15 +1153,6 @@ def record_request_metrics_for_replica(
12361153
dep_id
12371154
].record_request_metrics_for_replica(replica_metric_report)
12381155

1239-
def record_columnar_metrics_for_replica(
1240-
self, replica_id, metric_arrays, timestamp
1241-
) -> None:
1242-
dep_id = replica_id.deployment_id
1243-
if dep_id in self._deployment_autoscaling_states:
1244-
self._deployment_autoscaling_states[
1245-
dep_id
1246-
].record_columnar_metrics_for_replica(replica_id, metric_arrays, timestamp)
1247-
12481156
def record_request_metrics_for_handle(
12491157
self, handle_metric_report: HandleMetricReport
12501158
):
@@ -1441,15 +1349,6 @@ def record_request_metrics_for_replica(
14411349
if app_state:
14421350
app_state.record_request_metrics_for_replica(replica_metric_report)
14431351

1444-
def record_columnar_metrics_for_replica(
1445-
self, replica_id, metric_arrays, timestamp
1446-
) -> None:
1447-
app_state = self._app_autoscaling_states.get(replica_id.deployment_id.app_name)
1448-
if app_state:
1449-
app_state.record_columnar_metrics_for_replica(
1450-
replica_id, metric_arrays, timestamp
1451-
)
1452-
14531352
def record_request_metrics_for_handle(
14541353
self,
14551354
handle_metric_report: HandleMetricReport,

python/ray/serve/_private/controller.py

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -411,35 +411,6 @@ def record_autoscaling_metrics_from_replica(
411411
):
412412
_ingest_start = time.time()
413413
if isinstance(replica_metric_report, bytes):
414-
if autoscaling_metrics_codec.is_columnar(replica_metric_report):
415-
if not autoscaling_metrics_codec.can_decode_columnar():
416-
# Wire-detect works without numpy but decoding does not.
417-
autoscaling_metrics_codec.warn_columnar_undecodable_once()
418-
return
419-
_decode_start = time.time()
420-
(
421-
replica_id,
422-
metric_arrays,
423-
report_ts,
424-
) = autoscaling_metrics_codec.decode_replica_all_metrics(
425-
replica_metric_report
426-
)
427-
self._health_metrics_tracker.record_decompress(
428-
(time.time() - _decode_start) * 1000
429-
)
430-
self._record_metrics_delay(
431-
report_ts,
432-
replica_id.deployment_id,
433-
self.replica_metrics_delay_histogram,
434-
self._health_metrics_tracker.record_replica_metrics_delay,
435-
)
436-
self.autoscaling_state_manager.record_columnar_metrics_for_replica(
437-
replica_id, metric_arrays, report_ts
438-
)
439-
self._health_metrics_tracker.record_replica_ingest(
440-
(time.time() - _ingest_start) * 1000
441-
)
442-
return
443414
_decompress_start = time.time()
444415
replica_metric_report = decompress_metric_report(replica_metric_report)
445416
self._health_metrics_tracker.record_decompress(

python/ray/serve/_private/replica.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
from ray.dag.py_obj_scanner import _PyObjScanner
5454
from ray.remote_function import RemoteFunction
5555
from ray.serve import metrics
56-
from ray.serve._private import autoscaling_metrics_codec
5756
from ray.serve._private.common import (
5857
RUNNING_REQUESTS_KEY,
5958
DeploymentID,
@@ -978,11 +977,7 @@ def _push_autoscaling_metrics(self) -> None:
978977
self._pending_metrics_push_ref = (
979978
# Actor methods are resolved dynamically on the actor handle.
980979
self._controller_handle.record_autoscaling_metrics_from_replica.remote( # type: ignore[attr-defined]
981-
autoscaling_metrics_codec.encode(replica_metric_report)
982-
if autoscaling_metrics_codec.should_encode_columnar(
983-
replica_metric_report
984-
)
985-
else compress_metric_report(replica_metric_report)
980+
compress_metric_report(replica_metric_report)
986981
)
987982
)
988983

0 commit comments

Comments
 (0)