-
-
Notifications
You must be signed in to change notification settings - Fork 727
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
Make stealing more robust #8788
Make stealing more robust #8788
Conversation
@@ -569,7 +569,9 @@ def __hash__(self) -> int: | |||
return self._hash | |||
|
|||
def __eq__(self, other: object) -> bool: | |||
return isinstance(other, WorkerState) and other.server_id == self.server_id | |||
return self is other or ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should speed up equality checks since we should mostly (always?) be using the same instance anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should always be the same. I'd even be fine with removing __eq__
entirely in which case ==
is the same as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd even be fine with removing
__eq__
entirely in which case==
is the same asis
I like the idea, but I'd leave it for a separate PR.
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 29 files ±0 29 suites ±0 11h 59m 25s ⏱️ -58s For more details on these failures, see this check. Results for commit 9981803. ± Comparison against base commit 4adf564. |
ts = self.scheduler.tasks[key] | ||
self.put_key_in_stealable(ts) | ||
elif start == "processing": | ||
if start == "processing": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put an in code comment here since this will otherwise die in a refactoring
This PR improves a few details in the stealing code that could potentially cause the scheduler to deadlock. I don't have any reproducer that could confirm this, but I've identified these weak points while investigating #8787. Fixing these should be strictly beneficial.
pre-commit run --all-files