Skip to content

Scheduler: subtracting a request from a worker load skips resources the worker does not have - #1137

Open
chansigit wants to merge 1 commit into
It4innovations:mainfrom
chansigit:fix-workerload-bounds
Open

chansigit wants to merge 1 commit into
It4innovations:mainfrom
chansigit:fix-workerload-bounds

Conversation

@chansigit

Copy link
Copy Markdown

Fixes #1135.

WorkerResources::n_resources spans only the resources a worker declared (from_description), while a request may name a resource that another worker provides. get already treats such an id as zero, but remove, remove_multiple and remove_multiple_masked indexed the vector directly. With task priorities in use the scheduler's gap computation (GapCache::get_gap) reaches them with such requests (a multi-variant request whose variant needs a gpus-like resource, evaluated on a CPU-only worker), and the server died with index out of bounds: the len is 3 but the index is 4 at workerload.rs:160.

A worker holds none of a resource it did not declare, so there is nothing to subtract: the three functions now go through one subtract helper that skips ids past the vector.

Adds test_compute_gap_resource_the_worker_lacks in gap.rs (a cpus-only worker and a request naming a resource the worker lacks), which panics before this change.

Verification: the production-shaped stream from the issue (run.sh / driver.py) crashed the 2026-09-25 nightly within 16–18 s in every run; with this change the same stream ran 4/4 × 45 s without a crash.

…he worker does not have

`WorkerResources::n_resources` spans only the resources the worker declared (`from_description`), while a request
may name a resource another worker provides. `get` already returns zero for such an id, but `remove`,
`remove_multiple` and `remove_multiple_masked` indexed the vector directly, and the scheduler's gap computation
(`GapCache::get_gap`) reaches them with such requests once task priorities are in use: the server died with
`index out of bounds: the len is 3 but the index is 4` at workerload.rs:160 (issue It4innovations#1135). A worker holds none of a
resource it did not declare, so there is nothing to subtract. Adds a gap test with a cpus-only worker and a request
that names a resource the worker lacks (panics before this change).
@chansigit

chansigit commented Sep 27, 2026 •

Copy link
Copy Markdown
Author

For context, from our side: with this change (main + bounds-safe remove*) the stream from #1135 that crashed the 2026-09-25 nightly within 16–18 s in every run survives every run (4 × 45 s), and the full tako test suite passes. We validated it and are moving our deployment to a local build of main + #1136 + this change.

#1136 fixes the underlying gap computation, which is the real fix, and it also passes the same stream. This PR is a different, complementary kind of change: it only makes WorkerResources::remove / remove_multiple / remove_multiple_masked consistent with get (a resource the worker never declared is simply not there), so a future caller that hands them such a request cannot bring the server down again. Whether that safety net is worth keeping on top of #1136 is your call; feel free to close this if you'd rather not have it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant