[unsupervised AI] Schedule resource-restricted tasks against free resources

[vedjaw]

Warning

This PR was written autonomously by an AI agent and has not been reviewed
by a human yet. Maintainers should ignore it until the human author has reviewed,
understood, and approved everything that the AI agent wrote.

Resolves #9108.

Read this part first

This is a behaviour change, not a self-evident bug fix, and I want to be upfront about what the investigation actually found rather than argue only the side that supports the diff.

The issue's second claim does not hold. It says the scheduler "makes poor decisions based on wrong resource state". ws.used_resources has exactly six references in the tree: its declaration, its initialisation, the increment in acquire_resources, the decrement in release_resources, the reset in add_resources, and distributed/http/templates/worker.html. It is never read by any scheduling decision. valid_workers reads self.resources, the declared totals. So the over-commitment is not feeding back into placement.

The over-assignment looks deliberate. test_dont_steal_resource_restrictions and (before this PR) test_steal_resource_restrictions both assert that 100 tasks asking for A: 1 are all assigned to a worker declaring A: 2. The worker throttles execution itself in _resource_restrictions_satisfied, so handing it more than it can run at once is how it stays fed without a scheduler round-trip between tasks.

Put together: the concrete harm from #9108 is that used_resources reports assigned rather than executing, so the dashboard can show 45 of 10 in use. This PR fixes that by making the scheduler stop over-committing, which is the interpretation the issue asks for — but it does so by changing scheduling behaviour that appears to be intended, and it required rewriting one existing test. A maintainer may well prefer to keep the current admission behaviour and change what the counter or the dashboard means instead. I have no way to judge which of those you want, and @jacobtomlinson said in the issue that there is nobody available to guide the direction, so I would rather hand over the evidence than quietly pick.

What the change does

Three coordinated pieces. Each is load-bearing; I verified the tests fail with any one of them removed.

1. valid_workers considers free resources. A worker only counts for a resource restriction when it has enough of that resource unused.

2. Held-back tasks get retried. Subtracting used_resources on its own is not enough: tasks that no longer fit land in no-worker, and the only event that revisited no-worker tasks was a worker joining, so they stayed there forever. This is the failure mode @g199209 hit with the monkeypatch in the issue thread. stimulus_resources_maybe_released() is a sibling of stimulus_queue_slots_maybe_opened(), called from the same sites, and retries those tasks as running tasks release what they hold. Tasks are transitioned one at a time so each takes its resources before the next is considered, which both avoids bouncing tasks back to no-worker and keeps the retry bounded by what actually fits.

3. no-workers-timeout no longer kills them. A task waiting for a busy resource is not a task with unsatisfiable restrictions, but _check_unrunnable_task_timeouts would have failed it with NoValidWorkerError. valid_workers grows an only_available argument so the timeout can ask the different question — are these restrictions satisfiable at all — and skip tasks that are merely waiting.

The test I had to change

test_steal_resource_restrictions used A: 2 and A: 4 and expected all 100 tasks to be assigned to one worker, which only held because of the over-commitment this PR removes. I raised both capacities to A: 100 so the assignment is legitimate and the test still checks what it is named for, that stealing rebalances resource-restricted tasks. test_dont_steal_resource_restrictions still covers a thief that cannot satisfy the restriction at all, and test_steal_resource_restrictions_asym_diff passes untouched.

If you would rather that test keep its original capacities, that is a signal this PR is the wrong direction and I would close it rather than argue.

Tests added

• test_resources_not_over_allocated — the scenario from the issue. Asserts used_resources never exceeds what the worker declared and that the surplus tasks are visible as unrunnable instead of being reported as processing. Fails on main.
• test_resource_tasks_rescheduled_when_resources_released — a held-back task becomes runnable when a blocker finishes, with no new worker joining. Hangs until timeout without piece 2.
• test_no_workers_timeout_does_not_fail_tasks_awaiting_resources — with no-workers-timeout: 100ms, a task waiting on a busy resource survives. Fails on main.
• test_no_workers_timeout_still_fails_unsatisfiable_resources — a genuinely impossible restriction still times out.

What I ran

test_resources.py and test_steal.py in full: 108 passed, 18 skipped (skips are missing pandas/numpy locally).

test_scheduler.py: 527 passed. Eight failures (test_scheduler_file, test_profile_metadata, test_async_context_manager, test_finished, test_no_dangling_asyncio_tasks, test_multiple_listeners ×2, test_transition_failure_triggers_log_event) reproduce identically on an unmodified checkout on this machine — macOS, Python 3.14, no bokeh — so they are environmental, not from this change. I would want CI to confirm that.

I have not benchmarked the throughput effect of piece 2. Resource-restricted tasks now get a scheduler round-trip per released slot instead of sitting pre-assigned in a worker's queue, and on short tasks that could measurably hurt. That is the strongest argument against this direction and it deserves a number before anyone merges it.

[github-actions]

Unit Test Results

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

    40 files  ± 0      40 suites  ±0   15h 9m 46s ⏱️ + 24m 32s
 4 163 tests + 4   3 982 ✅ + 1    178 💤 ±0   3 ❌ + 3 
81 020 runs  +79  76 761 ✅ +58  4 236 💤  - 2  23 ❌ +23 

For more details on these failures, see this check.

Results for commit 7aac210. ± Comparison against base commit 40fcd99.

[vedjaw]

CI settles the design question this PR raised, and not in this PR's favour. Recording it here rather than papering over it.

distributed/tests/test_cancelled_state.py::test_cancelled_error_with_resources fails 20/20 runs, and reproduces locally as a 30s timeout. The test does:

@gen_cluster(client=True, nthreads=[("", 1, {"resources": {"A": 1}})])
...
fut1 = c.submit(block_execution, ..., resources={"A": 1})   # holds the only A
await executing.wait()
fut2 = c.submit(inc, 1, resources={"A": 1})
while fut2.key not in a.state.tasks:                        # <-- spins forever now
    await asyncio.sleep(0.01)

It waits for fut2 to be assigned to the worker while fut1 still holds the only unit of A. With admission control the scheduler correctly refuses, fut2 stays in no-worker, and the loop never exits.

That makes four existing tests that depend on the scheduler over-assigning resource-restricted tasks — test_dont_steal_resource_restrictions, test_steal_resource_restrictions, test_steal_resource_restrictions_asym_diff (2/20 on Windows here), and now this one, which is about worker-side cancellation and only uses resources as scaffolding. The behaviour is load-bearing across two unrelated subsystems, not an accident in one test.

Combined with what I noted at the top of this PR — ws.used_resources is read by nothing except distributed/http/templates/worker.html, so the over-commitment never feeds a scheduling decision — the conclusion is that this is designed behaviour: the scheduler hands a worker its share, the worker throttles execution in _resource_restrictions_satisfied, and the counter is a reporting field whose semantics are "assigned", not "executing".

So I don't think this PR should be pursued. Rewriting a fourth deliberate test to accommodate it would be bending the project around a premise that doesn't hold. If #9108 is worth acting on, the tractable part is that the dashboard can display 45 of 10 in use — a reporting fix, not an admission-control change.

Leaving the branch up in case the analysis is useful. Recommend closing.

The two other red signals here are unrelated to this diff: mypy fails on distributed/shuffle/tests/test_merge.py:27 and test_shuffle.py:80 with unused-type: ignore, in files this PR does not touch (other open PRs show the same), and test_nanny failed 1/20.