Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/rfc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting
- [Extension Contract](submitqueue/extension-contract.md) - When extensions take orchestrator identity (request/batch) and resolve granular content themselves vs. take controller-resolved data; revises the BuildRunner base/head contract
- [Gateway Status and List APIs](submitqueue/status-list-api.md) - Gateway-owned request context, materialized current status, sqid or change-URI status lookup, and queue admission listing
- [Speculation](submitqueue/speculation.md) - Why SubmitQueue speculates, the path/tree model, and the two pluggable seams: speculation-tree enumeration and path selection
- [Outcome Predictor](submitqueue/outcome-predictor.md) - How likely a batch is to reach Succeeded: the scorer prices the change, the predictor revises that price from the path set and batch state (`pathPassed`, `pathFailed`, `merging`, `cancelling`)
- [Best-First Speculation Path Generation](submitqueue/speculation-generator-best-first.md) - The default Generator: per-head lazy streams of flip subsets merged best-first across heads, log-probability ranking, and the strict snapshot contract
- [Modular Queue Wiring](submitqueue/modular-queue-wiring.md) - Declare-don't-assemble engine (`pipeline.Construct`) that unifies topic registry, controller registration, DLQ pairing, and lifecycle ordering into one typed call; services self-declare via Deps struct + Stages slice, hosts own per-queue profiles and transport

Expand Down
103 changes: 103 additions & 0 deletions doc/rfc/submitqueue/outcome-predictor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# Outcome Predictor

How likely a batch is to reach Succeeded, given the scorer's price for the change plus what this speculate run has already observed.

See [speculation.md](speculation.md) for batches, paths, heads, and the Speculator. This document is the price the default Generator ranks on.

## The idea

The **scorer** prices the change (lines, files, who wrote it). That number does not move after the batch is admitted.

The **predictor** prices the situation. It starts from the scorer's price and revises it with facts the speculate run already holds: a path *passed*, a path *failed*, the batch is *merging*, the batch is *cancelling*.

`bestfirst` ranks a path by the probability that every unresolved assumption holds. It now asks the predictor for that probability, not the scorer. Two heads whose changes score the same can rank differently once one of them has a *passed* build.

They are two contracts because they answer different questions. Putting path-set evidence on `Score` was tried: every content scorer took a parameter it discarded.

**Default is a no-op.** Every factor starts at `1`, so the predictor returns the scorer's price until someone sets a factor.

## What a factor is

A factor revises the scorer's price. It is not itself a probability: `10` does not mean `0.10`, and `0.3` does not mean the batch is 30% likely to succeed.

| Value | Meaning |
| --- | --- |
| `1` | Leave the scorer's price alone (the default if the key is omitted) |
| greater than `1` | More likely to reach Succeeded |
| between `0` and `1` | Less likely to reach Succeeded |

Config rejects `0` and negatives. There is no upper cap.

The unconfigured scorer prices every batch at `0.5`. From that price, one factor `f` produces:

| Factor | Price |
| --- | --- |
| `1` | 0.50 |
| `10` | ~0.91 |
| `12` | ~0.92 |
| `0.3` | ~0.23 |
| `0.25` | 0.20 |

A scorer price of `0.6` with `pathPassed: 10` becomes about `0.94`. `merging: 12` on top of that becomes about `0.995`.

`pathFailed: 0.3` from `0.5` becomes about `0.23`. A second *failed* path of the same kind multiplies again. `0` is rejected: it would pin the batch at probability 0 for the rest of the run.

The arithmetic multiplies odds (`p / (1-p)`), then converts back, so the result stays in `(0, 1)` and the same factor means the same thing at any scorer price. Adding to the probability does neither.

YAML:

```yaml
predictor:
type: evidence
factors:
pathPassed: 10
pathFailed: 0.3
merging: 12
cancelling: 0.1
```

The example values above are guesses, for reading the tables. The shipped default is to omit `factors` (every factor `1`).

## Evidence

| YAML key | When it applies | Typical direction |
| --- | --- | --- |
| `pathPassed` | Once, if a path that assumes every dependency *succeeds* has *passed* | Up |
| `pathFailed` | Once per *failed* path that assumes every dependency *succeeds* | Down |
| `merging` | While the batch is *merging* | Up |
| `cancelling` | While the batch is *cancelling* | Down |

`bestfirst` already treats a terminal batch as a fact (*Succeeded*, *Failed*, *Cancelled*). The predictor is not asked. *Merging* is not terminal: a merge can still fail, so how much it is worth stays a price.

### Only the *succeeds* path counts

The batch being priced is itself a head, so the run may have built it more than once under different assumptions about *its* dependencies. Only one of those builds is evidence.

Take `C` depending on `B`, and `B` depending on `A`. Ranking `C`'s candidates needs the probability that `B` reaches Succeeded, so the Generator calls `Predict` with `B` and `B`'s path set. That set can hold two finished builds:

| `B`'s path | What was compiled |
| --- | --- |
| `B` with `A` *succeeds* | `B` on top of `A`'s changes |
| `B` with `A` *fails* | `B` without them |

`B` merges after `A` does, so the first build is a build of the code that will actually land: if it *passed*, `B` is likely to merge, and `pathPassed` applies.

The second is a different set of changes. `B` may call something `A` introduces and fail to compile on its own — a *failed* result that says nothing about `B` merging in the normal case. Counting it would push `B` down the ranking over a build it was never going to need, while a green build of the real combination sits in the same set.

So `pathPassed` and `pathFailed` both look only at paths that assume every dependency *succeeds*. Results on any other path are skipped. This is a filter on which results are evidence, not a check on whether an assumption came true — nothing here revisits that.

## Rejected

**One contract, with the evidence on `Score`.** Tried: every scorer that prices content took a parameter it discarded, and the composite forwarded one it never read.

**One estimate over content and evidence together.** They change at different rates and need different amounts of data, and it would force every queue onto the same content scorer.

**Adding to the probability instead of multiplying odds.** Leaves the range, needs clamping, and the same increment means different things at different prices.

**More dimensions on the bucket table.** A second dimension squares it, a third makes it unwritable, and every cell is still a guess.

**Putting *merging* / *cancelling* in the Generator.** Tried and reverted: a merge can fail, so nothing is settled, and how much a state is worth is a price.

**A scoring stage.** Prices only mean anything inside the run that produced them; storing them would make them stale by construction.

**The predictor reads the path-set store.** Cheaper plumbing, stale or split-brain snapshot. The run reads once.
3 changes: 3 additions & 0 deletions service/submitqueue/orchestrator/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ go_library(
"//submitqueue/extension/conflict/pathoverlap:go_default_library",
"//submitqueue/extension/speculation/allocator/sticky:go_default_library",
"//submitqueue/extension/speculation/generator/bestfirst:go_default_library",
"//submitqueue/extension/speculation/predictor:go_default_library",
"//submitqueue/extension/speculation/predictor/evidence:go_default_library",
"//submitqueue/extension/speculation/scorer:go_default_library",
"//submitqueue/extension/speculation/scorer/composite:go_default_library",
"//submitqueue/extension/speculation/scorer/fake:go_default_library",
Expand Down Expand Up @@ -121,6 +123,7 @@ go_test(
"//submitqueue/extension/buildrunner:go_default_library",
"//submitqueue/extension/changeprovider:go_default_library",
"//submitqueue/extension/conflict:go_default_library",
"//submitqueue/extension/speculation/predictor:go_default_library",
"//submitqueue/extension/speculation/scorer:go_default_library",
"//submitqueue/extension/speculation/speculator:go_default_library",
"//submitqueue/extension/storage:go_default_library",
Expand Down
69 changes: 68 additions & 1 deletion service/submitqueue/orchestrator/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,22 @@ const (
// Ways a composite scorer combines its components.
const combineAvg = "avg"

// Predictor types selectable from configuration.
const predictorTypeEvidence = "evidence"

// Evidence an evidence predictor prices, as named in configuration. The set is
// closed: a factor under any other name would be applied to nothing and never
// noticed.
const (
factorPathPassed = "pathPassed"
factorPathFailed = "pathFailed"
factorMerging = "merging"
factorCancelling = "cancelling"
)

// neutralFactor leaves the scorer's price untouched: odds multiplied by one.
const neutralFactor = 1.0

// defaultBuildBudget is how many builds a queue may have occupying CI at once
// when it states no budget of its own. Four is enough for speculation to be
// visible — a queue that can only build one path never speculates — while
Expand Down Expand Up @@ -110,6 +126,7 @@ type namedQueueProfileConfig struct {
Analyzer *analyzerConfig `yaml:"analyzer"`
Scorer *scorerConfig `yaml:"scorer"`
Speculator *speculatorConfig `yaml:"speculator"`
Predictor *predictorConfig `yaml:"predictor"`
}

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

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

// predictorConfig tunes how a queue turns its scorer's price into the
// probability the generator ranks on. The scorer being revised is the queue's
// own, so it is not named again here.
type predictorConfig struct {
Type string `yaml:"type"`
// Factors multiply the odds of the scorer's price, one per piece of
// evidence, keyed by evidence name. An omitted factor is neutral, so an
// omitted block ranks on the scorer's price alone. Values are hand-set
// placeholders, not measured: they are uncalibrated until the fitting work
// in doc/rfc/submitqueue/outcome-predictor.md lands.
Factors map[string]float64 `yaml:"factors"`
}

// loadProfilesConfig reads and validates the profiles configuration at path.
func loadProfilesConfig(path string) (profilesConfig, error) {
data, err := os.ReadFile(path)
Expand Down Expand Up @@ -300,6 +331,11 @@ func (c *profilesConfig) normalizeAndValidate() error {
return err
}
}
if q.Predictor != nil {
if err := q.Predictor.normalizeAndValidate(where); err != nil {
return err
}
}
}
return c.validateGitRepoPaths()
}
Expand Down Expand Up @@ -358,6 +394,9 @@ func (c profilesConfig) resolve(q namedQueueProfileConfig) queueProfileConfig {
if q.Speculator != nil {
profile.Speculator = *q.Speculator
}
if q.Predictor != nil {
profile.Predictor = *q.Predictor
}
return profile
}

Expand All @@ -374,7 +413,10 @@ func (p *queueProfileConfig) normalizeAndValidate(where string) error {
if err := p.Scorer.normalizeAndValidate(where); err != nil {
return err
}
return p.Speculator.normalizeAndValidate(where)
if err := p.Speculator.normalizeAndValidate(where); err != nil {
return err
}
return p.Predictor.normalizeAndValidate(where)
}

func (c *changeProviderConfig) normalizeAndValidate(where string) error {
Expand Down Expand Up @@ -556,6 +598,31 @@ func (s *scorerConfig) normalizeAndValidate(where string) error {
return nil
}

// normalizeAndValidate applies defaults and rejects a predictor that could not
// be built. An empty block is an evidence predictor with every factor neutral,
// which prices a batch at exactly its scorer's price.
func (p *predictorConfig) normalizeAndValidate(where string) error {
if p.Type == "" {
p.Type = predictorTypeEvidence
}
if p.Type != predictorTypeEvidence {
return fmt.Errorf("%s: unknown predictor type %q", where, p.Type)
}
for name, factor := range p.Factors {
switch name {
case factorPathPassed, factorPathFailed, factorMerging, factorCancelling:
default:
return fmt.Errorf("%s: unknown predictor factor %q", where, name)
}
// Zero would pin every batch carrying the evidence to a probability of
// zero, and a negative multiplier on odds means nothing at all.
if factor <= 0 {
return fmt.Errorf("%s: predictor factor %q is %v, must be positive", where, name, factor)
}
}
return nil
}

func (s *speculatorConfig) normalizeAndValidate(where string) error {
// A negative budget is rejected rather than clamped: sticky would compute no
// free slots from it, so the queue would batch and then never build anything,
Expand Down
46 changes: 46 additions & 0 deletions service/submitqueue/orchestrator/server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,3 +666,49 @@ func TestLoadProfilesConfig_RejectsBadScorers(t *testing.T) {
})
}
}

func TestLoadProfilesConfig_RejectsBadPredictors(t *testing.T) {
tests := []struct {
name string
contents string
}{
{name: "unknown predictor type", contents: "defaults:\n predictor: {type: vibes}\n"},
{name: "unknown factor", contents: "defaults:\n predictor:\n factors: {pathPased: 2}\n"},
{name: "zero factor", contents: "defaults:\n predictor:\n factors: {merging: 0}\n"},
{name: "negative factor", contents: "defaults:\n predictor:\n factors: {pathFailed: -1}\n"},
{name: "bad factor on a queue override", contents: "defaults: {}\nqueues:\n - name: q\n predictor:\n factors: {merging: 0}\n"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := loadProfilesConfig(writeProfiles(t, tt.contents))
require.Error(t, err)
})
}
}

// An omitted predictor block leaves the queue ranking on its scorer's price
// alone, which is what every queue does until someone states a factor.
func TestLoadProfilesConfig_DefaultsThePredictorToNeutral(t *testing.T) {
cfg, err := loadProfilesConfig(writeProfiles(t, "defaults: {}\nqueues:\n - name: q\n"))
require.NoError(t, err)

assert.Equal(t, predictorTypeEvidence, cfg.Defaults.Predictor.Type)

factors := factorsFrom(cfg.resolve(cfg.Queues[0]).Predictor)
assert.Equal(t, neutralFactor, factors.PathPassed)
assert.Equal(t, neutralFactor, factors.PathFailed)
assert.Equal(t, neutralFactor, factors.Merging)
assert.Equal(t, neutralFactor, factors.Cancelling)
}

func TestLoadProfilesConfig_ReadsPredictorFactors(t *testing.T) {
cfg, err := loadProfilesConfig(writeProfiles(t,
"defaults:\n predictor:\n factors: {pathPassed: 10, pathFailed: 0.3, merging: 12, cancelling: 0.1}\n"))
require.NoError(t, err)

factors := factorsFrom(cfg.Defaults.Predictor)
assert.Equal(t, 10.0, factors.PathPassed)
assert.Equal(t, 0.3, factors.PathFailed)
assert.Equal(t, 12.0, factors.Merging)
assert.Equal(t, 0.1, factors.Cancelling)
}
Loading