Commit 86bfae8
committed
feat(speculation): reconcile dead paths and finalize from the tree
Two speculation.go behaviors were still driven off the pre-tree dependency-only model from earlier commits in this stack: dependency outcomes were folded into the batch via a bespoke tryFinalize/failOnDependency pair that only looked at batch dependency state and never at the tree, and per-path build outcomes were never reconciled back into the tree at all. A build finishing, or a dependency dying mid-flight, left stale Candidate/Selected/Building statuses that scoring and selection had to route around indirectly. This split made the tree an incomplete picture of a batch's speculation state and kept the merge decision in logic with no visibility into per-path build results.
speculateBatch now reconciles the tree against reality on every pass, before scoring. reconcile() folds each path's build outcome (via the path->build mapping and build store) into its status through reconcileStatus(), then walks the tree a second time to dead any path whose dependency chain can no longer land. Paths already at a terminal status (Passed/Failed/Cancelled) are settled and skipped without touching storage, and statuses are folded into the tree's own path slice in place (mirroring applyScores/applySelection — no per-pass copy), so a pass costs at most two point reads per unresolved path and stays O(paths) in reads and CPU while allocating nothing. The scaling rules for paths and trees — the enumerator implementation decides which and how many paths a batch gets, bounded frontier, never the 2^N power set of dependency subsets, O(paths) per pass — are captured in a new "speculation paths and trees" section in CLAUDE.md so future changes account for them.
reconcileStatus() is pure status arithmetic with two rules its doc now spells out: a terminal path status is never downgraded, and Cancelling records a decision about the path (deselected, or assumptions dead), not a prediction about the build — so a Cancelling path holds until its build reaches any terminal state and then settles to Cancelled, including a build that raced to Succeeded before the intent was enacted (its result is moot for a path that will never merge; the build stage never cancels an already-terminal build).
pathDead() treats a Failed base dependency, or a Succeeded non-base dependency, as fatal to a path; a Cancelled base dependency is deliberately tolerated (Phase-A leniency — a cancelled batch never lands and so cannot conflict, and with exactly one chain path per batch today, deading it would fail batches that currently survive). Tightening that case once multi-path enumeration lands is tracked in #369 and linked from the code.
A dead path that is still Building is captured as a cancel intent (Cancelling status) with no runner call. Actually cancelling the in-flight build is the build stage's job (build.go's enactCancel, added earlier in this stack), reached one hop later through the prioritize round speculate already publishes on every pass — prioritize's republishBuilds re-triggers the build stage for any tree with a Cancelling path. A dead path with no build yet drops straight to Cancelled.
finalize()/mergeableNow()/viable() replace tryFinalize()/failOnDependency(). finalize settles the batch's forward step in exactly one of three outcomes per pass, which its doc now enumerates: merge now (some path is mergeableNow — publish to merge, CAS to Merging), wait (no path mergeable but at least one still viable — no-op until the next event), or fail (no viable path remains — CAS to Failed, fan out to dependents, publish to conclude). mergeableNow checks the path's own status (must be Passed) in addition to its dependencies; viable distinguishes "still might merge" from "definitely won't" (Failed, Cancelled, and Cancelling paths are not viable). finalize runs unconditionally on every pass — it no longer needs the batch's original-state gate the previous commit introduced as a stopgap, since mergeableNow correctly requires a path to have actually passed before merging. The dependent fan-out on the fail outcome is publish-only — one speculate wake-up per dependent, each processed asynchronously on its own delivery — now stated explicitly at the call site.
Process's terminal self-heal now re-fans dependents for Failed batches too, not just Cancelled ones: a batch failed by finalize (no viable path left) never passes through mergesignal, so speculate's redelivery self-heal is the only stage that can re-notify its dependents if the fan-out publish was ever lost.
The buildRunners dependency and the tree-driven cancelBatch/cancelBuilds/cancelTree replacement are deliberately left out of this commit — reconcile() here never talks to a build runner by design. That lands in the next commit in the stack, which is also the one place a direct runner cancel remains justified (batch-level cancel, not path-level).
✅ `go build ./...` and `go vet ./...` — clean (pre-existing vet warnings in mergeconflictsignal/mergesignal test files are unrelated to this change).
✅ `bazel test //submitqueue/... //service/...` — 56/56 targets pass; new reconcile/dead-path/finalize/pure-function tests pass alongside the preserved legacy cancel tests, and the merge-gate tests pin that terminal paths cost no path->build reads.
✅ `make gazelle && make fmt` — no changes.
✅ `make e2e-test` — stovepipe and submitqueue e2e suites both pass.
Follow-up tracked in #369 (tighten pathDead's Cancelled-base-dependency leniency once multi-path enumeration lands).1 parent 6208e14 commit 86bfae8
3 files changed
Lines changed: 749 additions & 243 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
33 | 40 | | |
34 | 41 | | |
35 | 42 | | |
| |||
0 commit comments