Make decide_worker and rootish logic private

[hendrikmakait]

Baby-steps toward #8190

• Tests added / passed
• Passes pre-commit run --all-files

[fjetter]

I think we should nuke this entire document. definitely the reference to the methods

[hendrikmakait]

I don't care either way. If the interested user wants to read through those implementation details, sure, have at it.

[hendrikmakait]

...though I do see the challenge of keeping this up-to-date (which I'm not sure it currently is, and frankly, I'm also not going to read it in full and compare it to the status quo today)

[hendrikmakait]

Let's make this a follow-up issue in case someone else has strong opinions.

[hendrikmakait]

#8467

[github-actions]

Unit Test Results

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

    27 files  ±0      27 suites  ±0   9h 59m 50s ⏱️ + 7m 0s
 3 995 tests ±0   3 884 ✅ +1    110 💤 ±0  1 ❌  - 1 
50 241 runs  ±0  47 945 ✅ +2  2 295 💤 ±0  1 ❌  - 2 

For more details on these failures, see this check.

Results for commit 8aa7675. ± Comparison against base commit fcfa7bc.

♻️ This comment has been updated with latest results.

[barney54321]

I'm a bit late to the party but have only just noticed this change.

I'm working on a project where we override the rootish behaviour because tasks that have resource restrictions are not considered rootish, which (at least historically) results in them not being eligible for queuing and having very different scheduling behaviour, especially with dynamic workers (such as with a multi-spec adaptive cluster). The behaviour we observed when run with our use cases without our modification was that the first worker to spawn would receive every root task in the graph, with work stealing then balancing the workers. This in our experience would often lead to large idles times and sometimes even infinite loops deadlocking the scheduler. To overcome this we override rootish to always be true, disable work stealing and attach some heuristic worker-task assignments, as we found that for our use cases the resulting behaviour would have good load balancing and minimal idle times.

Given rootish has been made private, is there a public API that we can use to achieve the same behaviour as before? I see that TaskState has an attribute _rootish that is used in _is_rootish() and is referenced in the docstrings as being a means to override the behaviour, however it is both private and set to None in the __init__.

[hendrikmakait]

I'm a bit late to the party but have only just noticed this change.

I'm working on a project where we override the rootish behaviour because tasks that have resource restrictions are not considered rootish, which (at least historically) results in them not being eligible for queuing and having very different scheduling behaviour, especially with dynamic workers (such as with a multi-spec adaptive cluster).

This is still the case, tasks with worker restrictions are currently never considered rootish.

The behaviour we observed when run with our use cases without our modification was that the first worker to spawn would receive every root task in the graph, with work stealing then balancing the workers. This in our experience would often lead to large idles times and sometimes even infinite loops deadlocking the scheduler. To overcome this we override rootish to always be true, disable work stealing and attach some heuristic worker-task assignments, as we found that for our use cases the resulting behaviour would have good load balancing and minimal idle times.

The fact that you encounter deadlocks is alarming to me. If you can create a reproducer, I'd be interested in taking a look at that. While idle times may be expected depending on the workload and setup, deadlocks should not.

Given rootish has been made private, is there a public API that we can use to achieve the same behaviour as before? I see that TaskState has an attribute _rootish that is used in _is_rootish() and is referenced in the docstrings as being a means to override the behaviour, however it is both private and set to None in the __init__.

@barney54321: There is currently no API that I would consider public that allows you to override the scheduling behavior or rootish behavior. These APIs are rather fragile and changing them can easily result in deadlock or other undesirable behavior, so we generally do not encourage users to touch them. They are also subject to change without notice.

That being said, you could continue to override these APIs at your own risk. Historically, Dask has done a poor job at signaling which APIs are public vs. private. With this PR, we do not actively make a previously public API private. Instead, we make it explicit that these APIs have been considered private all along.