Skip to content

Commit 00a94c1

Browse files
committed
feat(orchestrator): compose per-queue speculators and turn speculation on
## Summary ### Why? Everything below this commit landed inert: the orchestrator passed the speculate run a placeholder that proposes nothing, so no path was ever funded and no speculative build started. The machinery is all reviewed; nothing configures it per queue or switches it on. ### What? This is the activation switch. Per-queue profiles gain a `Scorer` and a `Speculator`: each queue's speculator is composed from its own scorer as `standard.New(bestfirst.New(scorer), sticky.New(budget))`, with a factory adapter beside the existing ones so routing stays in the wiring layer, and `main.go` swaps the placeholder for `profiles.SpeculatorFactory()`. The scorer profiles are the policy knobs: the baseline scores everything 0.5, test-queue buckets by lines changed, and e2e-test-queue exercises the composite scorer. Every scorer is wrapped by `scorerfake` so a change URI carrying a failure marker forces a scoring error end-to-end. The build budget is a wiring-level constant (4 concurrent builds per queue) with a TODO to move it onto `entity.QueueConfig`. With this commit, paths are funded, builds run per path, and batches finalize from their paths — the whole stack goes live in one revertable step. ## Test Plan ✅ `bazel build //...`, `make fmt`, `make gazelle` ✅ `make e2e-test` — the full pipeline lands changes end-to-end with per-path speculation on.
1 parent 26ae2d9 commit 00a94c1

3 files changed

Lines changed: 109 additions & 39 deletions

File tree

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@ go_library(
3838
"//submitqueue/extension/conflict/fake:go_default_library",
3939
"//submitqueue/extension/conflict/fileoverlap:go_default_library",
4040
"//submitqueue/extension/conflict/none:go_default_library",
41+
"//submitqueue/extension/scorer:go_default_library",
42+
"//submitqueue/extension/scorer/composite:go_default_library",
43+
"//submitqueue/extension/scorer/fake:go_default_library",
44+
"//submitqueue/extension/scorer/heuristic:go_default_library",
45+
"//submitqueue/extension/speculation/allocator/sticky:go_default_library",
46+
"//submitqueue/extension/speculation/generator/bestfirst:go_default_library",
4147
"//submitqueue/extension/speculation/speculator:go_default_library",
48+
"//submitqueue/extension/speculation/speculator/standard:go_default_library",
4249
"//submitqueue/extension/storage/mysql:go_default_library",
4350
"//submitqueue/extension/validator/fake:go_default_library",
4451
"//submitqueue/orchestrator:go_default_library",

service/submitqueue/orchestrator/server/main.go

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,11 @@ 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"
4645
"github.com/uber/submitqueue/submitqueue/extension/changeprovider"
4746
cpfake "github.com/uber/submitqueue/submitqueue/extension/changeprovider/fake"
4847
githubprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/github"
4948
phabprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/phabricator"
5049
routingprovider "github.com/uber/submitqueue/submitqueue/extension/changeprovider/routing"
51-
"github.com/uber/submitqueue/submitqueue/extension/speculation/speculator"
5250
mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql"
5351
validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake"
5452
"github.com/uber/submitqueue/submitqueue/orchestrator"
@@ -198,12 +196,8 @@ func run() error {
198196
BuildRunner: profiles.BuildRunnerFactory(),
199197
ChangeProvider: profiles.ChangeProviderFactory(),
200198
Analyzer: profiles.AnalyzerFactory(),
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(),
199+
Speculator: profiles.SpeculatorFactory(),
200+
Validator: validatorfake.NewFactory(),
207201
}
208202

209203
// Assemble the pipeline: one call builds the topic registry, creates
@@ -438,20 +432,3 @@ func parseTimeout(envVal string, defaultVal time.Duration) time.Duration {
438432
}
439433
return defaultVal
440434
}
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-
}

service/submitqueue/orchestrator/server/profiles.go

Lines changed: 100 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
package main
1616

1717
import (
18+
"context"
1819
"fmt"
1920

2021
"github.com/uber-go/tally"
2122
"github.com/uber/submitqueue/submitqueue/core/changeset"
23+
"github.com/uber/submitqueue/submitqueue/entity"
2224
"github.com/uber/submitqueue/submitqueue/extension/buildrunner"
2325
buildfake "github.com/uber/submitqueue/submitqueue/extension/buildrunner/fake"
2426
"github.com/uber/submitqueue/submitqueue/extension/changeprovider"
@@ -27,6 +29,14 @@ import (
2729
conflictfake "github.com/uber/submitqueue/submitqueue/extension/conflict/fake"
2830
"github.com/uber/submitqueue/submitqueue/extension/conflict/fileoverlap"
2931
"github.com/uber/submitqueue/submitqueue/extension/conflict/none"
32+
"github.com/uber/submitqueue/submitqueue/extension/scorer"
33+
"github.com/uber/submitqueue/submitqueue/extension/scorer/composite"
34+
scorerfake "github.com/uber/submitqueue/submitqueue/extension/scorer/fake"
35+
"github.com/uber/submitqueue/submitqueue/extension/scorer/heuristic"
36+
"github.com/uber/submitqueue/submitqueue/extension/speculation/allocator/sticky"
37+
"github.com/uber/submitqueue/submitqueue/extension/speculation/generator/bestfirst"
38+
"github.com/uber/submitqueue/submitqueue/extension/speculation/speculator"
39+
specstandard "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator/standard"
3040
"go.uber.org/zap"
3141
)
3242

@@ -43,6 +53,15 @@ type Profile struct {
4353

4454
// Analyzer detects conflicts between concurrent batches in this queue.
4555
Analyzer conflict.Analyzer
56+
57+
// Scorer holds this queue's scoring profile. There is no scoring stage: the
58+
// scorer feeds the queue's speculator, which ranks candidate paths by how
59+
// likely their assumptions are to hold.
60+
Scorer scorer.Scorer
61+
62+
// Speculator decides which of this queue's speculation paths to build and
63+
// which running ones to preempt, within the build budget.
64+
Speculator speculator.Speculator
4665
}
4766

4867
// Profiles maps a queue name to its extension Profile, falling back to a
@@ -86,6 +105,14 @@ func (p Profiles) AnalyzerFactory() conflict.Factory {
86105
})
87106
}
88107

108+
// SpeculatorFactory returns a speculator.Factory that resolves the Speculator
109+
// for each queue from the profile registry.
110+
func (p Profiles) SpeculatorFactory() speculator.Factory {
111+
return speculatorFunc(func(c speculator.Config) (speculator.Speculator, error) {
112+
return p.For(c.QueueName).Speculator, nil
113+
})
114+
}
115+
89116
// Thin func-type adapters — the http.HandlerFunc trick applied to each
90117
// extension Factory interface. Each func type satisfies the Factory contract,
91118
// letting Profiles cross the host/library boundary without dedicated structs.
@@ -104,37 +131,68 @@ type analyzerFunc func(conflict.Config) (conflict.Analyzer, error)
104131

105132
func (f analyzerFunc) For(c conflict.Config) (conflict.Analyzer, error) { return f(c) }
106133

134+
type speculatorFunc func(speculator.Config) (speculator.Speculator, error)
135+
136+
func (f speculatorFunc) For(c speculator.Config) (speculator.Speculator, error) { return f(c) }
137+
107138
// newProfiles builds the per-queue extension profiles for the example.
108139
// Edge integrations (change provider) and the build runner form a shared
109140
// baseline; each per-queue profile starts from that baseline and overrides
110-
// only the extensions that differ — here the conflict analyzer.
141+
// only the extensions that differ — here the conflict analyzer and the scorer.
111142
// Queues without an explicit profile fall back to the baseline.
112143
func newProfiles(logger *zap.Logger, scope tally.Scope, resolver changeset.Resolver) (Profiles, error) {
113144
cp, err := newChangeProvider(logger, scope)
114145
if err != nil {
115146
return Profiles{}, fmt.Errorf("failed to create change provider: %w", err)
116147
}
117148

149+
// batchLines buckets a batch by total lines changed across all its changes —
150+
// larger batches are likelier to fail to land.
151+
batchLines := func(_ context.Context, changes entity.BatchChanges) (int, error) {
152+
return changes.TotalLinesChanged(), nil
153+
}
154+
118155
// Baseline profile: shared edge integrations + a fake build runner (every
119-
// build succeeds unless a head URI carries a failure marker). The build
120-
// runner instance is shared by the build and buildsignal controllers (same
121-
// profile, same instance) so a build's recorded outcome survives across
122-
// their separate factory lookups.
156+
// build succeeds unless a head URI carries a failure marker), plus permissive
157+
// defaults for scorer and conflict. The build runner instance is shared by
158+
// the build and buildsignal controllers (same profile, same instance) so a
159+
// build's recorded outcome survives across their separate factory lookups.
123160
//
124-
// The analyzer is wrapped by conflictfake with a nil predicate
125-
// (passthrough) — swap the predicate (e.g. conflictfake.FailAlways) on a
126-
// queue to exercise the analyzer error path, as e2e-conflict-error-queue
161+
// The scorer is wrapped by scorerfake so a change URI carrying
162+
// "sq-fake=score-error" forces a scoring error end-to-end; it is a pure
163+
// passthrough otherwise. The analyzer is wrapped by conflictfake with a nil
164+
// predicate (passthrough) — swap the predicate (e.g. conflictfake.FailAlways)
165+
// on a queue to exercise the analyzer error path, as e2e-conflict-error-queue
127166
// below does.
128167
base := Profile{
129168
ChangeProvider: cp,
130169
BuildRunner: buildfake.New(resolver),
131170
// TODO: replace the delegate with a real analyzer (e.g. Tango target
132171
// analysis). "all" serializes the queue conservatively.
133172
Analyzer: conflictfake.New(all.New(), nil),
173+
Scorer: scorerfake.New(resolver, heuristic.New(
174+
resolver,
175+
[]heuristic.Bucket{{Min: 0, Max: 1<<31 - 1, Score: 0.5}},
176+
batchLines, scope.SubScope("scorer.default"),
177+
)),
134178
}
135179

180+
// test-queue: bucketed heuristic scorer; conservative (serialized) conflicts
181+
// inherited from the baseline.
182+
testQueue := base
183+
testQueue.Scorer = scorerfake.New(resolver, heuristic.New(
184+
resolver,
185+
[]heuristic.Bucket{
186+
{Min: 0, Max: 1, Score: 0.95},
187+
{Min: 2, Max: 5, Score: 0.80},
188+
{Min: 6, Max: 20, Score: 0.60},
189+
{Min: 21, Max: 1<<31 - 1, Score: 0.40},
190+
},
191+
batchLines, scope.SubScope("scorer.test-queue"),
192+
))
193+
136194
// e2e-conflict-error-queue: every conflict analysis fails, exercising the
137-
// analyzer error path. Edge integrations inherit the baseline.
195+
// analyzer error path. Scorer/edge integrations inherit the baseline.
138196
conflictErrQueue := base
139197
conflictErrQueue.Analyzer = conflictfake.New(all.New(), conflictfake.FailAlways)
140198

@@ -143,16 +201,44 @@ func newProfiles(logger *zap.Logger, scope tally.Scope, resolver changeset.Resol
143201
fileOverlapQueue := base
144202
fileOverlapQueue.Analyzer = fileoverlap.New(resolver)
145203

146-
// e2e-test-queue: no conflicts (maximum parallelism).
204+
// e2e-test-queue: composite scorer; no conflicts (maximum parallelism).
147205
e2eQueue := base
148206
e2eQueue.Analyzer = conflictfake.New(none.New(), nil)
207+
e2eQueue.Scorer = scorerfake.New(resolver, composite.New(
208+
map[string]scorer.Scorer{
209+
"size": heuristic.New(resolver, []heuristic.Bucket{{Min: 0, Max: 1<<31 - 1, Score: 0.8}}, batchLines, scope),
210+
"flat": heuristic.New(resolver, []heuristic.Bucket{{Min: 0, Max: 1<<31 - 1, Score: 0.6}}, batchLines, scope),
211+
},
212+
composite.Avg, scope.SubScope("scorer.e2e-test-queue"),
213+
))
149214

215+
// The speculator is composed last, because it is built from whatever scorer
216+
// the profile ended up with.
150217
return Profiles{
151-
defaultProfile: base,
218+
defaultProfile: withSpeculator(base),
152219
byQueue: map[string]Profile{
153-
"e2e-test-queue": e2eQueue,
154-
"e2e-conflict-error-queue": conflictErrQueue,
155-
"file-overlap-queue": fileOverlapQueue,
220+
"test-queue": withSpeculator(testQueue),
221+
"e2e-test-queue": withSpeculator(e2eQueue),
222+
"e2e-conflict-error-queue": withSpeculator(conflictErrQueue),
223+
"file-overlap-queue": withSpeculator(fileOverlapQueue),
156224
},
157225
}, nil
158226
}
227+
228+
// defaultBuildBudget caps how many builds a queue may have occupying CI at
229+
// once. It is the only rationing lever the allocator has.
230+
//
231+
// TODO: move this onto entity.QueueConfig so operators can tune it per queue
232+
// without a code change. QueueConfig carries only the queue name today.
233+
const defaultBuildBudget = 4
234+
235+
// withSpeculator returns the profile with its speculator composed from its own
236+
// scorer: bestfirst ranks a queue's candidate paths by how likely all their
237+
// assumptions are to hold, and sticky spends the build budget down that ranking
238+
// without preempting builds already running. Swapping either part changes the
239+
// policy without touching the speculate controller, which depends only on the
240+
// Speculator contract.
241+
func withSpeculator(p Profile) Profile {
242+
p.Speculator = specstandard.New(bestfirst.New(p.Scorer), sticky.New(defaultBuildBudget))
243+
return p
244+
}

0 commit comments

Comments
 (0)