[core][stats-die/03] kill STATS in core worker component #58060

can-anyscale · 2025-10-23T20:18:45Z

This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components.

Details
Full context of this refactoring work.

Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component.
In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface.
This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding.
There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point.
Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR.

Test:

CI

can-anyscale · 2025-10-23T20:19:34Z

src/ray/core_worker/core_worker_process.h


-  ray::stats::Gauge task_by_state_gauge_{GetTaskByStateGaugeMetric()};
-  ray::stats::Gauge actor_by_state_gauge_{GetActorByStateGaugeMetric()};
+  std::unique_ptr<ray::stats::Gauge> task_by_state_gauge_;


note: I changed this to pointer to the metric initialization can happen after stats::Init inside core_worker_process.cc

edoakes · 2025-10-24T19:11:08Z

@ZacAttack PTAL

ZacAttack · 2025-10-24T19:29:20Z

src/ray/common/metrics.h

+      "be placed. This is the time from when the tasks dependencies are "
+      "resolved to when it actually reserves resources on a node to run.",
+      /*unit=*/"s",
+      /*boundaries=*/{0.1, 1, 10, 100, 1000, 10000},


So I've lately gotten a bit of a crash course in this thing. These.... buckets seem a little off right? scheduler_placement_time_s is in seconds (I'm assuming because of the _s). So we're saying the scheduler placement time falls within 0.1s->1s->10s->100s->16 minutes -> 2.5+ hours

The latter buckets seem absurd right? What is the actual realistic spread of latency for the scheduler to place something? If the _s is misleading and actually it's not in seconds then we should fix that. But we should choose our buckets along the lines of

-->healthy range
-->elevated range
-->error range

make sense, the series of number must’ve been pulled out of a hat at some point ;) - i'll fix them on a PR on top of this, to keep this PR purely a refactoring

yeah look like the value is only meaningful in the range of less than 10s; most are actually below 0.1s so further breakdown of bucket less than 0.1s is probably more useful

I wonder if it even makes sense to have this metric in seconds at all. It seems like we'd want this to usually be sub second

make total sense

there you go #58217

Signed-off-by: Cuong Nguyen <[email protected]>

cursor · 2025-10-28T19:34:42Z

src/ray/core_worker/task_submission/tests/normal_task_submitter_test.cc

                       const TaskSpecification &same2,
                       const TaskSpecification &different) {
  rpc::Address address;
+  ray::observability::FakeHistogram fake_scheduler_placement_time_s_histogram_;


Bug: Local Variable Passed to Constructor Causes Use-After-Free

Variable fake_scheduler_placement_time_s_histogram_ is declared as a local variable inside the TestSchedulingKey helper function at line 1436, but is then passed by reference to NormalTaskSubmitter constructor at line 1465. This creates a use-after-free bug because the local variable will be destroyed before NormalTaskSubmitter uses it. This should be a local variable declaration that persists for the lifetime of the submitter, not a declaration at the function scope start.

it's fine, the NormalTaskSubmitter is also a local object within the TestSchedulingKey function

This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. - Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]>

…#58060) This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. - Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]>

This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. - Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]> Signed-off-by: elliot-barn <[email protected]>

…#58060) This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. - Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]>

…#58060) This PR replace STATS with Metric as a way to define metric inside ray (as a unification effort) in all core worker components. For the most parts, metrics are defined as the top level component (core_worker_process.cc) and pass down as an interface to the sub-components. **Details** Full context of this refactoring work. - Each component (e.g., gcs, raylet, core_worker, etc.) now has a metrics.h file located in its top-level directory. This file defines all metrics for that component. - In most cases, metrics are defined once in the main entry point of each component (gcs/gcs_server_main.cc for GCS, raylet/main.cc for Raylet, core_worker/core_worker_process.cc for the Core Worker, etc.). These metrics are then passed down to subcomponents via the ray::observability::MetricInterface. - This approach significantly reduces rebuild time when metric infrastructure changes. Previously, a change would trigger a full Ray rebuild; now, only the top-level entry points of each component need rebuilding. - There are a few exceptions where metrics are tracked inside object libraries (e.g., task_specification). In these cases, metrics are defined within the library itself, since there is no corresponding top-level entry point. - Finally, the obsolete metric_defs.h and metric_defs.cc files can now be completely removed. This paves the way for further dead code cleanup in a future PR. Test: - CI Signed-off-by: Cuong Nguyen <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>

can-anyscale commented Oct 23, 2025

View reviewed changes

can-anyscale added the go add ONLY when ready to merge, run all tests label Oct 23, 2025

can-anyscale force-pushed the can-statdie03 branch from ba9e8e8 to a06bd02 Compare October 23, 2025 23:31

can-anyscale marked this pull request as ready for review October 23, 2025 23:49

can-anyscale requested a review from a team as a code owner October 23, 2025 23:49

This comment was marked as outdated.

Sign in to view

can-anyscale force-pushed the can-statdie03 branch from a06bd02 to b075c33 Compare October 24, 2025 01:04

ray-gardener bot added core Issues that should be addressed in Ray Core observability Issues related to the Ray Dashboard, Logging, Metrics, Tracing, and/or Profiling labels Oct 24, 2025

edoakes assigned ZacAttack Oct 24, 2025

ZacAttack reviewed Oct 24, 2025

View reviewed changes

can-anyscale requested a review from a team October 28, 2025 14:23

[core][stats-die/03] kill STATS in core worker component

b950cff

Signed-off-by: Cuong Nguyen <[email protected]>

can-anyscale force-pushed the can-statdie03 branch from b075c33 to b950cff Compare October 28, 2025 19:33

cursor bot reviewed Oct 28, 2025

View reviewed changes

ZacAttack approved these changes Oct 29, 2025

View reviewed changes

can-anyscale merged commit dc966a1 into master Oct 29, 2025
6 checks passed

can-anyscale deleted the can-statdie03 branch October 29, 2025 21:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[core][stats-die/03] kill STATS in core worker component #58060

[core][stats-die/03] kill STATS in core worker component #58060

Uh oh!

can-anyscale commented Oct 23, 2025

Uh oh!

can-anyscale Oct 23, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

edoakes commented Oct 24, 2025

Uh oh!

ZacAttack Oct 24, 2025

Uh oh!

can-anyscale Oct 24, 2025

Uh oh!

can-anyscale Oct 24, 2025

Uh oh!

ZacAttack Oct 24, 2025

Uh oh!

can-anyscale Oct 27, 2025

Uh oh!

can-anyscale Oct 27, 2025

Uh oh!

cursor bot Oct 28, 2025

Uh oh!

can-anyscale Oct 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[core][stats-die/03] kill STATS in core worker component #58060

[core][stats-die/03] kill STATS in core worker component #58060

Uh oh!

Conversation

can-anyscale commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

edoakes commented Oct 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot Oct 28, 2025

Choose a reason for hiding this comment

Bug: Local Variable Passed to Constructor Causes Use-After-Free

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants