search

dask / distributed

A distributed task scheduler for Dask
https://distributed.dask.org
BSD 3-Clause "New" or "Revised" License
1.57k stars 718 forks source link

Better documentation for `as_completed` class #3856

Open xKHUNx opened 4 years ago

xKHUNx commented 4 years ago

I would like to request for some improvement on the documentation for the class as_completed as found in here.

I'm having a use case where I had two different instances of Client object: client_1 and client_2. While using as_completed to gather futures from client_1, the loop is defaulted to the last Client object declared, which happens to be client_2. This resulted in the futures not returned properly and the tasks is stuck in the worker memory after completion. The error ranges from no message at all, to pickling error for some reason, like this:

distributed.protocol.pickle - INFO - Failed to deserialize bytearray(b'\....`)
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/distributed/protocol/pickle.py", line 66, in loads
    return pickle.loads(x)
_pickle.UnpicklingError: invalid load key, '\x00'.
distributed.protocol.core - CRITICAL - Failed to deserialize
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/distributed/protocol/core.py", line 130, in loads
    value = _deserialize(head, fs, deserializers=deserializers)
  File "/usr/local/lib/python3.7/site-packages/distributed/protocol/serialize.py", line 302, in deserialize
    return loads(header, frames)
  File "/usr/local/lib/python3.7/site-packages/distributed/protocol/serialize.py", line 64, in pickle_loads
    return pickle.loads(x, buffers=buffers)
  File "/usr/local/lib/python3.7/site-packages/distributed/protocol/pickle.py", line 66, in loads
    return pickle.loads(x)
_pickle.UnpicklingError: invalid load key, '\x00'.

It took me a long time, after reviewing the code here that I realized that we need to set the loop attribute to the loop of the corresponding Client object.

I think it can be make more clearer to user by explicitly stating it in the documentation for as_completed.

mrocklin commented 4 years ago

Thank you for reporting your experience @xKHUNx . In general documentation is good. However this case is sufficiently rare (multi-client workloads are very uncommon) that we wouldn't want to spend too much time in a documentation string on this topic. There are probably twenty similar issues that could occur with greater frequency and listing all of them makes the documentation long enough that no one reads it.

Instead, I wonder if it would make more sense to add a small warning in default_client if there are multiple clients present. The challenge here is that for people like you where this is the intended behavior we wouldn't want this warning to be too loud and obnoxious.

xKHUNx commented 4 years ago

Instead, I wonder if it would make more sense to add a small warning in default_client if there are multiple clients present.

That might be a good idea.

But I'm curious, why is some of the functions hidden from the API page?

mrocklin commented 4 years ago

But I'm curious, why is some of the functions hidden from the API page?

There are many internal functions that we don't think will be very useful to users. If we include all functions of the project then it is harder to find the functions that people want.