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

Support for nprocs? #349

Closed
hhuuggoo opened this issue Jul 21, 2021 · 8 comments
Closed

Support for nprocs? #349

hhuuggoo opened this issue Jul 21, 2021 · 8 comments

Comments

@hhuuggoo
Copy link

Is there was any interest in building in support for nprocs? I know in #84 the consensus was that having a 1-1 relationship between processes and pods makes the most sense.

We use nprocs because

  • Some workloads work better with processes vs threads.
  • We prefer to think in terms of machines rather than pods.

I've considered thinking in pods rather than machines but for the clusters we manage, machines are the fundamental unit people pay for, and it's easy to end up in a situation where machines are under-utilized at the k8s level. Yes k8s can move pods around, but that ends up potentially disrupting longer running workloads.

For the most part using dask-kubernetes with nprocs>1 has worked pretty well. It can get a little goofy because if nprocs=4 and I call scale(4) I end up with 16 workers. I think the most value would be accomplished in making adaptive understand nprocs

So the question is just if anyone else cares about this? If it's just me, I'll subclass Adaptive and call it a day. Otherwise I can add this functionality into dask-kubernetes .

@woodcockr
Copy link

I think this would be good. There's a difference between processes and Pods in terms of worker locality for data comms.
I also prefer to think of machines rather than pods.

@jacobtomlinson
Copy link
Member

I have no specific objection to having multiple processes within a pod. Ideally I would like things to be configurable with sensible defaults. One process per pod is a sensible default, but if you have a workload that would benefit from some additional tuning then sure go for it.

The issue with Adaptive not being aware of processes is not dask-kubernetes specific, that would need to be addressed upstream in distributed.

@hhuuggoo do you have someone at Saturn who could contribute here and work on this?

@hhuuggoo
Copy link
Author

@hhuuggoo do you have someone at Saturn who could contribute here and work on this?

Yes! It will probably be me - though I might be a bit slow on this. @jacobtomlinson Do you think I should move this issue to distributed? Are there other cluster types that typically have the same issue? dask-kubernetes was the only one I've been thinking about lately.

@jacobtomlinson
Copy link
Member

I think there are two things here:

  1. dask-kubernetes does not expose nprocs as a configuration option.
  2. Adaptive is not aware of multiple processes.

I think 1 should remain here in this issue, but it might be worth raising another issue on distributed for 2.

@hhuuggoo
Copy link
Author

@jacobtomlinson I'm not sure #2 needs to be addressed.

The scheduler already understands hosts:

https://github.com/dask/distributed/blob/1be9265ac11876df766bb8bd6d6eb519d04d3bac/distributed/scheduler.py#L6398

and Adaptive supports configuring that parameter

https://github.com/dask/distributed/blob/1be9265ac11876df766bb8bd6d6eb519d04d3bac/distributed/deploy/adaptive.py#L93

I think we would only need to modify dask-kubernetes to configure Adaptive with the proper key?

@hhuuggoo
Copy link
Author

Actually I think there is one other thing that I can raise with distributed. I'm not sure how important this is yet, but probably if we want to always be using host as the key in calling workers_to_close on the Scheduler, we would need to make sure the scheduler also passes those parameters. adaptive_target (which SpecCluster uses to figure out how many workers to scale down), calls workers_to_close with no arguments

@hhuuggoo
Copy link
Author

Just a note that I just started digging around, and I'm not sure this is an issue (was looking at 2021.07 earlier last week). I believe the recommendations I'm getting back for the scheduler are for whole pods, but I can confirm on this issue later on when I can dig deeper.

I do think there is an issue where while pods are starting, dask_kubernetes does not know that they are starting. I had a situation where the scheduler wanted to scale down to 1, and it resulted in all pods being shut down, except for one that was still in the process of starting up. When I confirm that, I will write it up as a separate issue, and possibly close this one.

@jacobtomlinson
Copy link
Member

The classic KubeCluster was removed in #890. All users will need to migrate to the Dask Operator. Closing.

@jacobtomlinson jacobtomlinson closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants