[Core] Skip reporter and event aggregator client creation in minimal …#59846
Conversation
…installs Signed-off-by: sampan <sampan@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly handles minimal installations by skipping the creation of metrics-related clients when the metrics agent is not available. The changes are consistent across core_worker_process.cc, gcs_server.cc, and raylet/main.cc, checking for a valid metrics agent port before initializing clients. This prevents unnecessary connection attempts and error logs in minimal install environments. The addition of a test case to document the behavior of exhausted retries is also a good practice. Overall, the changes are solid, and I have one suggestion to improve code conciseness.
Signed-off-by: sampan <sampan@anyscale.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| (options.metrics_agent_port > 0) | ||
| ? std::make_unique<rpc::EventAggregatorClientImpl>(options.metrics_agent_port, | ||
| *client_call_manager_) | ||
| : std::make_unique<rpc::EventAggregatorClientImpl>(*client_call_manager_); |
There was a problem hiding this comment.
Null pointer dereference in EventAggregatorClient when aggregator enabled
When metrics_agent_port <= 0 (minimal install), the code creates an EventAggregatorClientImpl with deferred connection, which leaves grpc_client_ uninitialized (null). However, if a user enables the event aggregator feature via RAY_enable_core_worker_ray_event_to_aggregator=1, the SendRayEventsToAggregator function will call event_aggregator_client_->AddEvents(), which internally dereferences grpc_client_. This causes a null pointer dereference crash. The original code always initialized grpc_client_ via Connect(port), even with an invalid port. A guard is needed to either disable the aggregator feature or handle the null client when metrics_agent_port <= 0.
ray-gardener
bot
added
core
ray-project#59846) ## Description dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (`pip install ray`). this pr skips client creation (and adds a info log to guide users) when using minimal installs. ## Related issues Fixes ray-project#59665 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: sampan <sampan@anyscale.com> Co-authored-by: sampan <sampan@anyscale.com> Signed-off-by: jasonwrwang <jasonwrwang@tencent.com>
ray-project#59846) ## Description dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (`pip install ray`). this pr skips client creation (and adds a info log to guide users) when using minimal installs. ## Related issues Fixes ray-project#59665 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: sampan <sampan@anyscale.com> Co-authored-by: sampan <sampan@anyscale.com>
ray-project#59846) ## Description dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (`pip install ray`). this pr skips client creation (and adds a info log to guide users) when using minimal installs. ## Related issues Fixes ray-project#59665 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: sampan <sampan@anyscale.com> Co-authored-by: sampan <sampan@anyscale.com>
ray-project#59846) ## Description dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (`pip install ray`). this pr skips client creation (and adds a info log to guide users) when using minimal installs. ## Related issues Fixes ray-project#59665 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: sampan <sampan@anyscale.com> Co-authored-by: sampan <sampan@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
ray-project#59846) ## Description dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (`pip install ray`). this pr skips client creation (and adds a info log to guide users) when using minimal installs. ## Related issues Fixes ray-project#59665 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: sampan <sampan@anyscale.com> Co-authored-by: sampan <sampan@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
2 participants
Description
dashboard agent services such as reporter agent and event aggregator agent do not run in minimal ray installs (
pip install ray). this pr skips client creation (and adds a info log to guide users) when using minimal installs.Related issues
Fixes #59665
Additional information