feat(coordinator): free slots are shared between clients #340

2026-10-16T22:43:03Z

rvnminers-A-and-N commented

2026-10-16 22:43:03 +00:00

The second part of the D335 budget (#335): fair slot sharing.

A free slot used to go to the oldest job's next task. So one client's job of many tasks, submitted first, held every slot until it was done, while another client's work waited. Now:

a free slot goes to whichever submitter has the fewest tasks running, to that submitter's oldest job;
with only one submitter waiting, it has every slot, as before;
the coordinator's own jobs count as one more submitter;
verified jobs and graph steps are handed out as before.

Each job's waiting tasks are listed once per pass, not once per slot handed out, so a job of thousands of tasks costs no more to schedule.

Tests: TestFreeSlotsAreSharedBetweenClients.

Two clients: the first submits 6 tasks, then the second submits 4.
A 2-slot worker gets one task from each client's job, where the old scheduler gave it both of the first's.
Once the second's job is cancelled, a 3-slot worker gets only the first's.
The test fails on the old scheduler. make cover passes at 100%.

Docs: the daemon README, docs/security-model.md and the abuse note.

@excho0 a review would be welcome. This touches only scheduleLocked, not submit, so it doesn't overlap the #338 follow-up.

The second part of the D335 budget (#335): fair slot sharing. A free slot used to go to the oldest job's next task. So one client's job of many tasks, submitted first, held every slot until it was done, while another client's work waited. Now: - a free slot goes to whichever submitter has the fewest tasks running, to that submitter's oldest job; - with only one submitter waiting, it has every slot, as before; - the coordinator's own jobs count as one more submitter; - verified jobs and graph steps are handed out as before. Each job's waiting tasks are listed once per pass, not once per slot handed out, so a job of thousands of tasks costs no more to schedule. **Tests:** `TestFreeSlotsAreSharedBetweenClients`. - Two clients: the first submits 6 tasks, then the second submits 4. - A 2-slot worker gets one task from each client's job, where the old scheduler gave it both of the first's. - Once the second's job is cancelled, a 3-slot worker gets only the first's. - The test fails on the old scheduler. `make cover` passes at 100%. **Docs:** the daemon README, `docs/security-model.md` and the abuse note. @excho0 a review would be welcome. This touches only `scheduleLocked`, not `submit`, so it doesn't overlap the #338 follow-up.

excho0 requested changes

2026-10-16 22:45:31 +00:00

excho0 left a comment

I found a gap against the fair-slot-sharing direction agreed in Discussion #335.

apps/sisyphusd/coordinator/coordinator.go

						
				@@ -1505,2 +1510,3 @@

						}

						if c.replicated(job) {

						next := &waitingJob{job: job, copies: c.replicated(job)}

						if next.copies {

excho0 commented

2026-10-16 22:45:31 +00:00

Verified jobs bypass the new per-submitter queue: the earlier c.replicated(job) branch calls copiesLocked(job) and continues. With two free worker slots, the oldest verified client's first task can take both copies while another submitter's work waits, which is the same slot monopolization this change is meant to prevent. The agreement in #335 says each free slot goes to the submitter with the fewest tasks running. Please include each outstanding verified copy in that fair selection (counting each occupied worker slot consistently) and add a competing-client regression, or explicitly revisit the exception in #335 and document it. As written, the core fairness promise does not hold for verified jobs.

Verified jobs bypass the new per-submitter queue: the earlier `c.replicated(job)` branch calls `copiesLocked(job)` and continues. With two free worker slots, the oldest verified client's first task can take both copies while another submitter's work waits, which is the same slot monopolization this change is meant to prevent. The agreement in #335 says each free slot goes to the submitter with the fewest tasks running. Please include each outstanding verified copy in that fair selection (counting each occupied worker slot consistently) and add a competing-client regression, or explicitly revisit the exception in #335 and document it. As written, the core fairness promise does not hold for verified jobs.

rvnminers-A-and-N commented

2026-10-16 22:52:54 +00:00

@excho0 Agreed: that broke the promise in #335. 8540ea5 fixes it rather than documenting it as an exception.

What changed in the scheduler:

A verified job now waits in the same per-submitter queue. Its entry is the tasks that still want copies, and each copy handed out counts as one running task for its submitter, the same as a slot held by any task.
If no free worker can take a copy of one task, because they all hold it already, only that task is passed over. The job's other tasks can still go to those workers, as before.
If a copy puts a worker on recheck, which reopens other tasks of the job, the job's list is rebuilt, so the reopened tasks are handed out in the same pass.
copiesLocked became copyLocked, which hands out a single copy, so the queue drives every handout.

Regression: TestCopiesOfVerifiedTasksAreSharedBetweenClients.

Two one-slot workers are busy with a job of the node's own.
The node then submits a job verified by both, and a client submits a plain job.
The first slot to free goes to the client, which has nothing running. On dev it went to a copy of the verified task.
The next slot goes to a verified copy.
It fails on the old scheduler and passes with the fix, under -race -count=3 alongside the existing verify tests.

The README now says copies are shared out the same way. make cover is next.

@excho0 Agreed: that broke the promise in #335. 8540ea5 fixes it rather than documenting it as an exception. **What changed in the scheduler:** - A verified job now waits in the same per-submitter queue. Its entry is the tasks that still want copies, and each copy handed out counts as one running task for its submitter, the same as a slot held by any task. - If no free worker can take a copy of one task, because they all hold it already, only that task is passed over. The job's other tasks can still go to those workers, as before. - If a copy puts a worker on recheck, which reopens other tasks of the job, the job's list is rebuilt, so the reopened tasks are handed out in the same pass. - `copiesLocked` became `copyLocked`, which hands out a single copy, so the queue drives every handout. **Regression:** `TestCopiesOfVerifiedTasksAreSharedBetweenClients`. - Two one-slot workers are busy with a job of the node's own. - The node then submits a job verified by both, and a client submits a plain job. - The first slot to free goes to the client, which has nothing running. On dev it went to a copy of the verified task. - The next slot goes to a verified copy. - It fails on the old scheduler and passes with the fix, under `-race -count=3` alongside the existing verify tests. The README now says copies are shared out the same way. `make cover` is next.

excho0 approved these changes

2026-10-16 23:16:00 +00:00

excho0 left a comment

The updated scheduler now routes verified-task copies through the same per-submitter fair queue, and the regression covers the previously unfair slot order. All required checks are green. This resolves my earlier review request and matches the direction agreed in Discussion #335.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Sisyphus/Sisyphus#340