Repository navigation
Stage 3.2: role queue caps, isolated workers and lease recovery - #1734
Merged
Merged
Conversation
A per-role queue cap (8 by default, capped per graph) refuses an enqueue over it naming the role and the cap, and a re-offer keeps the card's id. Every claim gets its own worker, and requeueing or an expired lease drops it, so no claim reads another's state. Expired leases are recovered by one read that records the recovery on the card's evidence and requeues it exactly once; a live lease still refuses a claim naming its holder and survives a restart. Deliveries and worker seconds are counted from the rows, per role and per run. A graph marked strict starts one node at a time in declared order. The workflow plugin reads the lease and the counters through the host. Docs-not-needed: no — docs/subsystems/core-host.md and workflow.md updated. Written-by: Claim 0f76
A per-role queue cap (8 by default, capped per graph) refuses an enqueue over it naming the role and the cap, and a re-offer keeps the card's id. Every claim gets its own worker, and requeueing or an expired lease drops it, so no claim reads another's state. Expired leases are recovered by one read that records the recovery on the card's evidence and requeues it exactly once; a live lease still refuses a claim naming its holder and survives a restart. Deliveries and worker seconds are counted from the rows, per role and per run. A graph marked strict starts one node at a time in declared order. The workflow plugin reads the lease and the counters through the host. The patch-series guard now expects the new 0017 entry. Docs-not-needed: no — docs/subsystems/core-host.md and workflow.md updated. Written-by: Claim 0f76
Collaborator
Author
|
Verdict: PASS at a6dc7c9 Ran at this tip:
Not run: the smoke bench (no |
This was referenced Oct 29, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Claim 0f76
Fixes #1729
Part of #1550
Owner: Chief
Reviewer: Reviewer
Tip SHA: a6dc7c9
Base: 80709a7
Docs-not-needed: no —
docs/subsystems/core-host.mdanddocs/subsystems/workflow.mdupdated in this PR.What changed
Role queues carry a stated cap.
ROLE_QUEUE_CAP(8) is the default ceiling for one role's queue in a group; a graph states its own throughcapon the enqueued item, kept on the card asqueueCap. An enqueue over the ceiling is refusedqueue-fullwith a sentence naming the role and the cap, and a re-offer keeps the card's id and is never counted against itself.Every claim gets its own worker. The claim creates
worker-<hex>and names it on the card;rt.taskWorkerState(id)is the only read, so a second claim cannot see the first'sfilesorvars. Requeueing and an expired lease drop the worker, so the next claim starts clean.One owner for an expired lease.
expireLeasesreads the lease once, clears it, then returns the card to its role queue, so two expired reads requeue once. The recovery is written to the card'sevidence(lease expired (held by <name>)), the dead worker is dropped, and a card that is not a queued item only loses the dead lease. A live lease refuses a claim naming its holder and survives a restart unchanged.rt.taskLease(id)andrt.taskExpireLeases()are the host reads; the workflow plugin gets them throughfb.tasks.lease/fb.tasks.countersand keeps no copy.Measured deliveries from the rows.
rt.taskCounters({ role?, run? })counts per-role and per-run states,deliveries(accepted cards) andworkerSeconds(accepted cards with a lease). No second store.Strict DAG is an option.
strict: trueon a graph starts one node at a time in declared order with no parallel fan-out; the default and the release-shaped delivery template are unchanged.Why
Stage 3.2 of #1550, second slice of Stage 3. A card claimed by a dead worker used to stay claimed until someone noticed, and two runs of one role could write through one worker's state. Both are recovery and isolation questions, so they live on the pool, not the scheduler. #1720 shipped the typed fields (
lease,evidence); this slice consumes them and does not invent a second record or store.Tests
core/test/task-queue-lease.test.js(new): cap refusal names role and cap; graph overridecap: 2; re-offer keeps the id; two concurrent claims share no worker state; two expired reads requeue once and record the holder; a live lease refuses a claim naming the holder; a restart with a live lease changes nothing; counters read from the rows.plugins/workflow/test/strict-order.test.js(new): a strict graph starts one node at a time in declared order; the same graph without the flag still fans out; the delivery template passes unchanged.core/test/task-queue.test.js,task-contract.test.js,task-route.test.js— 21/21 pass.Red first
Reverting the four source files (
core/src/host/tasks.js,tools.js,hooks.js,plugins/workflow/src/graph.js) to the base blob turns the new suites red:core/test/task-queue-lease.test.js7 fail / 0 pass,plugins/workflow/test/strict-order.test.js3 fail / 0 pass. Restored, both are green (7/7 and 3/3).Ran / not run
Ran at the tip:
TZ=UTC npm run verifyin CI —tests-okgreen ata6dc7c94b4b046533da62b0f902432612ccbd86e(core suite, all four plugin suites, lint, plugin lint,build-client --check,ds-bot-patches check,package). This box has nogitbinary, so the same command run locally stops atarch-docs("cannot diff against origin/main"); the checks that do not need git ran here and are green: lint 0 errors, plugin lint,build-client --check,ds-bot-patches check(11 patches apply cleanly),core/test/task-queue-lease.test.js7/7,.github/scripts/ds-bot-patches.test.mjs5/5, and the directly related host suites (task-queue,task-contract,task-route) 21/21. The full local core run hits this container's thread limit (SIGABRT in unrelated files), not a code failure.Not run: the smoke bench (no
index.jsor session-path change), and the DSH real-model path.Risk
T2/T3 expected from risk-gate: host queue state (
core/src/host/) + workflow plugin + generated patches. The default cap (8) and the default non-strict graph keep existing behavior unless a caller opts in.{ "pr": 1734, "claim": "Chief", "issue": 1729, "summary": "Adds a per-role queue cap with a refusal naming the role and cap, one isolated worker per claim, one owner for expired leases that requeues a card exactly once and records the recovery on the card's evidence, per-role/per-run counters read from the rows, and an opt-in strict graph order. The workflow plugin reads the lease and counters through the host. Defaults keep the previous behavior.", "files": [ "core/src/host/tasks.js", "core/src/host/tools.js", "core/src/host/hooks.js", "core/test/task-queue-lease.test.js", "plugins/workflow/src/graph.js", "plugins/workflow/test/strict-order.test.js", "docs/subsystems/core-host.md", "docs/subsystems/workflow.md", "changelog.d/1729.added.md", "patches/ds-bot/0017-task-pool-role-queues.patch" ], "checks": { "ran": "TZ=UTC npm run verify in CI (tests-ok green at the tip)", "result": "green", "at": "a6dc7c94b4b046533da62b0f902432612ccbd86e" }, "next": "none — ready for review" }