Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions distributed/tests/test_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1271,3 +1271,50 @@ def test_startup2():
# Final exception test
with pytest.raises(ZeroDivisionError):
yield c.register_worker_callbacks(setup=lambda: 1 / 0)


@gen_cluster(client=True)
def test_worker_keeps_data_gh2420(c, s, a, b):
# Make a piece of data which is slow to serialize, to trigger the race
# Put data X on worker A
# Give task Y to worker B so that it fetches X from A
# Cancel the task on B
# Wait until the transfer is done
# Ensure that the scheduler's who_has for X is accurate
class SlowSerializeObj(object):
"""An object which is slow to serialize and deserialize."""
def __getstate__(self):
import time
time.sleep(1)
return {}

def __setstate__(self, d):
import time
time.sleep(1)

x = c.submit(SlowSerializeObj, workers=a.address)
yield wait(x)
assert not a.executing
assert x.key in a.data

def bogus_task(bogus_input):
import time
time.sleep(1)
return 2

y = c.submit(bogus_task, x, workers=b.address)
yield c._cancel(y)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can safely drop the underscore here. The public functions will operate asynchronously in an asynchronous context.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly concerned that y may never start. Should we wait until b is executing something?

while not b.executing:
    yield gen.sleep(0.01)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think it's important that y never starts - we need to cause x to be transferred from a to b, but then have y be cancelled while that transfer is occurring - so that x is never actually placed in memory on b. This timing is hard to arrange in the test (and I suspect actually now even harder with #2428),


# The notification to the scheduler happens in a batched send
# Make sure it has time to arrive
yield gen.sleep(5)
try:
yield wait(y)
except:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend except Exception: otherwise this catches KeyboardInterrupts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree this except is way too broad actually - it's really for the CancelledError. Will fix.

pass

# for entry in b.log:
# print("b log: %s" % str(entry))

assert (x.key in a.data) == (a.address in s.who_has[x.key])
assert (x.key in b.data) == (b.address in s.who_has[x.key])
4 changes: 0 additions & 4 deletions distributed/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1638,10 +1638,6 @@ def gather_dep(self, worker, dep, deps, total_nbytes, cause=None):
self.incoming_count += 1

self.log.append(('receive-dep', worker, list(response['data'])))

if response['data']:
self.batched_stream.send({'op': 'add-keys',
'keys': list(response['data'])})
except EnvironmentError as e:
logger.exception("Worker stream died during communication: %s",
worker)
Expand Down