Rewrite Client-Scheduler Graph Transfer
- 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
Assessment
This issue has not been assessed yet.