Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure client key cancellation uses ordered messages #7583

Merged
merged 4 commits into from Mar 8, 2023

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Feb 24, 2023

Another breakout from #7564

I can't specifically recall where I ran into this but while working on the refactor I somehow ran into race conditions around cancellation that prompted me to rewrite this section.


All administrative message, e.g. compute, release, cancel are sensitive to ordering. The current cancellation mechanism works around this by "waiting" for a task to show up because the current cancel was using an unordered RPC call (see also #7480)
The only other reason why one would want to poll at this point is if another client could cancel a given key. There is no API for this. User exposed APIs require the existence of a future object which, given message ordering, ensure that the task exists on the scheduler.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2023

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       26 files  +       2         26 suites  +2   5d 14h 51m 55s ⏱️ + 5d 4h 6m 33s
  3 491 tests +     79    3 387 ✔️ +     80     103 💤 ±    0  1  - 1 
44 123 runs  +3 966  42 055 ✔️ +3 740  2 066 💤 +227  2  - 1 

For more details on these failures, see this check.

Results for commit 83695f1. ± Comparison against base commit f9306da.

♻️ This comment has been updated with latest results.

@hendrikmakait hendrikmakait self-requested a review March 7, 2023 13:08
distributed/client.py Outdated Show resolved Hide resolved
distributed/scheduler.py Outdated Show resolved Hide resolved
distributed/scheduler.py Outdated Show resolved Hide resolved
Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

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

Thanks, @fjetter!

@hendrikmakait hendrikmakait merged commit 73606ec into dask:main Mar 8, 2023
@fjetter fjetter deleted the key_cancellation_ordered branch March 8, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants