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

Fix edge case in autoscaler with poor bin packing #5702

Merged
merged 2 commits into from
Sep 13, 2019

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Sep 13, 2019

Why are these changes needed?

When the cluster is backlogged, bump the "estimated busy nodes" count by the number of nonidle nodes + 1. This takes into account the case where the head node cannot accept tasks and hence is idle, or more generally, when nodes don't register as fully utilized due to poor bin packing, but there are tasks in a backlog somewhere in the cluster.

One side effect is that we are slightly more aggressive at scaling up, but this is probably ok.

Related issue number

Closes #5696

Checks

@ericl ericl changed the title Fix edge case in autoscaler with small head nodes Fix edge case in autoscaler with poor bin packing Sep 13, 2019
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/17008/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/17009/
Test FAILed.

Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

Fixing some of the unnecessary output too, nice :)

LGTM assuming you've validated manually.

if max_frac > 0:
num_nonidle += 1

# If any nodes have a queue buildup, assume all non-idle nodes are 100%
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems very aggressive, for example in a case where we somehow have nearly the whole cluster idle but a single node with a queue (could happen if there were many short-lived tasks scheduled on other nodes?). Maybe we should make it a bit more of a heuristic instead of this hard rule. Seems like a fine fix for now, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't trigger an up-scaling event unless there is at least "some" load on almost every node. So the case you mentioned is probably OK.

But yeah agreed we could be smarter, perhaps by looking at the queue size and extrapolating the load.

@ericl ericl merged commit 3ed18d0 into ray-project:master Sep 13, 2019
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.

Poor auto-scaling
3 participants