Skip to content

Commit 61c005d

Browse files
behinddwallsgithub-actions[bot]
authored andcommitted
feat(speculate): write the speculation tree in shadow mode
## Summary ### Why? This is the first of four slices reproducing the tree-driven speculate/build/buildsignal rework as an incremental, always-shippable stack instead of one large commit. Each slice keeps the orchestrator working end to end so the e2e suite stays green throughout, and reviewers can follow the pipeline shift (direct publish -> tree -> prioritize -> build) one deliberate step at a time. This slice only adds the speculation tree as bookkeeping: nothing downstream reads it yet, so the batch's forward step (when it builds, when it merges) is untouched — with one deliberate exception called out below: a liveness fix to the dependent wake-up that review of this slice surfaced. ### What? speculate's Controller now takes an enumerator, path scorer, selector, and dependency-limit factory, and every Created/Scored/Speculating pass loads or creates the batch's entity.SpeculationTree, applies the scorer's and selector's outputs, and persists it only if something changed (the apply steps report whether they mutated anything, gating a version+1 conditional Update). The controller stays pure mechanics: which paths exist, how they score, and which are promoted or cancelled are the seams' decisions alone — the controller validates and records them. Tree creation is gated by the queue's dependency limit; the controller wraps the enumerator's structure-only paths into persisted entries itself — stamping Candidate, minting each path's immutable ID as `{batchID}/path/{i}`, and skipping structural duplicates as a contract violation — and a concurrent create race re-reads the winner's tree instead of erroring. Seam outputs are consumed by path ID: scores merged by ID (unknown IDs skipped, out-of-[0,1] values clamped, both logged), selector decisions resolved through the ID-keyed PathIndex with duplicates logged and skipped. The pre-existing forward step is otherwise unchanged: Created/Scored batches CAS to Speculating and publish straight to build, and Speculating batches run the original tryFinalize (merge once every dependency has landed, cascade-fail on a failed dependency). Cancelling is the unmodified pre-existing flow. Liveness fix in the forward step: a batch waiting on dependencies (in tryFinalize, or now in the dependency gate) was only ever woken when a dependency was cancelled — a dependency reaching Succeeded or Failed never re-published its dependents, because mergesignal routes every terminal transition back through speculate under the batch's own ID and the terminal branch only re-published conclude. The terminal branch now fans out to dependents for every terminal state, and failOnDependency wakes its own dependents right after the terminal CAS so failures cascade downstream. The fan-out publishes a wake for every listed dependent; a dangling reverse-index entry (left by an abandoned batch creation) surfaces as a not-found speculate message that dead-letters and is skipped by the DLQ reconciler — eliminating that class at the source, by creating the batch before the reverse-index update, is tracked in #354. Cancel decisions on tree paths are recorded as status only (Cancelling on an in-flight build) — nothing in this controller calls a build runner, which is why it takes no buildrunner.Factory. main.go wires the new seams with parity defaults: chain enumerates a single path per batch, the probability path scorer, the all-selector promotes every candidate, and a static dependency limit that is effectively ungated. ## Test Plan `bazel test //submitqueue/... //service/...` — 56/56 targets pass. New unit coverage for the wake-up fix: dependent fan-out on every terminal state (with publish-order assertions), missing BatchDependent row surfaced as an invariant error, and dependent-publish failure after the terminal CAS nacking for redelivery convergence. `make gazelle && make fmt` — no diffs beyond the intended BUILD file updates. ## Issue Part of the speculation rework. Interim fan-out caveats tracked in #354.
1 parent b0bc04d commit 61c005d

5 files changed

Lines changed: 1012 additions & 434 deletions

File tree

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,17 @@ go_library(
4141
"//submitqueue/extension/scorer/composite:go_default_library",
4242
"//submitqueue/extension/scorer/fake:go_default_library",
4343
"//submitqueue/extension/scorer/heuristic:go_default_library",
44+
"//submitqueue/extension/speculation/dependencylimit:go_default_library",
45+
"//submitqueue/extension/speculation/dependencylimit/static:go_default_library",
46+
"//submitqueue/extension/speculation/enumerator:go_default_library",
47+
"//submitqueue/extension/speculation/enumerator/chain:go_default_library",
48+
"//submitqueue/extension/speculation/pathscorer:go_default_library",
49+
"//submitqueue/extension/speculation/pathscorer/probability:go_default_library",
4450
"//submitqueue/extension/speculation/prioritizationlimit/static:go_default_library",
4551
"//submitqueue/extension/speculation/prioritizer:go_default_library",
4652
"//submitqueue/extension/speculation/prioritizer/sticky:go_default_library",
53+
"//submitqueue/extension/speculation/selector:go_default_library",
54+
"//submitqueue/extension/speculation/selector/all:go_default_library",
4755
"//submitqueue/extension/storage:go_default_library",
4856
"//submitqueue/extension/storage/mysql:go_default_library",
4957
"//submitqueue/extension/validator/fake:go_default_library",

service/submitqueue/orchestrator/server/main.go

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,17 @@ import (
6161
"github.com/uber/submitqueue/submitqueue/extension/scorer/composite"
6262
scorerfake "github.com/uber/submitqueue/submitqueue/extension/scorer/fake"
6363
"github.com/uber/submitqueue/submitqueue/extension/scorer/heuristic"
64+
"github.com/uber/submitqueue/submitqueue/extension/speculation/dependencylimit"
65+
dependencylimitstatic "github.com/uber/submitqueue/submitqueue/extension/speculation/dependencylimit/static"
66+
"github.com/uber/submitqueue/submitqueue/extension/speculation/enumerator"
67+
"github.com/uber/submitqueue/submitqueue/extension/speculation/enumerator/chain"
68+
"github.com/uber/submitqueue/submitqueue/extension/speculation/pathscorer"
69+
pathscorerprobability "github.com/uber/submitqueue/submitqueue/extension/speculation/pathscorer/probability"
6470
prioritizationlimitstatic "github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizationlimit/static"
6571
"github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizer"
6672
"github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizer/sticky"
73+
"github.com/uber/submitqueue/submitqueue/extension/speculation/selector"
74+
selectorall "github.com/uber/submitqueue/submitqueue/extension/speculation/selector/all"
6775
"github.com/uber/submitqueue/submitqueue/extension/storage"
6876
mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql"
6977
validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake"
@@ -238,7 +246,7 @@ func run() error {
238246
// back to a baseline profile for queues without an explicit entry. This is
239247
// the single place queue topology is known; the extension packages stay
240248
// queue-agnostic.
241-
queues, err := newQueueRegistry(logger, scope, changeset.New(store.GetRequestStore(), store.GetChangeStore()))
249+
queues, err := newQueueRegistry(logger, scope, changeset.New(store.GetRequestStore(), store.GetChangeStore()), store.GetBatchStore())
242250
if err != nil {
243251
return fmt.Errorf("failed to build queue registry: %w", err)
244252
}
@@ -249,9 +257,13 @@ func run() error {
249257
scf := scorerFactory{queues}
250258
cof := analyzerFactory{queues}
251259
prf := prioritizerFactory{queues}
260+
enf := enumeratorFactory{queues}
261+
ssf := speculationScorerFactory{queues}
262+
slf := speculationSelectorFactory{queues}
263+
dlf := dependencyLimitFactory{queues}
252264

253265
// Register controllers
254-
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, registry, cpf, brf, scf, cof, prf, cnt, store)
266+
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, registry, cpf, brf, scf, cof, prf, enf, ssf, slf, dlf, cnt, store)
255267
if err != nil {
256268
return err
257269
}
@@ -502,11 +514,15 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
502514
// read as "for this queue, here are its scorer, analyzer, change provider, …", and lets
503515
// a queue profile start from a baseline and override only what differs.
504516
type queueExtensions struct {
505-
changeProvider changeprovider.ChangeProvider
506-
buildRunner buildrunner.BuildRunner
507-
scorer scorer.Scorer
508-
analyzer conflict.Analyzer
509-
prioritizer prioritizer.Prioritizer
517+
changeProvider changeprovider.ChangeProvider
518+
buildRunner buildrunner.BuildRunner
519+
scorer scorer.Scorer
520+
analyzer conflict.Analyzer
521+
prioritizer prioritizer.Prioritizer
522+
enumerator enumerator.Enumerator
523+
speculationScorer pathscorer.Scorer
524+
speculationSelector selector.Selector
525+
dependencyLimit dependencylimit.DependencyLimit
510526
}
511527

512528
// queueRegistry maps a queue name to its extensions, falling back to a default
@@ -558,7 +574,31 @@ func (f prioritizerFactory) For(cfg prioritizer.Config) (prioritizer.Prioritizer
558574
return f.reg.get(cfg.QueueName).prioritizer, nil
559575
}
560576

561-
func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, cpf changeprovider.Factory, brf buildrunner.Factory, scf scorer.Factory, cof conflict.Factory, prf prioritizer.Factory, cnt counter.Counter, store storage.Storage) (int, error) {
577+
type enumeratorFactory struct{ reg queueRegistry }
578+
579+
func (f enumeratorFactory) For(cfg enumerator.Config) (enumerator.Enumerator, error) {
580+
return f.reg.get(cfg.QueueName).enumerator, nil
581+
}
582+
583+
type speculationScorerFactory struct{ reg queueRegistry }
584+
585+
func (f speculationScorerFactory) For(cfg pathscorer.Config) (pathscorer.Scorer, error) {
586+
return f.reg.get(cfg.QueueName).speculationScorer, nil
587+
}
588+
589+
type speculationSelectorFactory struct{ reg queueRegistry }
590+
591+
func (f speculationSelectorFactory) For(cfg selector.Config) (selector.Selector, error) {
592+
return f.reg.get(cfg.QueueName).speculationSelector, nil
593+
}
594+
595+
type dependencyLimitFactory struct{ reg queueRegistry }
596+
597+
func (f dependencyLimitFactory) For(cfg dependencylimit.Config) (dependencylimit.DependencyLimit, error) {
598+
return f.reg.get(cfg.QueueName).dependencyLimit, nil
599+
}
600+
601+
func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, cpf changeprovider.Factory, brf buildrunner.Factory, scf scorer.Factory, cof conflict.Factory, prf prioritizer.Factory, enf enumerator.Factory, ssf pathscorer.Factory, slf selector.Factory, dlf dependencylimit.Factory, cnt counter.Counter, store storage.Storage) (int, error) {
562602
var count int
563603
requestController := start.NewController(
564604
logger,
@@ -648,6 +688,10 @@ func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger,
648688
logger,
649689
scope,
650690
store,
691+
enf,
692+
ssf,
693+
slf,
694+
dlf,
651695
registry,
652696
topickey.TopicKeySpeculate,
653697
"orchestrator-speculate",
@@ -899,14 +943,19 @@ func newPhabChangeProvider(logger *zap.Logger, scope tally.Scope) (changeprovide
899943
// effectively admit-all — until per-queue budgets are configured.
900944
const defaultPrioritizationLimit = 1000
901945

946+
// defaultDependencyLimit is the baseline cap on active dependencies a batch
947+
// may speculate over. It is a parity default — effectively ungated — until
948+
// per-queue limits are configured.
949+
const defaultDependencyLimit = 1000
950+
902951
// newQueueRegistry builds the per-queue extension profiles for the example.
903952
// Edge integrations (change provider) and the build
904953
// runner form a shared baseline; each per-queue profile starts from that
905954
// baseline and overrides only the extensions that differ — here the scorer and
906955
// conflict analyzer. Queues without an explicit profile fall back to the
907956
// baseline. This is the one place queue topology lives; extension packages stay
908957
// queue-agnostic.
909-
func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.Resolver) (queueRegistry, error) {
958+
func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.Resolver, batchStore storage.BatchStore) (queueRegistry, error) {
910959
cp, err := newChangeProvider(logger, scope)
911960
if err != nil {
912961
return queueRegistry{}, fmt.Errorf("failed to create change provider: %w", err)
@@ -933,6 +982,12 @@ func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.
933982
// below does. The prioritizer is sticky over a static budget: it never
934983
// preempts a running build and admits Selected candidates by score until
935984
// defaultPrioritizationLimit concurrent builds are in flight.
985+
//
986+
// The speculation seams default to the single-chain parity policies:
987+
// chain enumerates one path per batch (built on the full ordered
988+
// dependency chain), probability scores paths from the batches'
989+
// predicted-success probabilities and resolved outcomes, all promotes
990+
// every candidate, and the dependency limit is effectively ungated.
936991
base := queueExtensions{
937992
changeProvider: cp,
938993
buildRunner: buildfake.New(resolver),
@@ -943,8 +998,12 @@ func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.
943998
)),
944999
// TODO: replace the delegate with a real analyzer (e.g. Tango target
9451000
// analysis). "all" serializes the queue conservatively.
946-
analyzer: conflictfake.New(all.New(), nil),
947-
prioritizer: sticky.New(prioritizationlimitstatic.New(defaultPrioritizationLimit)),
1001+
analyzer: conflictfake.New(all.New(), nil),
1002+
prioritizer: sticky.New(prioritizationlimitstatic.New(defaultPrioritizationLimit)),
1003+
enumerator: chain.New(),
1004+
speculationScorer: pathscorerprobability.New(batchStore),
1005+
speculationSelector: selectorall.New(),
1006+
dependencyLimit: dependencylimitstatic.New(defaultDependencyLimit),
9481007
}
9491008

9501009
// test-queue: bucketed heuristic scorer; conservative (serialized) conflicts

submitqueue/orchestrator/controller/speculate/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ go_library(
1111
"//platform/metrics:go_default_library",
1212
"//submitqueue/core/topickey:go_default_library",
1313
"//submitqueue/entity:go_default_library",
14+
"//submitqueue/extension/speculation/dependencylimit:go_default_library",
15+
"//submitqueue/extension/speculation/enumerator:go_default_library",
16+
"//submitqueue/extension/speculation/pathscorer:go_default_library",
17+
"//submitqueue/extension/speculation/selector:go_default_library",
1418
"//submitqueue/extension/storage:go_default_library",
1519
"@com_github_uber_go_tally//:go_default_library",
1620
"@org_uber_go_zap//:go_default_library",
@@ -28,6 +32,14 @@ go_test(
2832
"//platform/extension/messagequeue/mock:go_default_library",
2933
"//submitqueue/core/topickey:go_default_library",
3034
"//submitqueue/entity:go_default_library",
35+
"//submitqueue/extension/speculation/dependencylimit/fake:go_default_library",
36+
"//submitqueue/extension/speculation/dependencylimit/mock:go_default_library",
37+
"//submitqueue/extension/speculation/enumerator/fake:go_default_library",
38+
"//submitqueue/extension/speculation/enumerator/mock:go_default_library",
39+
"//submitqueue/extension/speculation/pathscorer/fake:go_default_library",
40+
"//submitqueue/extension/speculation/pathscorer/mock:go_default_library",
41+
"//submitqueue/extension/speculation/selector/fake:go_default_library",
42+
"//submitqueue/extension/speculation/selector/mock:go_default_library",
3143
"//submitqueue/extension/storage:go_default_library",
3244
"//submitqueue/extension/storage/mock:go_default_library",
3345
"@com_github_stretchr_testify//assert:go_default_library",

0 commit comments

Comments
 (0)