Skip to content

Commit fd31568

Browse files
committed
feat(orchestrator): re-plan the queue from the Speculator each run
## Summary ### Why? The speculate controller had no speculation in it: it advanced one batch at a time along a single hard-coded chain — every dependency assumed to pass — and nothing ever called the Speculator, the Generator, or the Allocator that had been built for it. This wires that machinery into the pipeline: a queue's paths are chosen by a swappable extension within a build budget, and each is built against only the dependencies it assumes will succeed. ### What? Every message is a dirty signal naming a batch; the controller re-plans that batch's whole queue from a single read: read the state, cancel paths whose assumptions a finished dependency has proven wrong, ask the Speculator, filter its proposals, dispatch what survives. Nothing carries over between runs, so duplicated or reordered signals are harmless and a later run repairs whatever an earlier one left half-done. `doc.go` explains the model in plain terms — no vocabulary section, every term is defined where it is used. The path set keeps exactly one writer — this run. The build stages record what CI did on per-build records; the run folds those into the set and alone decides each path's status. Pending paths are re-dispatched every run until their build is seen running, dispatches partition by batch so heads proceed in parallel, and cancelling paths need no dispatch at all: the poll loop reads the stop off the set and enacts it. Speculation lands inert. The wiring layer passes a placeholder Speculator that proposes nothing, so the run executes end to end but funds no paths; composing real per-queue speculators and turning the feature on is the wiring change at the top of this stack. Batch outcomes still come from the legacy per-batch finalizer, which waits on every dependency — strictly stricter than path-aware finalization, so the system stays correct until the next commit replaces it. ## Test Plan ✅ `bazel test //submitqueue/orchestrator/controller/speculate/...` — assumption checks and proposal filtering are table-driven; run tests cover funding a first path, re-dispatching pending paths, broken-path cancellation, build results recorded onto paths, lost CAS races skipped rather than failed, and Speculator errors abandoning the run. ✅ `make fmt`, `make gazelle`
1 parent d51250d commit fd31568

15 files changed

Lines changed: 1895 additions & 85 deletions

File tree

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ go_library(
2525
"//platform/http:go_default_library",
2626
"//platform/pipeline:go_default_library",
2727
"//submitqueue/core/changeset:go_default_library",
28+
"//submitqueue/entity:go_default_library",
2829
"//submitqueue/extension/buildrunner:go_default_library",
2930
"//submitqueue/extension/buildrunner/fake:go_default_library",
3031
"//submitqueue/extension/changeprovider:go_default_library",
@@ -37,6 +38,7 @@ go_library(
3738
"//submitqueue/extension/conflict/fake:go_default_library",
3839
"//submitqueue/extension/conflict/fileoverlap:go_default_library",
3940
"//submitqueue/extension/conflict/none:go_default_library",
41+
"//submitqueue/extension/speculation/speculator:go_default_library",
4042
"//submitqueue/extension/storage/mysql:go_default_library",
4143
"//submitqueue/extension/validator/fake:go_default_library",
4244
"//submitqueue/orchestrator:go_default_library",

service/submitqueue/orchestrator/server/main.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,13 @@ import (
4242
"github.com/uber/submitqueue/platform/http"
4343
"github.com/uber/submitqueue/platform/pipeline"
4444
"github.com/uber/submitqueue/submitqueue/core/changeset"
45+
"github.com/uber/submitqueue/submitqueue/entity"
4546
"github.com/uber/submitqueue/submitqueue/extension/changeprovider"
4647
cpfake "github.com/uber/submitqueue/submitqueue/extension/changeprovider/fake"
4748
githubprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/github"
4849
phabprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/phabricator"
4950
routingprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/routing"
51+
"github.com/uber/submitqueue/submitqueue/extension/speculation/speculator"
5052
mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql"
5153
validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake"
5254
"github.com/uber/submitqueue/submitqueue/orchestrator"
@@ -196,7 +198,12 @@ func run() error {
196198
BuildRunner: profiles.BuildRunnerFactory(),
197199
ChangeProvider: profiles.ChangeProviderFactory(),
198200
Analyzer: profiles.AnalyzerFactory(),
199-
Validator: validatorfake.NewFactory(),
201+
// Speculation is wired but inert: the placeholder below proposes
202+
// nothing, so no path is ever funded and no speculative build starts.
203+
// The wiring change at the top of this stack replaces it with real
204+
// per-queue speculators composed from each profile's scorer.
205+
Speculator: noopSpeculators{},
206+
Validator: validatorfake.NewFactory(),
200207
}
201208

202209
// Assemble the pipeline: one call builds the topic registry, creates
@@ -431,3 +438,20 @@ func parseTimeout(envVal string, defaultVal time.Duration) time.Duration {
431438
}
432439
return defaultVal
433440
}
441+
442+
// noopSpeculators resolves every queue to a speculator that proposes nothing.
443+
// It keeps the speculate stage inert — no path funded, no build started —
444+
// until per-queue speculators are composed in the profiles.
445+
type noopSpeculators struct{}
446+
447+
// For returns the propose-nothing speculator for any queue.
448+
func (noopSpeculators) For(speculator.Config) (speculator.Speculator, error) {
449+
return noopSpeculator{}, nil
450+
}
451+
452+
type noopSpeculator struct{}
453+
454+
// Speculate proposes no actions, whatever the queue looks like.
455+
func (noopSpeculator) Speculate(context.Context, []entity.Batch, []entity.SpeculationPathSet) ([]entity.Speculation, error) {
456+
return nil, nil
457+
}

submitqueue/orchestrator/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ go_library(
1414
"//submitqueue/extension/buildrunner:go_default_library",
1515
"//submitqueue/extension/changeprovider:go_default_library",
1616
"//submitqueue/extension/conflict:go_default_library",
17+
"//submitqueue/extension/speculation/speculator:go_default_library",
1718
"//submitqueue/extension/storage:go_default_library",
1819
"//submitqueue/extension/validator:go_default_library",
1920
"//submitqueue/orchestrator/controller:go_default_library",

submitqueue/orchestrator/controller/speculate/BUILD.bazel

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,23 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["speculate.go"],
5+
srcs = [
6+
"check.go",
7+
"dispatch.go",
8+
"doc.go",
9+
"run.go",
10+
"snapshot.go",
11+
"speculate.go",
12+
],
613
importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/speculate",
714
visibility = ["//visibility:public"],
815
deps = [
9-
"//platform/base/messagequeue:go_default_library",
1016
"//platform/consumer:go_default_library",
1117
"//platform/metrics:go_default_library",
18+
"//submitqueue/core/publish:go_default_library",
1219
"//submitqueue/core/topickey:go_default_library",
1320
"//submitqueue/entity:go_default_library",
21+
"//submitqueue/extension/speculation/speculator:go_default_library",
1422
"//submitqueue/extension/storage:go_default_library",
1523
"@com_github_uber_go_tally//:go_default_library",
1624
"@org_uber_go_zap//:go_default_library",
@@ -19,7 +27,12 @@ go_library(
1927

2028
go_test(
2129
name = "go_default_test",
22-
srcs = ["speculate_test.go"],
30+
srcs = [
31+
"check_test.go",
32+
"run_test.go",
33+
"snapshot_test.go",
34+
"speculate_test.go",
35+
],
2336
embed = [":go_default_library"],
2437
deps = [
2538
"//platform/base/messagequeue:go_default_library",
@@ -29,6 +42,7 @@ go_test(
2942
"//platform/extension/messagequeue/mock:go_default_library",
3043
"//submitqueue/core/topickey:go_default_library",
3144
"//submitqueue/entity:go_default_library",
45+
"//submitqueue/extension/speculation/speculator:go_default_library",
3246
"//submitqueue/extension/storage:go_default_library",
3347
"//submitqueue/extension/storage/mock:go_default_library",
3448
"@com_github_stretchr_testify//assert:go_default_library",
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package speculate
16+
17+
import "github.com/uber/submitqueue/submitqueue/entity"
18+
19+
// rejection is why one proposed action was dropped. The reasons are the metric
20+
// dimension for a misbehaving Speculator, so each names a distinct fault.
21+
type rejection string
22+
23+
const (
24+
// rejectUnknownAction is a zero-value or unrecognized action.
25+
rejectUnknownAction rejection = "unknown_action"
26+
// rejectUnknownHead names a batch this run never read.
27+
rejectUnknownHead rejection = "unknown_head"
28+
// rejectHeadNotSpeculating targets a batch that is not open to new work.
29+
rejectHeadNotSpeculating rejection = "head_not_speculating"
30+
// rejectMalformedPath is a path whose assumptions do not line up with its
31+
// head's dependency list: one missing or extra, a duplicate, a wrong head,
32+
// or a made-up assumption value.
33+
rejectMalformedPath rejection = "malformed_path"
34+
// rejectBrokenAssumption is a path with an assumption a finished
35+
// dependency has already proven wrong.
36+
rejectBrokenAssumption rejection = "broken_assumption"
37+
// rejectPathTerminal would rebuild a path whose build already finished.
38+
rejectPathTerminal rejection = "path_terminal"
39+
// rejectCancelNotInFlight cancels a path that is not running.
40+
rejectCancelNotInFlight rejection = "cancel_not_in_flight"
41+
// rejectCancelPassed would throw away a build that already passed.
42+
rejectCancelPassed rejection = "cancel_passed"
43+
)
44+
45+
// filterProposals narrows a Speculator's proposals down to the ones the
46+
// controller is willing to enact, returning the survivors and a reason for
47+
// each drop.
48+
//
49+
// The Speculator is an extension, so its output is untrusted input: it decides
50+
// which paths run, never whether a batch merges or fails. Every rule here
51+
// protects an invariant the extension could otherwise break — acting on a batch
52+
// that is finalizing, resurrecting a path a resolved dependency has ruled out,
53+
// or discarding a passed build the queue is about to merge on. A proposal that
54+
// trips one of these is a bug in the Speculator, not a normal outcome, which is
55+
// why the caller counts them.
56+
func filterProposals(proposals []entity.Speculation, snap snapshot) ([]entity.Speculation, []rejection) {
57+
var kept []entity.Speculation
58+
var rejected []rejection
59+
60+
for _, proposal := range proposals {
61+
if reason, ok := rejectionReason(proposal, snap); ok {
62+
rejected = append(rejected, reason)
63+
continue
64+
}
65+
kept = append(kept, proposal)
66+
}
67+
68+
return kept, rejected
69+
}
70+
71+
// rejectionReason reports why a proposal cannot be enacted, or ok=false if
72+
// it can.
73+
func rejectionReason(proposal entity.Speculation, snap snapshot) (rejection, bool) {
74+
switch proposal.Action {
75+
case entity.PathActionBuild, entity.PathActionCancel:
76+
default:
77+
return rejectUnknownAction, true
78+
}
79+
80+
head, ok := snap.batches[proposal.Path.Head]
81+
if !ok {
82+
return rejectUnknownHead, true
83+
}
84+
// Only a speculating head is open to new work. Batches in every other
85+
// state are facts the Speculator may reason from, never action targets.
86+
if head.State != entity.BatchStateSpeculating {
87+
return rejectHeadNotSpeculating, true
88+
}
89+
if !isWellFormed(proposal.Path, head) {
90+
return rejectMalformedPath, true
91+
}
92+
if assumptionBroken(proposal.Path, snap) {
93+
return rejectBrokenAssumption, true
94+
}
95+
96+
entry, stored := findPath(snap.pathSets[head.ID], proposal.Path.ID())
97+
98+
if proposal.Action == entity.PathActionCancel {
99+
// Cancelling is only meaningful for a path that is actually running,
100+
// and never for one that passed: that build is the head's way out of
101+
// the queue, and the budget it holds is already spent.
102+
if !stored {
103+
return rejectCancelNotInFlight, true
104+
}
105+
if entry.Status == entity.SpeculationPathStatusPassed {
106+
return rejectCancelPassed, true
107+
}
108+
if entry.Status.IsTerminal() {
109+
return rejectCancelNotInFlight, true
110+
}
111+
return "", false
112+
}
113+
114+
// A build proposal for a path whose build already finished would discard a
115+
// recorded result and start the same work again.
116+
if stored && entry.Status.IsTerminal() {
117+
return rejectPathTerminal, true
118+
}
119+
120+
return "", false
121+
}
122+
123+
// isWellFormed reports whether a path is a proper guess about its head:
124+
// exactly one assumption for each of the head's dependencies, no more and no
125+
// fewer, and every assumption a real value.
126+
//
127+
// A malformed path is not merely suboptimal, it is unmergeable — the merge
128+
// preconditions are read off the path's assumptions, so a path missing a
129+
// dependency would let its head merge without waiting for it.
130+
func isWellFormed(path entity.SpeculationPath, head entity.Batch) bool {
131+
if path.Head != head.ID {
132+
return false
133+
}
134+
if len(path.Dependencies) != len(head.Dependencies) {
135+
return false
136+
}
137+
138+
required := make(map[string]struct{}, len(head.Dependencies))
139+
for _, dep := range head.Dependencies {
140+
required[dep] = struct{}{}
141+
}
142+
143+
for _, dep := range path.Dependencies {
144+
if _, ok := required[dep.Batch]; !ok {
145+
return false
146+
}
147+
delete(required, dep.Batch)
148+
149+
switch dep.Assumption {
150+
case entity.DependencyAssumptionSucceeds,
151+
entity.DependencyAssumptionFails,
152+
entity.DependencyAssumptionIgnored:
153+
default:
154+
return false
155+
}
156+
}
157+
158+
return len(required) == 0
159+
}
160+
161+
// findPath returns the entry for a path ID in the set.
162+
func findPath(set entity.SpeculationPathSet, pathID string) (entity.SpeculationPathEntry, bool) {
163+
for _, entry := range set.Paths {
164+
if entry.ID == pathID {
165+
return entry, true
166+
}
167+
}
168+
return entity.SpeculationPathEntry{}, false
169+
}

0 commit comments

Comments
 (0)