Skip to content

Commit caeb62b

Browse files
committed
feat(speculation): wire predictor into speculation pipeline
## Summary ### Why? The predictor implementation is additive until the speculation pipeline supplies each dependency's run-local path evidence and uses the revised probability for ranking. ### What? Thread path sets through the Generator contract and standard Speculator, replace `bestfirst`'s scorer dependency with the predictor, and compose the evidence predictor from per-queue YAML configuration in orchestrator profiles. Neutral default factors preserve scorer-only ranking when no factors are configured. ## Test Plan - `bazel test //submitqueue/extension/speculation/generator/... //submitqueue/extension/speculation/speculator/... //service/submitqueue/orchestrator/server:go_default_test` - `make check-gazelle`
1 parent e3dbe23 commit caeb62b

13 files changed

Lines changed: 443 additions & 124 deletions

File tree

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ go_library(
5252
"//submitqueue/extension/conflict/pathoverlap:go_default_library",
5353
"//submitqueue/extension/speculation/allocator/sticky:go_default_library",
5454
"//submitqueue/extension/speculation/generator/bestfirst:go_default_library",
55+
"//submitqueue/extension/speculation/predictor:go_default_library",
56+
"//submitqueue/extension/speculation/predictor/evidence:go_default_library",
5557
"//submitqueue/extension/speculation/scorer:go_default_library",
5658
"//submitqueue/extension/speculation/scorer/composite:go_default_library",
5759
"//submitqueue/extension/speculation/scorer/fake:go_default_library",
@@ -121,6 +123,7 @@ go_test(
121123
"//submitqueue/extension/buildrunner:go_default_library",
122124
"//submitqueue/extension/changeprovider:go_default_library",
123125
"//submitqueue/extension/conflict:go_default_library",
126+
"//submitqueue/extension/speculation/predictor:go_default_library",
124127
"//submitqueue/extension/speculation/scorer:go_default_library",
125128
"//submitqueue/extension/speculation/speculator:go_default_library",
126129
"//submitqueue/extension/storage:go_default_library",

service/submitqueue/orchestrator/server/config.go

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,22 @@ const (
6767
// Ways a composite scorer combines its components.
6868
const combineAvg = "avg"
6969

70+
// Predictor types selectable from configuration.
71+
const predictorTypeEvidence = "evidence"
72+
73+
// Evidence an evidence predictor prices, as named in configuration. The set is
74+
// closed: a factor under any other name would be applied to nothing and never
75+
// noticed.
76+
const (
77+
factorPathPassed = "pathPassed"
78+
factorPathFailed = "pathFailed"
79+
factorMerging = "merging"
80+
factorCancelling = "cancelling"
81+
)
82+
83+
// neutralFactor leaves the scorer's price untouched: odds multiplied by one.
84+
const neutralFactor = 1.0
85+
7086
// defaultBuildBudget is how many builds a queue may have occupying CI at once
7187
// when it states no budget of its own. Four is enough for speculation to be
7288
// visible — a queue that can only build one path never speculates — while
@@ -110,6 +126,7 @@ type namedQueueProfileConfig struct {
110126
Analyzer *analyzerConfig `yaml:"analyzer"`
111127
Scorer *scorerConfig `yaml:"scorer"`
112128
Speculator *speculatorConfig `yaml:"speculator"`
129+
Predictor *predictorConfig `yaml:"predictor"`
113130
}
114131

115132
// queueProfileConfig is the full set of extensions a queue resolves to.
@@ -119,6 +136,7 @@ type queueProfileConfig struct {
119136
Analyzer analyzerConfig `yaml:"analyzer"`
120137
Scorer scorerConfig `yaml:"scorer"`
121138
Speculator speculatorConfig `yaml:"speculator"`
139+
Predictor predictorConfig `yaml:"predictor"`
122140
}
123141

124142
// changeProviderConfig selects how change metadata is fetched. The github and
@@ -239,6 +257,19 @@ type speculatorConfig struct {
239257
BuildBudget int `yaml:"buildBudget"`
240258
}
241259

260+
// predictorConfig tunes how a queue turns its scorer's price into the
261+
// probability the generator ranks on. The scorer being revised is the queue's
262+
// own, so it is not named again here.
263+
type predictorConfig struct {
264+
Type string `yaml:"type"`
265+
// Factors multiply the odds of the scorer's price, one per piece of
266+
// evidence, keyed by evidence name. An omitted factor is neutral, so an
267+
// omitted block ranks on the scorer's price alone. Values are hand-set
268+
// placeholders, not measured: they are uncalibrated until the fitting work
269+
// in doc/rfc/submitqueue/outcome-predictor.md lands.
270+
Factors map[string]float64 `yaml:"factors"`
271+
}
272+
242273
// loadProfilesConfig reads and validates the profiles configuration at path.
243274
func loadProfilesConfig(path string) (profilesConfig, error) {
244275
data, err := os.ReadFile(path)
@@ -300,6 +331,11 @@ func (c *profilesConfig) normalizeAndValidate() error {
300331
return err
301332
}
302333
}
334+
if q.Predictor != nil {
335+
if err := q.Predictor.normalizeAndValidate(where); err != nil {
336+
return err
337+
}
338+
}
303339
}
304340
return c.validateGitRepoPaths()
305341
}
@@ -358,6 +394,9 @@ func (c profilesConfig) resolve(q namedQueueProfileConfig) queueProfileConfig {
358394
if q.Speculator != nil {
359395
profile.Speculator = *q.Speculator
360396
}
397+
if q.Predictor != nil {
398+
profile.Predictor = *q.Predictor
399+
}
361400
return profile
362401
}
363402

@@ -374,7 +413,10 @@ func (p *queueProfileConfig) normalizeAndValidate(where string) error {
374413
if err := p.Scorer.normalizeAndValidate(where); err != nil {
375414
return err
376415
}
377-
return p.Speculator.normalizeAndValidate(where)
416+
if err := p.Speculator.normalizeAndValidate(where); err != nil {
417+
return err
418+
}
419+
return p.Predictor.normalizeAndValidate(where)
378420
}
379421

380422
func (c *changeProviderConfig) normalizeAndValidate(where string) error {
@@ -556,6 +598,31 @@ func (s *scorerConfig) normalizeAndValidate(where string) error {
556598
return nil
557599
}
558600

601+
// normalizeAndValidate applies defaults and rejects a predictor that could not
602+
// be built. An empty block is an evidence predictor with every factor neutral,
603+
// which prices a batch at exactly its scorer's price.
604+
func (p *predictorConfig) normalizeAndValidate(where string) error {
605+
if p.Type == "" {
606+
p.Type = predictorTypeEvidence
607+
}
608+
if p.Type != predictorTypeEvidence {
609+
return fmt.Errorf("%s: unknown predictor type %q", where, p.Type)
610+
}
611+
for name, factor := range p.Factors {
612+
switch name {
613+
case factorPathPassed, factorPathFailed, factorMerging, factorCancelling:
614+
default:
615+
return fmt.Errorf("%s: unknown predictor factor %q", where, name)
616+
}
617+
// Zero would pin every batch carrying the evidence to a probability of
618+
// zero, and a negative multiplier on odds means nothing at all.
619+
if factor <= 0 {
620+
return fmt.Errorf("%s: predictor factor %q is %v, must be positive", where, name, factor)
621+
}
622+
}
623+
return nil
624+
}
625+
559626
func (s *speculatorConfig) normalizeAndValidate(where string) error {
560627
// A negative budget is rejected rather than clamped: sticky would compute no
561628
// free slots from it, so the queue would batch and then never build anything,

service/submitqueue/orchestrator/server/config_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,3 +666,49 @@ func TestLoadProfilesConfig_RejectsBadScorers(t *testing.T) {
666666
})
667667
}
668668
}
669+
670+
func TestLoadProfilesConfig_RejectsBadPredictors(t *testing.T) {
671+
tests := []struct {
672+
name string
673+
contents string
674+
}{
675+
{name: "unknown predictor type", contents: "defaults:\n predictor: {type: vibes}\n"},
676+
{name: "unknown factor", contents: "defaults:\n predictor:\n factors: {pathPased: 2}\n"},
677+
{name: "zero factor", contents: "defaults:\n predictor:\n factors: {merging: 0}\n"},
678+
{name: "negative factor", contents: "defaults:\n predictor:\n factors: {pathFailed: -1}\n"},
679+
{name: "bad factor on a queue override", contents: "defaults: {}\nqueues:\n - name: q\n predictor:\n factors: {merging: 0}\n"},
680+
}
681+
for _, tt := range tests {
682+
t.Run(tt.name, func(t *testing.T) {
683+
_, err := loadProfilesConfig(writeProfiles(t, tt.contents))
684+
require.Error(t, err)
685+
})
686+
}
687+
}
688+
689+
// An omitted predictor block leaves the queue ranking on its scorer's price
690+
// alone, which is what every queue does until someone states a factor.
691+
func TestLoadProfilesConfig_DefaultsThePredictorToNeutral(t *testing.T) {
692+
cfg, err := loadProfilesConfig(writeProfiles(t, "defaults: {}\nqueues:\n - name: q\n"))
693+
require.NoError(t, err)
694+
695+
assert.Equal(t, predictorTypeEvidence, cfg.Defaults.Predictor.Type)
696+
697+
factors := factorsFrom(cfg.resolve(cfg.Queues[0]).Predictor)
698+
assert.Equal(t, neutralFactor, factors.PathPassed)
699+
assert.Equal(t, neutralFactor, factors.PathFailed)
700+
assert.Equal(t, neutralFactor, factors.Merging)
701+
assert.Equal(t, neutralFactor, factors.Cancelling)
702+
}
703+
704+
func TestLoadProfilesConfig_ReadsPredictorFactors(t *testing.T) {
705+
cfg, err := loadProfilesConfig(writeProfiles(t,
706+
"defaults:\n predictor:\n factors: {pathPassed: 10, pathFailed: 0.3, merging: 12, cancelling: 0.1}\n"))
707+
require.NoError(t, err)
708+
709+
factors := factorsFrom(cfg.Defaults.Predictor)
710+
assert.Equal(t, 10.0, factors.PathPassed)
711+
assert.Equal(t, 0.3, factors.PathFailed)
712+
assert.Equal(t, 12.0, factors.Merging)
713+
assert.Equal(t, 0.1, factors.Cancelling)
714+
}

service/submitqueue/orchestrator/server/profiles.go

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ import (
4747
"github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap"
4848
"github.com/uber/submitqueue/submitqueue/extension/speculation/allocator/sticky"
4949
"github.com/uber/submitqueue/submitqueue/extension/speculation/generator/bestfirst"
50+
"github.com/uber/submitqueue/submitqueue/extension/speculation/predictor"
51+
"github.com/uber/submitqueue/submitqueue/extension/speculation/predictor/evidence"
5052
"github.com/uber/submitqueue/submitqueue/extension/speculation/scorer"
5153
"github.com/uber/submitqueue/submitqueue/extension/speculation/scorer/composite"
5254
scorerfake "github.com/uber/submitqueue/submitqueue/extension/speculation/scorer/fake"
@@ -80,6 +82,10 @@ type Profile struct {
8082
// likely their assumptions are to hold.
8183
Scorer scorer.Factory
8284

85+
// Predictor turns this queue's scorer price into the probability the
86+
// generator ranks on, revising it with the batch's observed progress.
87+
Predictor predictor.Factory
88+
8389
// Speculator decides which of this queue's speculation paths to build and
8490
// which running ones to preempt, within the build budget.
8591
Speculator speculator.Factory
@@ -142,6 +148,14 @@ func (p Profiles) ScorerFactory() scorer.Factory {
142148
})
143149
}
144150

151+
// PredictorFactory returns a predictor.Factory that resolves the
152+
// Predictor for each queue from the profile registry.
153+
func (p Profiles) PredictorFactory() predictor.Factory {
154+
return predictorFunc(func(c predictor.Config) (predictor.Predictor, error) {
155+
return p.For(c.QueueName).Predictor.For(c)
156+
})
157+
}
158+
145159
// StorageFactory returns a storage.Factory that routes each queue to its
146160
// profile's storage backend before binding the queue-scoped store aggregate.
147161
func (p Profiles) StorageFactory() storage.Factory {
@@ -176,6 +190,10 @@ type scorerFunc func(scorer.Config) (scorer.Scorer, error)
176190

177191
func (f scorerFunc) For(c scorer.Config) (scorer.Scorer, error) { return f(c) }
178192

193+
type predictorFunc func(predictor.Config) (predictor.Predictor, error)
194+
195+
func (f predictorFunc) For(c predictor.Config) (predictor.Predictor, error) { return f(c) }
196+
179197
type speculatorFunc func(speculator.Config) (speculator.Speculator, error)
180198

181199
func (f speculatorFunc) For(c speculator.Config) (speculator.Speculator, error) { return f(c) }
@@ -265,33 +283,71 @@ func (b *profileBuilder) build(cfg queueProfileConfig, where string) (Profile, e
265283
if err != nil {
266284
return Profile{}, err
267285
}
268-
// The speculator is composed last, because it is built from whatever scorer
269-
// the profile ended up with.
270-
return withSpeculator(Profile{
286+
// The predictor and the speculator are composed last, because each is built
287+
// from what the profile ended up with one level below it.
288+
return withSpeculator(withPredictor(Profile{
271289
ChangeProvider: provider,
272290
BuildRunner: runner,
273291
Analyzer: analyzer,
274292
Storage: b.stores,
275293
Scorer: sc,
276-
}, cfg.Speculator.BuildBudget), nil
294+
}, cfg.Predictor, b.scope), cfg.Speculator.BuildBudget), nil
295+
}
296+
297+
// withPredictor returns the profile with its predictor composed over its own
298+
// scorer: the scorer prices the batch's change, and the predictor revises that
299+
// price with what the batch's builds have done.
300+
//
301+
// The scorer is resolved lazily, at the queue the predictor itself was asked
302+
// for, so the queue's identity reaches one level down into the scorer too.
303+
func withPredictor(p Profile, cfg predictorConfig, scope tally.Scope) Profile {
304+
p.Predictor = predictorFunc(func(c predictor.Config) (predictor.Predictor, error) {
305+
sc, err := p.Scorer.For(scorer.Config{QueueName: c.QueueName})
306+
if err != nil {
307+
return nil, fmt.Errorf("failed to resolve scorer for queue %q: %w", c.QueueName, err)
308+
}
309+
return evidence.New(c, sc, factorsFrom(cfg), scope.SubScope("predictor"))
310+
})
311+
return p
312+
}
313+
314+
// factorsFrom reads the configured factors onto the named fields the predictor
315+
// takes, leaving an unstated one neutral. Names are validated when the config
316+
// is loaded.
317+
func factorsFrom(cfg predictorConfig) evidence.Factors {
318+
factors := evidence.AllOnes()
319+
for name, factor := range cfg.Factors {
320+
switch name {
321+
case factorPathPassed:
322+
factors.PathPassed = factor
323+
case factorPathFailed:
324+
factors.PathFailed = factor
325+
case factorMerging:
326+
factors.Merging = factor
327+
case factorCancelling:
328+
factors.Cancelling = factor
329+
}
330+
}
331+
return factors
277332
}
278333

279334
// withSpeculator returns the profile with its speculator composed from its own
280-
// scorer: bestfirst ranks a queue's candidate paths by how likely all their
335+
// predictor: bestfirst ranks a queue's candidate paths by how likely all their
281336
// assumptions are to hold, and sticky spends buildBudget down that ranking
282337
// without preempting builds already running. Swapping either part changes the
283338
// policy without touching the speculate controller, which depends only on the
284339
// Speculator contract.
285340
//
286-
// The scorer is resolved lazily, at the queue the speculator itself was asked
287-
// for, so the queue's identity reaches one level down into the scorer too.
341+
// The predictor is resolved lazily, at the queue the speculator itself was
342+
// asked for, so the queue's identity reaches down through the predictor to the
343+
// scorer under it.
288344
func withSpeculator(p Profile, buildBudget int) Profile {
289345
p.Speculator = speculatorFunc(func(c speculator.Config) (speculator.Speculator, error) {
290-
sc, err := p.Scorer.For(scorer.Config{QueueName: c.QueueName})
346+
pred, err := p.Predictor.For(predictor.Config{QueueName: c.QueueName})
291347
if err != nil {
292-
return nil, fmt.Errorf("failed to resolve scorer for queue %q: %w", c.QueueName, err)
348+
return nil, fmt.Errorf("failed to resolve predictor for queue %q: %w", c.QueueName, err)
293349
}
294-
return specstandard.New(c, bestfirst.New(sc), sticky.New(buildBudget)), nil
350+
return specstandard.New(c, bestfirst.New(pred), sticky.New(buildBudget)), nil
295351
})
296352
return p
297353
}

0 commit comments

Comments
 (0)