feat(speculation): allocator contract and sticky impl - #450
Open
behinddwalls wants to merge 1 commit into
Open
Conversation
This was referenced Jul 28, 2026
behinddwalls
force-pushed
the
preetam/speculation-allocator
branch
from
July 28, 2026 19:32
86a9654 to
10e097e
Compare
behinddwalls
marked this pull request as ready for review
July 28, 2026 19:59
sbalabanov
requested changes
Jul 28, 2026
sbalabanov
left a comment
sbalabanov
left a comment
Contributor
There was a problem hiding this comment.
critical comment about failing fast
|
|
||
| Because cancellation is best-effort, an `Allocator` should not spend capacity it merely expects a cancel to release — a build cancelled to make room keeps charging the budget until its cancel reaches a terminal state, so the queue converges over successive runs rather than oversubscribing the hard cap on concurrent builds in a single pass. | ||
|
|
||
| ## `sticky` |
Contributor
There was a problem hiding this comment.
move that into sticky/README.md instead
|
|
||
| ## `sticky` | ||
|
|
||
| The `sticky` `Allocator` fills only free budget slots and leaves every in-flight build running. Against the budget it counts the paths that still hold a build slot — `pending`, `building`, and `cancelling` (a cancel is a request, and the build keeps its slot until it actually stops). It then proposes a build for each new candidate, in order, until the budget fills; it never proposes a cancel. |
Contributor
There was a problem hiding this comment.
add that 'sticky' operates the "builds" as a build unit budget (so, it is quite coarse).
More interesting implementations may operate with more granular units, understanding the size of the build needed to be done (i.e. based on number of targets, historical data etc).
| } | ||
|
|
||
| // New returns a sticky allocator.Allocator with the given concurrent-build | ||
| // budget. |
Contributor
There was a problem hiding this comment.
explain what unit of measure for the budget is
| // data defect. Skip it rather than counting it: a status that | ||
| // never turns terminal would pin a build slot forever. That leaves | ||
| // the path free to be proposed again, which is self-correcting, | ||
| // but the malformed entry is worth knowing about. |
Contributor
There was a problem hiding this comment.
SQ default on data inconsistencies is to fail fast and hard
behinddwalls
force-pushed
the
preetam/speculation-allocator
branch
from
July 29, 2026 18:12
10e097e to
bf09ae6
Compare
Add submitqueue/extension/speculation/allocator, the budget-spending composition point (Allocator) the standard Speculator hands its candidate stream to, plus the sticky implementation and mocks. sticky fills only free budget slots and never preempts a running build, trading responsiveness for never discarding work already started; its policy counterpart is a preempting allocator.
behinddwalls
force-pushed
the
preetam/speculation-allocator
branch
from
July 29, 2026 18:24
bf09ae6 to
6977b91
Compare
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
2 participants
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.
Summary
Why?
The
standardSpeculatorsplits into two halves: rank the candidate paths (#446), then spend the build budget on them. This is the spending half.Like the generator, it is not controller-facing — the controller knows only the
Speculatorcontract — so there is noConfigorFactory; anAllocatoris chosen when thestandardSpeculatoris constructed.What?
Adds
allocator(the contract) andallocator/sticky(the first implementation).Allocatetakes the queue's path sets and the generator's candidate iterator and returns the build and cancel actions for this run. Reconciling candidates against paths already in flight is the allocator's job; whether to preempt anything is its policy.stickynever preempts. It counts the paths still holding a build slot —pending,building, andcancelling(a cancel is a request; the build keeps its slot until it actually stops) — then pulls candidates in order into whatever is free and proposes a build for each new one. It never proposes a cancel. That trades responsiveness for never throwing away a build already started. Its counterpart, a preempting allocator, would cancel a low-value in-flight path to fund a better candidate.A path stored with no status at all is a data defect. It holds no slot — one that never reaches a terminal state would pin a slot forever, permanently lowering the queue's capacity — so it is skipped and logged as a warning rather than passing unnoticed.
Newtherefore takes a logger.Includes the package
README.mdand generated mocks.Test Plan
✅
bazel test //submitqueue/extension/speculation/...,make lint,make check-gazellesticky's table test covers filling free budget in candidate order; keeping an in-flight path funded while filling the remaining slot; which statuses hold a slot (pending,building, andcancellingdo, terminal ones do not); a path with no status holding nothing; deduplicating a repeated candidate; and propagating an iterator error. Two further tests assert the warning fires exactly once for a status-less path and stays silent otherwise.Both new behaviors were mutation-checked — reverting the skip, or removing the warn, each fails the suite — so the assertions are not passing vacuously.
Stack