[Serve] Fix excessive CPU from per-handle cached metrics tasks at scale

[Krishnachaitanyakc]

Why are these changes needed?

On a Ray Serve cluster running ~1500 Serve apps, the ingress replica consumes excessive CPU. Profiling shows the dominant consumer is the metrics reporting pipeline in RouterMetricsManager._report_cached_metrics_forever, not actual request processing.

Root cause: Each RouterMetricsManager instance (one per deployment handle per Router) creates its own _report_cached_metrics_forever asyncio task. With ~1500 cached deployment handles on the ingress, this produces:

• 1500 concurrent asyncio tasks on a single event loop
• Each waking every 100ms (the default RAY_SERVE_METRICS_EXPORT_INTERVAL_MS)
• ~15,000 task wake-ups per second, each calling _report_cached_metrics() which iterates over cached counters, increments Prometheus metrics, and sums replica request dicts

Fix: Introduce SharedCachedMetricsReporter — a singleton per event loop that consolidates all per-handle reporting tasks into a single shared asyncio task. All RouterMetricsManager instances register with the shared reporter via a WeakSet, so metrics for every handle are flushed in one loop iteration. This reduces N asyncio tasks to 1, regardless of deployment count.

The design follows the same singleton-per-event-loop pattern already established by SharedRouterLongPollClient in the same file.

Changes

• SharedCachedMetricsReporter (new class in router.py):
  • Thread-safe get_or_create() singleton keyed by event loop
  • WeakSet-based registration (auto-cleanup when managers are GC'd)
  • Single _report_forever() loop with per-manager error isolation and exponential backoff
• RouterMetricsManager:
  • Constructor registers with the shared reporter instead of creating a per-instance asyncio task
  • Removed _report_cached_metrics_forever() method (logic moved to shared reporter)
  • shutdown() deregisters from the shared reporter instead of cancelling a task
• Tests (test_router.py):
  • test_multiple_managers_share_single_reporter — verifies N managers share 1 reporter and all get metrics flushed
  • test_deregister_on_shutdown — verifies shutdown deregisters the manager
  • test_shutdown_one_does_not_affect_others — verifies one manager's shutdown doesn't disrupt others

Related issue number

Fixes #62619

Checks

• I've signed off all my commits for the DCO.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've added any new APIs to the API Reference.
• I've made sure the tests are passing. Tests can be run with pytest python/ray/serve/tests/unit/test_router.py -xvs -k TestSharedCachedMetricsReporter and existing TestRouterMetricsManager tests continue to pass.

[gemini-code-assist]

Code Review

This pull request introduces a SharedCachedMetricsReporter to consolidate metrics reporting for RouterMetricsManager instances into a single asyncio task per event loop, significantly reducing CPU overhead at scale. The feedback highlights a resource leak where the background task and reporter instance are not cleaned up when all managers are deregistered. Additionally, it is recommended to verify interval consistency in the singleton factory and to explicitly handle asyncio.CancelledError for a cleaner shutdown process.

[Krishnachaitanyakc]

@abrarsheikh can you review this?

[abrarsheikh]

change makes sense. Can you produce some benchmark numbers for support the improvements.

[abrarsheikh]

curious why catch CancelledError?

[Krishnachaitanyakc]

When deregister() is called for the last manager, it calls self._task.cancel(). That raises CancelledError inside the task at the next await point (the asyncio.sleep). Without catching it, asyncio would log an unhandled-exception warning on the event loop — noisy tracebacks during clean shutdown.

Since CancelledError inherits from BaseException (not Exception) in Python 3.9+, the inner except Exception blocks don't catch it — so the outer except asyncio.CancelledError: pass is needed for a silent, clean exit. This is the standard asyncio cleanup pattern (same as what the old per-handle _report_cached_metrics_forever did in RouterMetricsManager.shutdown()).

[Krishnachaitanyakc]

deregister() calls task.cancel() which raises CancelledError at the next await. Without catching it, asyncio logs unhandled exception tracebacks which can be noisy. Since CancelledError inherits from BaseException, the inner blocks don't catch it. So, the outer handler is needed for a cleaner shutdown.

[Krishnachaitanyakc]

Benchmark Results

Ran a microbenchmark comparing old (N per-handle asyncio tasks) vs new (1 shared task with periodic yielding every 50 managers). Each simulated manager does the same work as _report_cached_metrics(): dict iteration, counter increment, gauge set. Interval = 100ms (default RAY_SERVE_METRICS_EXPORT_INTERVAL_MS), duration = 5s per scenario.

100 managers

Metric	OLD	NEW	Ratio
Asyncio tasks	100	1	100x
Task wake-ups/sec	1,000	10	100x
CPU time (s)	0.899	0.752	1.2x
Request throughput	4,368	4,333	1.0x

500 managers

Metric	OLD	NEW	Ratio
Asyncio tasks	500	1	500x
Task wake-ups/sec	5,000	10	500x
CPU time (s)	1.004	0.747	1.3x
Request throughput	4,387	4,321	1.0x

1500 managers (production scale)

Metric	OLD	NEW	Ratio
Asyncio tasks	1,500	1	1,500x
Task wake-ups/sec	15,000	10	1,500x
CPU time (s)	1.173	0.772	1.5x
Request throughput	4,337	4,145	1.0x

Notes

• The synthetic benchmark understates the real-world improvement because simulated sync_lightweight_work() is much lighter than actual Prometheus metric operations with tag lookups. The real system also has request processing, actor communication, and other tasks competing for the event loop.
• The 1,500x reduction in asyncio tasks and wake-ups is the primary win — it drastically reduces scheduler bookkeeping overhead that scales linearly with task count.
• Request throughput is unaffected (1.0x), confirming the change doesn't hurt request processing.
• Based on @abrarsheikh's feedback, added periodic yielding (_YIELD_BATCH_SIZE = 50) so the loop yields to the event loop every 50 managers, preventing any single iteration from blocking too long at scale.

Benchmark script: python/ray/serve/tests/unit/bench_shared_reporter.py

[abrarsheikh]

Benchmark Results

Ran a microbenchmark comparing old (N per-handle asyncio tasks) vs new (1 shared task with periodic yielding every 50 managers). Each simulated manager does the same work as _report_cached_metrics(): dict iteration, counter increment, gauge set. Interval = 100ms (default RAY_SERVE_METRICS_EXPORT_INTERVAL_MS), duration = 5s per scenario.

100 managers

Metric OLD NEW Ratio
Asyncio tasks 100 1 100x
Task wake-ups/sec 1,000 10 100x
CPU time (s) 0.899 0.752 1.2x
Request throughput 4,368 4,333 1.0x

500 managers

Metric OLD NEW Ratio
Asyncio tasks 500 1 500x
Task wake-ups/sec 5,000 10 500x
CPU time (s) 1.004 0.747 1.3x
Request throughput 4,387 4,321 1.0x

1500 managers (production scale)

Metric OLD NEW Ratio
Asyncio tasks 1,500 1 1,500x
Task wake-ups/sec 15,000 10 1,500x
CPU time (s) 1.173 0.772 1.5x
Request throughput 4,337 4,145 1.0x

Notes

• The synthetic benchmark understates the real-world improvement because simulated sync_lightweight_work() is much lighter than actual Prometheus metric operations with tag lookups. The real system also has request processing, actor communication, and other tasks competing for the event loop.
• The 1,500x reduction in asyncio tasks and wake-ups is the primary win — it drastically reduces scheduler bookkeeping overhead that scales linearly with task count.
• Request throughput is unaffected (1.0x), confirming the change doesn't hurt request processing.
• Based on @abrarsheikh's feedback, added periodic yielding (_YIELD_BATCH_SIZE = 50) so the loop yields to the event loop every 50 managers, preventing any single iteration from blocking too long at scale.

Benchmark script: python/ray/serve/tests/unit/bench_shared_reporter.py

Synthetic benchmark is not that important here. Do you have the compute to run like a real test?

[Krishnachaitanyakc]

@abrarsheikh adding them here. Numbers are flat. I don't have access to a cluster to reproduce the production conditions (GIL contention, real gRPC I/O, ~1500 live apps) where the original issue's profiling showed _report_cached_metrics_forever as the dominant CPU consumer. Do you have a staging environment where we could validate this?

Real benchmark (actual RouterMetricsManager + Prometheus metrics)

Used real RouterMetricsManager instances with ray.util.metrics.Counter/Gauge, 200 concurrent request handlers, interval=100ms, 5s per scenario.

Managers	Metric	OLD (N tasks)	NEW (1 shared)	Ratio
100	CPU time (s)	2.168	2.166	1.0x
100	Request throughput	483,496	483,273	1.00x
500	CPU time (s)	2.226	2.238	1.0x
500	Request throughput	484,502	482,412	1.00x
1500	CPU time (s)	2.688	2.734	1.0x
1500	Request throughput	484,262	477,766	0.99x

[abrarsheikh]

@jugalshah291 would you be able to try this PR out in your cluster and share some benchmark results?

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[Krishnachaitanyakc]

@abrarsheikh pinging as this PR has been marked stale, anything I can do here?

[jugalshah291]

Hey sorry, have been busy with getting ray 2.54.0 on our side to cust (so we can upgrade kuberay next) and handling quarter end priorities
I would spend some time today to see if I reproduce the issue in our dev cluster and update the same

[Krishnachaitanyakc]

@jugalshah291 @abrarsheikh just bumping this up

[kouroshHakha]

@jugalshah291 Any updates on the benchmark results?

[jugalshah291]

Hi I had done a run and the result where not expected, probably something interfered with the perf
I need to do a rerun
would do one tomorrow and give an update

[jugalshah291]

Update:
Ran into some internal env issue, working on fixing it so I can test this, hoping to get the env stable today