[core] (3/n) [Removing GCS Centralized Scheduling] Removing GlobalGC from the GCS #60019
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ability to send global GC requests to all raylets in the cluster. Moving the GC throttler to the Raylet's package now that it's only used by Raylets. Signed-off-by: irabbani <[email protected]>
israbbani
added
core
israbbani
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively removes the GlobalGC functionality from the GCS, which is a good cleanup of an unused feature. The changes are clear and align with the goal of removing Centralized Actor Scheduling.
As part of this, the Throttler utility has been moved from src/ray/util to src/ray/raylet since it's now only used by the raylet. I've made a few suggestions to improve this move:
- Moving
Throttlerto theray::rayletnamespace for better code organization. - Removing a redundant header include.
Overall, this is a solid contribution to simplifying the codebase.
| #include "ray/core_worker_rpc_client/core_worker_client_pool.h" | ||
| #include "ray/flatbuffers/node_manager_generated.h" | ||
| #include "ray/raylet/local_object_manager_interface.h" | ||
| #include "ray/raylet/throttler.h" |
Contributor
There was a problem hiding this comment.
This include is redundant because ray/raylet/throttler.h is already included in node_manager.h. Including headers in implementation files when they are already in the corresponding header can lead to longer compile times and is generally discouraged.
Contributor
Author
There was a problem hiding this comment.
Gemini, you need to talk to whoever wrote https://google.github.io/styleguide/cppguide.html#Include_What_You_Use at Google.
Sparks0219
reviewed
Jan 10, 2026
Signed-off-by: irabbani <[email protected]>
israbbani
merged commit e17b2d2
into
irabbani/remove-centralized-actor-scheduling-2
5 checks passed
edoakes
pushed a commit
that referenced
this pull request
Jan 14, 2026
…aseManager from GCS. (#60008) This PR stacks on #59979. This is 2/N and 3/N (#60019) in a series of PRs to remove Centralized Actor Scheduling by the GCS (introduced in #15943). The feature is off by default and no longer in use or supported. In this PR, 1. I remove use of the ClusterLeaseManager (from the Raylet's scheduler) in the GCS. 2. I remove GC requests published globally from the GCS to all Raylets through the RaySyncer. This feature was introduced as part of Centralized Scheduling through GCS in #28663. 3. I've moded the Throttler from util to the raylet package and made it's visibility private. The GC implementation used to check to see if there were pending leases for two consecutive intervals (of 10s) each from the ClusterLeaseManager in the GCS. If so, the GCS would trigger GlobalGC. Therefore, with GCS Centralized Scheduling disabled, the GCS would never send GlobalGC requests. It's safe to delete now. --------- Signed-off-by: irabbani <[email protected]>
edoakes
pushed a commit
that referenced
this pull request
Jan 15, 2026
…eManager from GCS (#60121) This PR stacks on #60019. This is 3/N in a series of PRs to remove Centralized Actor Scheduling by the GCS (introduced in #15943). The feature is off by default and no longer in use or supported. In this PR, I've removed the GCS's dependency on the LocalLeaseManager. I've also moved LocalLeaseManager to the raylet/scheduling package and made it's visibility private to the package. Also deleted the NoopLocalLeaseManager. The LocalLeaseManager is used by the ClusterLeaseManager to see if a task can scheduled locally by a Raylet. The GCS used only the Noop implementation. --------- Signed-off-by: irabbani <[email protected]> Signed-off-by: Ibrahim Rabbani <[email protected]>
jeffery4011
pushed a commit
to jeffery4011/ray
that referenced
this pull request
Jan 20, 2026
…eManager from GCS (ray-project#60121) This PR stacks on ray-project#60019. This is 3/N in a series of PRs to remove Centralized Actor Scheduling by the GCS (introduced in ray-project#15943). The feature is off by default and no longer in use or supported. In this PR, I've removed the GCS's dependency on the LocalLeaseManager. I've also moved LocalLeaseManager to the raylet/scheduling package and made it's visibility private to the package. Also deleted the NoopLocalLeaseManager. The LocalLeaseManager is used by the ClusterLeaseManager to see if a task can scheduled locally by a Raylet. The GCS used only the Noop implementation. --------- Signed-off-by: irabbani <[email protected]> Signed-off-by: Ibrahim Rabbani <[email protected]> Signed-off-by: jeffery4011 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR stacks on #60008.
This is 3/N in a series of PRs to remove Centralized Actor Scheduling by the GCS (introduced in #15943). The feature is off by default and no longer in use or supported.
In this PR, I remove GC requests published globally from the GCS to all Raylets through the RaySyncer. This feature was introduced as part of Centralized Scheduling through GCS in #28663.
The implementation used to check to see if there were pending leases for two consecutive intervals (of 10s) each from the ClusterLeaseManager in the GCS. If so, the GCS would trigger GlobalGC. Therefore, with GCS Centralized Scheduling disabled, the GCS would never send GlobalGC requests. It's safe to delete now.
I've also moved the Throttler from util to the raylet package and made it's visibility private.