Uh oh!
There was an error while loading. Please reload this page.
feat(orchestrator): poll builds and stop the ones nothing wants - #504
Open
behinddwalls wants to merge 1 commit into
Open
feat(orchestrator): poll builds and stop the ones nothing wants#504behinddwalls wants to merge 1 commit into
behinddwalls wants to merge 1 commit into
Conversation
behinddwalls
marked this pull request as ready for review
August 4, 2026 01:53
behinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 4, 2026 01:58
c2d441b to
37428c4Compare
This was referenced Aug 4, 2026
behinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 4, 2026 05:11
37428c4 to
c33a2d6Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 03:15
c33a2d6 to
6cabb60Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 03:26
6cabb60 to
8a077b1Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 03:38
8a077b1 to
492d1cbComparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 03:43
492d1cb to
65aad34Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 04:11
65aad34 to
3e3666fComparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 04:57
3e3666f to
7f2f2feComparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
2 times, most recently
from
August 5, 2026 17:28
246145c to
b2a17a5Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 5, 2026 19:05
b2a17a5 to
bf0df7aComparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 6, 2026 02:15
bf0df7a to
a6b5f8dComparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 6, 2026 02:34
a6b5f8d to
60970f0Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 6, 2026 03:04
60970f0 to
e780560Comparebehinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 6, 2026 03:17
e780560 to
4142ca7Compare## Summary ### Why? The poll loop was writing the path set on every terminal build — the third concurrent writer on a row where the speculate run, which holds a version across its whole Speculator call, was structurally the one to lose. And with the build stage now start-only, something has to enact cancellation. ### What? The poll loop becomes speculation's kill mechanism. On every poll of a non-terminal build it checks whether anything still wants the build running — batch not halted, the path's entry live and on this build's attempt, the attempt's link naming this very build — and asks the runner to cancel when nothing does. That one level-triggered check subsumes path cancels, batch halts, superseded attempts, and lost dispatch races: no cancel message exists to go stale, and a check that misses one poll is remade on the next. It cannot cancel a wanted build: every "unwanted" condition is permanent once true, so a stale read only errs toward keeping, and store anomalies (a set, entry, or link that cannot legitimately be missing) also keep the build — a cancel is irreversible. The Cancel call is best-effort so a failure never kills the poll chain that would retry it. The path set is read as that kill list and never written. Polls now partition on the build ID rather than the batch, and each re-poll mints a distinct message ID so the queue never dedups it away. The halted short-circuit stays removed: a cancelling batch reaches terminal only once its builds stop, and this loop is both what stops them and what watches them stop. ## Test Plan ✅ `bazel test //submitqueue/orchestrator/...` — every unwanted condition cancels; a wanted build never sees a Cancel; anomalies keep the build; a failed Cancel does not fail the poll; statuses recorded per terminal state; and no path-set write happens at all (the set store is wired read-only on the mock). ✅ `make fmt`, `make gazelle` # Conflicts: # submitqueue/orchestrator/controller/buildsignal/BUILD.bazel # submitqueue/orchestrator/controller/buildsignal/buildsignal.go # submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # interactive rebase in progress; onto 5658532 # Last command done (1 command done): # pick 246145c # feat(orchestrator): poll builds and stop the ones nothing wants # No commands remaining. # You are currently rebasing branch 'preetam/speculation-buildsignal-paths' on '5658532b'. # # Changes to be committed: # modified: submitqueue/orchestrator/controller/buildsignal/BUILD.bazel # modified: submitqueue/orchestrator/controller/buildsignal/buildsignal.go # modified: submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go # modified: submitqueue/orchestrator/controller/dlq/buildsignal.go # modified: submitqueue/orchestrator/controller/dlq/buildsignal_test.go #
behinddwallsforce-pushed
the
preetam/speculation-buildsignal-paths
branch
from
August 6, 2026 03:32
4142ca7 to
bd68e1fCompare
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 freeto 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.
Summary
Why?
The poll loop was writing the path set on every terminal build — the third concurrent writer on a row where the speculate run, which holds a version across its whole Speculator call, was structurally the one to lose. And with the build stage now start-only, something has to enact cancellation.
What?
The poll loop becomes speculation's kill mechanism. On every poll of a non-terminal build it checks whether anything still wants the build running — batch not halted, the path's entry live and on this build's attempt, the attempt's link naming this very build — and asks the runner to cancel when nothing does. That one level-triggered check subsumes path cancels, batch halts, superseded attempts, and lost dispatch races: no cancel message exists to go stale, and a check that misses one poll is remade on the next.
It cannot cancel a wanted build: every "unwanted" condition is permanent once true, so a stale read only errs toward keeping, and store anomalies (a set, entry, or link that cannot legitimately be missing) also keep the build — a cancel is irreversible. The Cancel call is best-effort so a failure never kills the poll chain that would retry it. The path set is read as that kill list and never written.
Polls now partition on the build ID rather than the batch, and each re-poll mints a distinct message ID so the queue never dedups it away. The halted short-circuit stays removed: a cancelling batch reaches terminal only once its builds stop, and this loop is both what stops them and what watches them stop.
Test Plan
✅
bazel test //submitqueue/orchestrator/...— every unwanted condition cancels; a wanted build never sees a Cancel; anomalies keep the build; a failed Cancel does not fail the poll; statuses recorded per terminal state; and no path-set write happens at all (the set store is wired read-only on the mock).✅
make fmt,make gazelleIssues