dask / dask/distributed

Is it intended that any error from a handler makes `Server.handle_stream` close the comm?

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

Description

In https://github.com/dask/distributed/issues/5480, an unexpected error (https://github.com/dask/distributed/issues/5482) in a stream handler caused the entire stream to close. It turns out we didn't properly reconnect when the worker<->scheduler stream closed (https://github.com/dask/distributed/issues/5481), so things got into a broken state and deadlocked.

I'm curious though if it makes sense for `handle_stream` to close the whole stream in this case though. You can see here that there are a couple unhandled error cases that would cause the comm to close:
https://github.com/dask/distributed/blob/11c41b525267cc144caa8d077b84a0939fecae97/distributed/core.py#L559-L564

* `stream_handlers[op]` does not exist (message requesting an invalid op).
* `handler(**merge(extra, msg))` raises an error (what happened here).
* Interestingly, if `handler` is async, the stream will stay open even if `handler` fails whenever it runs in the future. Why the inconsistency with synchronous?

I'm not quite sure what the protocol is meant to be here. Is closing the comm the only way we have to tell the sender that something went wrong, so we're using it as a signal? Or do we believe that after any failure, we can't trust subsequent messages to be valid, so we should give up and wait to restart the connection if desired?

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.