Optionally use pickle5

[jakirkham]

Fixes#2495

This extends the changes in PR ( #3784 ) to also support using pickle5 (when available). Depends on PR ( cloudpipe/cloudpickle#370 ), which is in cloudpickle version 1.5.0.

cc @pierreglaser@pitrou

[jakirkham]

Am seeing this failure. After debugging this more closely, it appears we need this change ( cloudpipe/cloudpickle#368 (review) ).

Details

__________________________test_feed_large_bytestring__________________________15631564deftest_func():
1565result=None1566workers= []
1567withclean(timeout=active_rpc_timeout, **clean_kwargs) asloop:
15681569asyncdefcoro():
1570withdask.config.set(config):
1571s=False1572foriinrange(5):
1573try:
1574s, ws=awaitstart_cluster(
1575nthreads,
1576scheduler,
1577loop,
1578security=security,
1579Worker=Worker,
1580scheduler_kwargs=scheduler_kwargs,
1581worker_kwargs=worker_kwargs,
1582 )
1583exceptExceptionase:
1584logger.error(
1585"Failed to start gen_cluster, retrying",
1586exc_info=True,
1587 )
1588awaitasyncio.sleep(1)
1589else:
1590workers[:] =ws1591args= [s] +workers1592break1593ifsisFalse:
1594raiseException("Could not start cluster")
1595ifclient:
1596c=awaitClient(
1597s.address,
1598loop=loop,
1599security=security,
1600asynchronous=True,
1601**client_kwargs,
1602 )
1603args= [c] +args1604try:
1605future=func(*args)
1606iftimeout:
1607future=asyncio.wait_for(future, timeout)
1608result=awaitfuture1609ifs.validate:
1610s.validate_state()
1611finally:
1612ifclientandc.statusnotin ("closing", "closed"):
1613awaitc._close(fast=s.status=="closed")
1614awaitend_cluster(s, workers)
1615awaitasyncio.wait_for(cleanup_global_workers(), 1)
16161617try:
1618c=awaitdefault_client()
1619exceptValueError:
1620pass1621else:
1622awaitc._close(fast=True)
16231624defget_unclosed():
1625return [cforcinComm._instancesifnotc.closed()] + [
1626c1627forcin_global_clients.values()
1628ifc.status!="closed"1629 ]
16301631try:
1632start=time()
1633whiletime() <start+5:
1634gc.collect()
1635ifnotget_unclosed():
1636break1637awaitasyncio.sleep(0.05)
1638else:
1639ifallow_unclosed:
1640print(f"Unclosed Comms: {get_unclosed()}")
1641else:
1642raiseRuntimeError("Unclosed Comms", get_unclosed())
1643finally:
1644Comm._instances.clear()
1645_global_clients.clear()
16461647returnresult16481649result=loop.run_sync(
1650>coro, timeout=timeout*2iftimeoutelsetimeout1651 )
16521653distributed/utils_test.py:954: 1654_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 1655../../../miniconda/envs/dask-distributed/lib/python3.6/site-packages/tornado/ioloop.py:576: inrun_sync1656returnfuture_cell[0].result()
1657distributed/utils_test.py:912: incoro1658result=awaitfuture1659../../../miniconda/envs/dask-distributed/lib/python3.6/asyncio/tasks.py:358: inwait_for1660returnfut.result()
1661_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 16621663s=<Scheduler: "tcp://127.0.0.1:42742"processes: 0cores: 0>1664a=<Worker: 'tcp://127.0.0.1:33191', 0, Status.closed, stored: 0, running: 0/1, ready: 0, comm: 0, waiting: 0>1665b=<Worker: 'tcp://127.0.0.1:37948', 1, Status.closed, stored: 0, running: 0/2, ready: 0, comm: 0, waiting: 0>16661667 @gen_cluster()
1668asyncdeftest_feed_large_bytestring(s, a, b):
1669np=pytest.importorskip("numpy")
16701671x=np.ones(10000000)
16721673deffunc(scheduler):
1674y=x1675returnTrue16761677comm=awaitconnect(s.address)
1678awaitcomm.write({"op": "feed", "function": dumps(func), "interval": 0.05})
16791680foriinrange(5):
1681response=awaitcomm.read()
1682>assertresponseisTrue1683EAssertionError: assert {'exception': ValueError('The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()'...aywithmorethanoneelementisambiguous. Usea.any() ora.all()', 'traceback': <tracebackobjectat0x7f0a35b87cc8>} isTrue16841685distributed/tests/test_scheduler.py:377: AssertionError

Edit: Fixed upstream.

[jakirkham]

Am seeing this failure (snippet below). Think it has something to do with how the dispatch_table is being used in cloudpickle. Discussion can be found in thread ( cloudpipe/cloudpickle#368 (review) ).

Details

__________________________test_chained_error_message__________________________c=<Client: notconnected>s=<Scheduler: "tcp://127.0.0.1:46028"processes: 0cores: 0>a=<Worker: 'tcp://127.0.0.1:44402', 0, Status.closed, stored: 0, running: 0/1, ready: 0, comm: 0, waiting: 0>b=<Worker: 'tcp://127.0.0.1:45385', 1, Status.closed, stored: 0, running: 0/2, ready: 0, comm: 0, waiting: 0>
@gen_cluster(client=True)
asyncdeftest_chained_error_message(c, s, a, b):
defchained_exception_fn():
classMyException(Exception):
def__init__(self, msg):
self.msg=msgdef__str__(self):
return"MyException(%s)"%self.msgexception=MyException("Foo")
inner_exception=MyException("Bar")
try:
raiseinner_exceptionexceptExceptionase:
raiseexceptionfromef=c.submit(chained_exception_fn)
try:
>awaitfdistributed/tests/test_worker.py:406: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self=<Future: cancelled, key: chained_exception_fn-103274397419cf5bcfd188fdd51fd53a>raiseit=Trueasyncdef_result(self, raiseit=True):
awaitself._state.wait()
ifself.status=="error":
exc=clean_exception(self._state.exception, self._state.traceback)
ifraiseit:
typ, exc, tb=exc>raiseexc.with_traceback(tb)
distributed/client.py:235: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ >raiseexceptionfromeEtest_worker.test_chained_error_message.<locals>.chained_exception_fn.<locals>.MyException: MyException(Foo)
distributed/tests/test_worker.py:402: MyExceptionDuringhandlingoftheaboveexception, anotherexceptionoccurred:
deftest_func():
result=Noneworkers= []
withclean(timeout=active_rpc_timeout, **clean_kwargs) asloop:
asyncdefcoro():
withdask.config.set(config):
s=Falseforiinrange(5):
try:
s, ws=awaitstart_cluster(
nthreads,
scheduler,
loop,
security=security,
Worker=Worker,
scheduler_kwargs=scheduler_kwargs,
worker_kwargs=worker_kwargs,
)
exceptExceptionase:
logger.error(
"Failed to start gen_cluster, retrying",
exc_info=True,
)
awaitasyncio.sleep(1)
else:
workers[:] =wsargs= [s] +workersbreakifsisFalse:
raiseException("Could not start cluster")
ifclient:
c=awaitClient(
s.address,
loop=loop,
security=security,
asynchronous=True,
**client_kwargs,
)
args= [c] +argstry:
future=func(*args)
iftimeout:
future=asyncio.wait_for(future, timeout)
result=awaitfutureifs.validate:
s.validate_state()
finally:
ifclientandc.statusnotin ("closing", "closed"):
awaitc._close(fast=s.status=="closed")
awaitend_cluster(s, workers)
awaitasyncio.wait_for(cleanup_global_workers(), 1)
try:
c=awaitdefault_client()
exceptValueError:
passelse:
awaitc._close(fast=True)
defget_unclosed():
return [cforcinComm._instancesifnotc.closed()] + [
cforcin_global_clients.values()
ifc.status!="closed"
]
try:
start=time()
whiletime() <start+5:
gc.collect()
ifnotget_unclosed():
breakawaitasyncio.sleep(0.05)
else:
ifallow_unclosed:
print(f"Unclosed Comms: {get_unclosed()}")
else:
raiseRuntimeError("Unclosed Comms", get_unclosed())
finally:
Comm._instances.clear()
_global_clients.clear()
returnresultresult=loop.run_sync(>coro, timeout=timeout*2iftimeoutelsetimeout
)
distributed/utils_test.py:954: __ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../../miniconda/envs/dask-distributed/lib/python3.7/site-packages/tornado/ioloop.py:532: inrun_syncreturnfuture_cell[0].result()
distributed/utils_test.py:912: incororesult=awaitfuture
../../../miniconda/envs/dask-distributed/lib/python3.7/asyncio/tasks.py:442: inwait_forreturnfut.result()
__ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ c=<Client: notconnected>s=<Scheduler: "tcp://127.0.0.1:46028"processes: 0cores: 0>a=<Worker: 'tcp://127.0.0.1:44402', 0, Status.closed, stored: 0, running: 0/1, ready: 0, comm: 0, waiting: 0>b=<Worker: 'tcp://127.0.0.1:45385', 1, Status.closed, stored: 0, running: 0/2, ready: 0, comm: 0, waiting: 0>
@gen_cluster(client=True)
asyncdeftest_chained_error_message(c, s, a, b):
defchained_exception_fn():
classMyException(Exception):
def__init__(self, msg):
self.msg=msgdef__str__(self):
return"MyException(%s)"%self.msgexception=MyException("Foo")
inner_exception=MyException("Bar")
try:
raiseinner_exceptionexceptExceptionase:
raiseexceptionfromef=c.submit(chained_exception_fn)
try:
awaitfexceptExceptionase:
>asserte.__cause__isnotNoneEAssertionError: assertNoneisnotNoneE+whereNone=MyException('Foo').__cause__distributed/tests/test_worker.py:408: AssertionError

Edit: Should add this is the only failure I'm seeing in the full test suite.

[jakirkham]

We now have all tests passing using the development version of cloudpickle 😄

[jakirkham]

@fjetter, if you have time/interest, it might be worth trying this out. Would need to install pickle5 and cloudpickle, based off of PR ( cloudpipe/cloudpickle#370 ). If you do try this, would be interested in hearing how things go for you 🙂

[jakirkham]

cc-ing @jcrist (in case you are interested 😉)

[quasiben]

Generally, this looks good. I spoke briefly with @jakirkham about this and thought I should bring it up here as well. Do others have issues with bumping cloudpickle version. Are there dask subprojects that have a pinned cloudpickle version for a particular reason ?

[mrocklin]

I'm comfortable bumping the cloudpickle version. I don't know of anyone who is pinned. I feel a little odd bumping it to something that was released this week, but not entirely against.

…

On Thu, Jul 2, 2020 at 5:22 AM Benjamin Zaitlen ***@***.***> wrote: Generally, this looks good. I spoke briefly with @jakirkham <https://github.com/jakirkham> about this and thought I should bring it up here as well. Do others have issues with bumping cloudpickle version. Are there dask subprojects that have a pinned cloudpickle version for a particular reason ? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3849 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTBFYOAKU7PCI35PHV3RZR3XDANCNFSM4NREMG3Q> .

[jakirkham]

That's a fair concern. We could try to add some conditional logic here to handle an older cloudpickle. Alternatively we could just wait on merging this until after today's release ships, which would give us a 2 week gap between the cloudpickle release and this change.

[mrocklin]

A two week gap sounds good to me. I suspect that we'll have a good sense then if things are wonky.

…

On Thu, Jul 2, 2020 at 8:51 AM jakirkham ***@***.***> wrote: That's a fair concern. We could try to add some conditional logic here to handle an older cloudpickle. Alternatively we could just wait on merging this until after today's release ships, which would give us a 2 week gap between the cloudpickle release and this change. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3849 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTDLYLHTTVRLLUJP5ALRZSUJ7ANCNFSM4NREMG3Q> .

[jakirkham]

2.20.0 is now out. So I think we should be good here. Please let me know if we need anything else 🙂

[mrocklin]

How confident are we that defaults will pick this up by the next release cycle?

[jakirkham]

Sorry pick-up what?

[mrocklin]

cloudpickle 1.5

[jakirkham]

Sure. I mean that will be a requirement to update distributed. So I would think yes.

[jakirkham]

cc @jjhelmus (in case you have thoughts on updating cloudpickle in defaults)

[jjhelmus]

I do not foresee any issue updating cloudpickle prior to the next release of distributed in defaults. We are currently holding off building new packages as we are preparing for a release of Anaconda Individual Edition but once that is out updates should proceed at a normal pace.

In the future @anaconda-pkg-build is a good handle to ping on these type of issue as it goes to multiple engineers at Anaconda who do packaging.

[jakirkham]

In the future @anaconda-pkg-build is a good handle to ping on these type of issue as it goes to multiple engineers at Anaconda who do packaging.

Good tip! Thanks Jonathan. Will keep this in mind for the future 🙂

[jakirkham]

Any other questions @mrocklin? 🙂

[mrocklin]

1.5 seems to be released. If we're highly confident that there won't be any issues with this then yes, let's go ahead. If you think that there is some small chance of issues then let's wait. I think that you're probably the most educated person to make this call :)