dask / dask/distributed

Rewrite Client-Scheduler Graph Transfer

Open
#6,024 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
1.7k
Forks
778
Avg merge
2h 50m
Merged PRs (30d)
3

Description

Currently the system to send graphs to the scheduler is somewhat complex. This is due, I think, to working around a variety of constraints that may no longer actually constrain us. There has been effort to evolve past the old system. I'd like for us to engage in some thought about what a rewrite might look like.

### Naive Proposal

Let's just pickle the graph and ship it up to the scheduler, and then let the scheduler do whatever it wants to

```python
self.scheduler.send({
"op": "update-graph",
"graph": ToPickle(hlg),
})
```

### Complications

But there are a variety of complications to this today. Let's list them out:

1. The scheduler will call pickle.loads, which is potentially unsafe

We're proposing that we're ok with this

2. The scheduler will have to serialize tasks for the workers, rather than having the client do this

Maybe we're ok with this too

3. We'll have to unpack futures and things

Same. Maybe the scheduler can do this. Maybe futures get a little less eager to connect to a local Client

4. Stringification

Maybe we don't do this any more? Or maybe we do and just do it on the scheduler if we find that the performance benefit is significant.

### Less naive proposal

I propose that we ...

1. Change futures to not connect to a Client on deserialization, but instead on-demand
2. Ship the entire graph up to the scheduler with pickle
3. Remove the current `Layer.cull` method, replace it instead with a method that looks like the following:

```python
def get_graph(self, keys):
...
return dict_of_concrete_tasks, list_of_dependency_keys
```

If anything like a Future exists in the graph then it is this method's job to remove it, and provide something that can be safely serialized and shpiped to the worker.

The second return output here is the list of dependency keys to give to upstream layers that this layer requires

I'm probably missing lots of complications. I could use help to identify these. cc @rjzamora @ian-r-rose

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.