Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Datasets] [Autoscaling Actor Pool - 2/2] Add autoscaling support to MapOperator actor pool. #31987

Merged

Conversation

clarkzinzow
Copy link
Contributor

This PR adds support for autoscaling to the actor pool implementation of MapOperator (this PR is stacked on top of #31986).

The same autoscaling policy as the legacy ActorPoolStrategy is maintained, as well as providing more aggressive and sensible downscaling via:

  • If there are more idle actors than running/pending actors, scale down.
  • Once we're done submitting tasks, cancel pending actors and kill idle actors.

In addition to autoscaling, max_tasks_in_flight capping is also implemented.

Related issue number

Closes #31723

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • 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 made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@clarkzinzow clarkzinzow force-pushed the datasets/feat/autoscaling-actor-pool-map branch from e48c0aa to 9e61692 Compare January 27, 2023 20:49
@clarkzinzow clarkzinzow force-pushed the datasets/feat/autoscaling-actor-pool-map branch from 9e61692 to c03347e Compare January 28, 2023 00:32
Copy link
Contributor

@c21 c21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Glad we also fix the bug with downscaling support here!

ready_to_total_workers_ratio: float = 0.8
# Maximum ratio of idle workers to the total number of workers. If the pool goes
# above this ratio, the pool will be scaled down.
idle_to_total_workers_ratio: float = 0.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why we have different ratio between scale up and scale down?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're pretty much orthogonal! ready includes both active and idle workers (only pending is excluded), and idle only includes idle workers (both active and pending are excluded), so they're not inverses of each other. A bit more on each of these heuristics...

The 80% cap for the ready/total ratio ensures that, in the worst case, we don't try to create more actors than 125% of the cluster's CPU/GPU resource capacity; this is a basic heuristic for resource-based backpressure. This cap will probably be replaced with the executor's resource management + a heuristic based on the work queue size and misc. profiling (queue arrival rate, actor startup time, task execution time).

The 50% cap for idle/total is a super basic "more than half of the actors are idle" heuristic for when we've created too many actors; this was the simplest heuristic I could come up with that bootstraps, scales, and degrades pretty gracefully:

  1. At operator startup, pending actors aren't penalized, so we're able to start an initial wave of actors in parallel.
  2. The pool's steady-state should be > half of the actors are active with < half idle, so we should have solid load-balancing + an idle pool ready for input queue bursts. With load-balancing based on tasks-in-flight, this is probably a bit conservative, actually; we could look at decreasing this to 0.25 on some benchmarks to see if we really need that much pool slack.
  3. As a pool becomes temporarily less active, e.g. there's a delay bubble in upstream processing, we won't downscale.

I'm guessing that we might keep this heuristic around as a super conservative idle cap, since it's a simple alternative to the common solution of idle timeouts and can be complementary (a fallback) to downscaling triggered by the executor control loop based on its resource management.

Should be fun to experiment with tweaking these going forward!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @clarkzinzow for the detailed explanation!

@ericl ericl merged commit 22177cb into ray-project:master Jan 28, 2023
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
…`MapOperator` actor pool. (ray-project#31987)

This PR adds support for autoscaling to the actor pool implementation of `MapOperator` (this PR is stacked on top of ray-project#31986).

The same autoscaling policy as the legacy `ActorPoolStrategy` is maintained, as well as providing more aggressive and sensible downscaling via:
* If there are more idle actors than running/pending actors, scale down.
* Once we're done submitting tasks, cancel pending actors and kill idle actors.

In addition to autoscaling, `max_tasks_in_flight` capping is also implemented.

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data] [streaming] Add autoscaling for ActorPoolTaskSubmitter
4 participants