Tweaks to update_graph (backport from #8185) #8498
Conversation
distributed/scheduler.py
Outdated
| dsk = dsk2 | ||
|
|
||
| # - Add in deps for any tasks that depend on futures | ||
| for k, futures in fut_deps.items(): | ||
| dependencies[k].update(f.key for f in futures) | ||
| dependencies[k].update(f.key for f in futures if f.key != k) |
There was a problem hiding this comment.
This is from @fjetter and I'm unclear about the use case for it...
Rest of the changes to the function are non-functional.
There was a problem hiding this comment.
I don't really follow this either.
There was a problem hiding this comment.
I'm going to revert it to unblock this PR. Once @fjetter responds, I'll potentially write a dedicated PR (with unit test to demonstrate the edge case).
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 27 files + 1 27 suites +1 10h 23m 21s ⏱️ + 47m 48s For more details on these failures, see this check. Results for commit 471715c. ± Comparison against base commit 9a9468c. ♻️ This comment has been updated with latest results. |
crusaderky
force-pushed
the
update_graph
branch
from
cbdb6de to
e2fc981
Compare
crusaderky
force-pushed
the
update_graph
branch
2 times, most recently
from
271e037 to
7d876e4
Compare
crusaderky
force-pushed
the
update_graph
branch
from
7d876e4 to
d21a179
Compare
There was a problem hiding this comment.
Thanks, @crusaderky! CI failure is unrelated. There's one mysterious change in here that I'd like for us to understand better (cc @fjetter), but that's non-blocking.
distributed/scheduler.py
Outdated
| dsk = dsk2 | ||
|
|
||
| # - Add in deps for any tasks that depend on futures | ||
| for k, futures in fut_deps.items(): | ||
| dependencies[k].update(f.key for f in futures) | ||
| dependencies[k].update(f.key for f in futures if f.key != k) |
There was a problem hiding this comment.
I don't really follow this either.
Miscellaneous non-controversial tweaks to update_graph, backported from #8185 (which may not happen for some time).
Notably, if update_graph fails this PR notifies the client who invoked update_graph in addition to all other clients in who_wants.
I didn't write a unit test as I'm unsure of how to trigger this use case without #8185.