[core][actor scalability] Move the pending resource load pull off the GCS main io_context - #65024
Conversation
…-project#65024 Signed-off-by: yicheng <yicheng@anyscale.com>
fad185a to
daae6a3
Compare
…-project#65024 Signed-off-by: yicheng <yicheng@anyscale.com>
1febf9e to
4021c50
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces GcsResourceLoadPuller to pull resource loads from raylets on a dedicated io_context thread and post the results back to the main thread, offloading this work from the main thread. The review feedback highlights a critical lifetime issue in GcsResourceLoadPuller::Pull where capturing 'this' in the RPC callback could lead to a use-after-free error if the puller is destroyed before the callback executes. It is recommended to capture only the necessary outliving references instead.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 4021c50. Configure here.
Signed-off-by: yicheng <yicheng@anyscale.com>
| gcs_resource_manager_->UpdateResourceLoads(resources); | ||
| gcs_autoscaler_state_manager_->UpdateResourceLoadAndUsage(std::move(resources)); |
There was a problem hiding this comment.
Is there a guarantee that these are initialized when this callback is invoked?
There was a problem hiding this comment.
This is currently guaranteed because we post it to the main IO context, since is a FIFO queue, the callback is guaranteed to run after these are initialized.
That said, I think you're right that it should be explicit. Moved the puller start after these are initialized.
Signed-off-by: yicheng <yicheng@anyscale.com>
Signed-off-by: yicheng <yicheng@anyscale.com>

Description
I'm working on improving actor scalability for RL / post-training / pre-training workloads. At large scale, say 10k nodes, GCS is still under pressure even with no workloads running, because of the default work it does: periodically collecting pending requests from all raylets for autoscaler decisions, health checking every raylet, and so on.
These tasks scale with node count, and at the 10k level they are no longer negligible but become a killer. When I trying to optimize actor start path. realized that at 10k nodes this default work is a real problem and basically undermines whatever optimizations we make to the actor start path, because no matter what we do, these tasks already keep GCS busy enough to slow GCS down.
Measuring an idle 10k node cluster for 150s (per thread CPU sampled from
/procat 1 Hz), the GCS mainio_contextthread is ~55% busy, and roughly 40 points of it is this poll.This PR moves the sends and reply callbacks (~20 points) off the main thread. the poll's remaining ~19 points are the per reply applies + the cost of posting, which can be addressed in following up pr: add a small delay to do batch posting.
Before this PR
RayletLoadPulledtimer fires on the mainio_contextGetResourceLoadrequest to each live raylet, still on the mainio_contextio_context, and GCS updates the resource loads for autoscaler useAfter this PR
resource_load_pull_io_contextresource_load_pull_io_context.io_context. We prefer not to hold locks and instead modify all state on a single thread, since this data is read and modified frequently by multiple components. This can be further optimized in future. Like we could add a small delay to do batch posting. I would prefer making it a separate PR.e2e Perf
before:
after:

End to end wall clock: 82.7 → 70.8 s (−14.4%) at 10k. 2k within noise (12.9 → 13.9 s; per-actor e2e −3.3%)
Note that wall time at this scale ranged 70.8 to 74.4 s against the 82.7 s baseline, so even the worst repeat win 10%
Note
Even if we had centralized scheduling, this is still useful and still needed, because there are resource requests pending on the worker, for example some tasks are never even submitted due to back pressure across different task classes. So we still need the
worker -> raylet -> GCSpath to gather all pending requests for the autoscaler.