Skip to content

Commit f7b7d43

Browse files
authored
Merge branch 'main' into mnoah1/stovepipe-process-gate-defer
2 parents 5b651c5 + 22f670e commit f7b7d43

25 files changed

Lines changed: 1479 additions & 121 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ Domain objects live under each domain's `entity/` tree, or under `platform/base/
120120
4. Every field must have a comment
121121
5. Reference other entities by ID (string or int), not directly
122122
6. String enums with sentinel values (`""` for unknown)
123+
7. Docs describe the data, not the choreography — say what a type or field *is* and its invariants (immutability, uniqueness scope, units, valid range), never which controller/stage/seam reads or writes it. Ownership and write-path rules live with the code that owns them (controller, store, or extension docs). Lifecycle enums may define states in terms of pipeline stages where that *is* the state's meaning (e.g. "admitted under the build budget"), but must not name the components that perform transitions.
123124

124125
### Extensions
125126

api/runway/messagequeue/proto/merge.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ message MergeStep {
4848
// Runway echoes it back unchanged.
4949
message MergeRequest {
5050
option (uber.base.messagequeue.topic_keys) = "merge-conflict-check";
51-
option (uber.base.messagequeue.topic_keys) = "merge";
51+
option (uber.base.messagequeue.topic_keys) = "runway-merge";
5252

5353
// id is the client-owned correlation id for this request (one per request).
5454
// Runway echoes it back on the result unchanged.

api/runway/messagequeue/protopb/merge.pb.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/runway/messagequeue/topics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const (
3434
// TopicKeyMerge carries committing merge requests. A client publishes a
3535
// MergeRequest here; Runway applies the steps, commits the result, and
3636
// reports the revisions it produced.
37-
TopicKeyMerge TopicKey = "merge"
37+
TopicKeyMerge TopicKey = "runway-merge"
3838
// TopicKeyMergeSignal carries committing merge results. Runway publishes a
3939
// MergeResult here (with the produced revisions populated); the requesting
4040
// client consumes it.

platform/consumer/registry.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ type topicGroup struct {
6262
}
6363

6464
// NewTopicRegistry creates a new TopicRegistry from a list of TopicConfigs.
65-
// Returns an error if any topic name is invalid.
65+
// Returns an error if any topic name is invalid, or if two configs share a
66+
// topic key — a duplicate key would silently shadow the earlier entry (last
67+
// write wins on the key→queue/name maps), routing publishes and subscriptions
68+
// registered against one topic onto another.
6669
func NewTopicRegistry(configs []TopicConfig) (TopicRegistry, error) {
6770
queues := make(map[TopicKey]extqueue.Queue, len(configs))
6871
topicNames := make(map[TopicKey]string, len(configs))
@@ -73,6 +76,12 @@ func NewTopicRegistry(configs []TopicConfig) (TopicRegistry, error) {
7376
return TopicRegistry{}, fmt.Errorf("invalid topic name for key %s: %w", cfg.Key, err)
7477
}
7578

79+
if existing, ok := topicNames[cfg.Key]; ok {
80+
return TopicRegistry{}, fmt.Errorf(
81+
"duplicate topic key %s: already registered with name %q, cannot also register name %q",
82+
cfg.Key, existing, cfg.Name)
83+
}
84+
7685
queues[cfg.Key] = cfg.Queue
7786
topicNames[cfg.Key] = cfg.Name
7887

service/runway/server/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
275275
},
276276
{
277277
Key: runwaymq.TopicKeyMerge,
278-
Name: "merge",
278+
Name: "runway-merge",
279279
Queue: q,
280280
Subscription: extqueue.DefaultSubscriptionConfig(
281281
subscriberName, "runway-merge",

service/submitqueue/orchestrator/server/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ 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/prioritizationlimit/static:go_default_library",
45+
"//submitqueue/extension/speculation/prioritizer:go_default_library",
46+
"//submitqueue/extension/speculation/prioritizer/sticky:go_default_library",
4447
"//submitqueue/extension/storage:go_default_library",
4548
"//submitqueue/extension/storage/mysql:go_default_library",
4649
"//submitqueue/extension/validator/fake:go_default_library",
@@ -54,6 +57,7 @@ go_library(
5457
"//submitqueue/orchestrator/controller/merge:go_default_library",
5558
"//submitqueue/orchestrator/controller/mergeconflictsignal:go_default_library",
5659
"//submitqueue/orchestrator/controller/mergesignal:go_default_library",
60+
"//submitqueue/orchestrator/controller/prioritize:go_default_library",
5761
"//submitqueue/orchestrator/controller/score:go_default_library",
5862
"//submitqueue/orchestrator/controller/speculate:go_default_library",
5963
"//submitqueue/orchestrator/controller/start:go_default_library",

service/submitqueue/orchestrator/server/main.go

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ 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+
prioritizationlimitstatic "github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizationlimit/static"
65+
"github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizer"
66+
"github.com/uber/submitqueue/submitqueue/extension/speculation/prioritizer/sticky"
6467
"github.com/uber/submitqueue/submitqueue/extension/storage"
6568
mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql"
6669
validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake"
@@ -74,6 +77,7 @@ import (
7477
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/merge"
7578
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergeconflictsignal"
7679
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergesignal"
80+
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/prioritize"
7781
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/score"
7882
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/speculate"
7983
"github.com/uber/submitqueue/submitqueue/orchestrator/controller/start"
@@ -244,13 +248,14 @@ func run() error {
244248
brf := buildRunnerFactory{queues}
245249
scf := scorerFactory{queues}
246250
cof := analyzerFactory{queues}
251+
prf := prioritizerFactory{queues}
247252

248253
// Register controllers
249-
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, registry, cpf, brf, scf, cof, cnt, store)
254+
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, registry, cpf, brf, scf, cof, prf, cnt, store)
250255
if err != nil {
251256
return err
252257
}
253-
dlqCount, err := registerDLQControllers(dlqConsumer, logger.Sugar(), scope, store)
258+
dlqCount, err := registerDLQControllers(dlqConsumer, logger.Sugar(), scope, registry, store)
254259
if err != nil {
255260
return err
256261
}
@@ -380,9 +385,10 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
380385
{topickey.TopicKeyBatch, "batch", "orchestrator-batch"},
381386
{topickey.TopicKeyScore, "score", "orchestrator-score"},
382387
{topickey.TopicKeySpeculate, "speculate", "orchestrator-speculate"},
388+
{topickey.TopicKeyPrioritize, "prioritize", "orchestrator-prioritize"},
383389
{topickey.TopicKeyBuild, "build", "orchestrator-build"},
384390
{topickey.TopicKeyBuildSignal, "buildsignal", "orchestrator-buildsignal"},
385-
{topickey.TopicKeyMerge, "merge", "orchestrator-merge"},
391+
{topickey.TopicKeyMerge, "submitqueue-merge", "orchestrator-merge"},
386392
{runwaymq.TopicKeyMergeSignal, "merge-signal", "orchestrator-mergesignal"},
387393
{topickey.TopicKeyConclude, "conclude", "orchestrator-conclude"},
388394
}
@@ -450,7 +456,7 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
450456
// consumed primary topic above.
451457
configs = append(configs, consumer.TopicConfig{
452458
Key: runwaymq.TopicKeyMerge,
453-
Name: "merge",
459+
Name: "runway-merge",
454460
Queue: q,
455461
})
456462

@@ -471,6 +477,13 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
471477
// merge-conflict-check queue (⇢); runway performs the merge attempt and
472478
// publishes the result to merge-conflict-check-signal, which mergeconflictsignal
473479
// consumes before fanning the request out to batch.
480+
//
481+
// prioritize sits alongside this per-batch flow rather than in its line: it
482+
// is queue-wide, not batch-scoped. Its message carries only a queue name; on
483+
// each invocation it loads every Speculating batch's speculation tree for
484+
// that queue, ranks the queue-wide candidate paths against the queue's build
485+
// budget, applies the resulting decisions, and republishes to build for any
486+
// path newly (or still) cleared to run.
474487

475488
// TODO(wiring abstraction): queueExtensions + queueRegistry currently live here
476489
// as example-local wiring. Evaluate promoting them into a defined abstraction in
@@ -493,6 +506,7 @@ type queueExtensions struct {
493506
buildRunner buildrunner.BuildRunner
494507
scorer scorer.Scorer
495508
analyzer conflict.Analyzer
509+
prioritizer prioritizer.Prioritizer
496510
}
497511

498512
// queueRegistry maps a queue name to its extensions, falling back to a default
@@ -538,7 +552,13 @@ func (f analyzerFactory) For(cfg conflict.Config) (conflict.Analyzer, error) {
538552
return f.reg.get(cfg.QueueName).analyzer, nil
539553
}
540554

541-
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, cnt counter.Counter, store storage.Storage) (int, error) {
555+
type prioritizerFactory struct{ reg queueRegistry }
556+
557+
func (f prioritizerFactory) For(cfg prioritizer.Config) (prioritizer.Prioritizer, error) {
558+
return f.reg.get(cfg.QueueName).prioritizer, nil
559+
}
560+
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) {
542562
var count int
543563
requestController := start.NewController(
544564
logger,
@@ -637,6 +657,20 @@ func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger,
637657
}
638658
count++
639659

660+
prioritizeController := prioritize.NewController(
661+
logger,
662+
scope,
663+
store,
664+
prf,
665+
registry,
666+
topickey.TopicKeyPrioritize,
667+
"orchestrator-prioritize",
668+
)
669+
if err := c.Register(prioritizeController); err != nil {
670+
return count, fmt.Errorf("failed to register prioritize controller: %w", err)
671+
}
672+
count++
673+
640674
buildController := build.NewController(
641675
logger,
642676
scope,
@@ -712,7 +746,7 @@ func registerPrimaryControllers(c consumer.Consumer, logger *zap.SugaredLogger,
712746
// registers them with the DLQ consumer. Each reconciler drives the affected
713747
// request or batch into a terminal Error/Failed state so the gateway stops
714748
// reporting it as stuck-in-progress.
715-
func registerDLQControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, store storage.Storage) (int, error) {
749+
func registerDLQControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, store storage.Storage) (int, error) {
716750
dlqScope := scope.SubScope("dlq")
717751
dlqRegs := []struct {
718752
name string
@@ -725,6 +759,7 @@ func registerDLQControllers(c consumer.Consumer, logger *zap.SugaredLogger, scop
725759
{"batch_dlq", dlq.NewDLQRequestController(logger, dlqScope, store, dlq.DecodeRequestID, dlq.TopicKey(topickey.TopicKeyBatch), "orchestrator-batch-dlq")},
726760
{"score_dlq", dlq.NewDLQBatchController(logger, dlqScope, store, dlq.TopicKey(topickey.TopicKeyScore), "orchestrator-score-dlq")},
727761
{"speculate_dlq", dlq.NewDLQBatchController(logger, dlqScope, store, dlq.TopicKey(topickey.TopicKeySpeculate), "orchestrator-speculate-dlq")},
762+
{"prioritize_dlq", dlq.NewDLQQueueController(logger, dlqScope, registry, dlq.TopicKey(topickey.TopicKeyPrioritize), "orchestrator-prioritize-dlq")},
728763
{"build_dlq", dlq.NewDLQBatchController(logger, dlqScope, store, dlq.TopicKey(topickey.TopicKeyBuild), "orchestrator-build-dlq")},
729764
{"buildsignal_dlq", dlq.NewDLQBuildSignalController(logger, dlqScope, store, dlq.TopicKey(topickey.TopicKeyBuildSignal), "orchestrator-buildsignal-dlq")},
730765
{"merge_dlq", dlq.NewDLQBatchController(logger, dlqScope, store, dlq.TopicKey(topickey.TopicKeyMerge), "orchestrator-merge-dlq")},
@@ -859,6 +894,11 @@ func newPhabChangeProvider(logger *zap.Logger, scope tally.Scope) (changeprovide
859894
}), nil
860895
}
861896

897+
// defaultPrioritizationLimit is the baseline queue-wide concurrent-build
898+
// budget handed to the sticky prioritizer. It is a parity default —
899+
// effectively admit-all — until per-queue budgets are configured.
900+
const defaultPrioritizationLimit = 1000
901+
862902
// newQueueRegistry builds the per-queue extension profiles for the example.
863903
// Edge integrations (change provider) and the build
864904
// runner form a shared baseline; each per-queue profile starts from that
@@ -880,16 +920,19 @@ func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.
880920

881921
// Baseline profile: shared edge integrations + a fake build runner (every
882922
// build succeeds unless a head URI carries a failure marker), plus permissive
883-
// defaults for scorer and conflict. The build runner instance is shared by
884-
// the build and buildsignal controllers (same profile, same instance) so a
885-
// build's recorded outcome survives across their separate factory lookups.
923+
// defaults for scorer, conflict, and prioritization. The build runner
924+
// instance is shared by the build and buildsignal controllers (same
925+
// profile, same instance) so a build's recorded outcome survives across
926+
// their separate factory lookups.
886927
//
887928
// The scorer is wrapped by scorerfake so a change URI carrying
888929
// "sq-fake=score-error" forces a scoring error end-to-end; it is a pure
889930
// passthrough otherwise. The analyzer is wrapped by conflictfake with a nil
890931
// predicate (passthrough) — swap the predicate (e.g. conflictfake.FailAlways)
891932
// on a queue to exercise the analyzer error path, as e2e-conflict-error-queue
892-
// below does.
933+
// below does. The prioritizer is sticky over a static budget: it never
934+
// preempts a running build and admits Selected candidates by score until
935+
// defaultPrioritizationLimit concurrent builds are in flight.
893936
base := queueExtensions{
894937
changeProvider: cp,
895938
buildRunner: buildfake.New(resolver),
@@ -900,7 +943,8 @@ func newQueueRegistry(logger *zap.Logger, scope tally.Scope, resolver changeset.
900943
)),
901944
// TODO: replace the delegate with a real analyzer (e.g. Tango target
902945
// analysis). "all" serializes the queue conservatively.
903-
analyzer: conflictfake.New(all.New(), nil),
946+
analyzer: conflictfake.New(all.New(), nil),
947+
prioritizer: sticky.New(prioritizationlimitstatic.New(defaultPrioritizationLimit)),
904948
}
905949

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

stovepipe/extension/storage/mock/queue_store_mock.go

Lines changed: 10 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

submitqueue/core/topickey/topickey.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ const (
3333
TopicKeyScore TopicKey = "score"
3434
// TopicKeySpeculate is the pipeline stage where scored batches are published for speculation.
3535
TopicKeySpeculate TopicKey = "speculate"
36+
// TopicKeyPrioritize is the queue-wide reconcile stage that rations the
37+
// build budget across every in-flight batch of a queue. Each message
38+
// carries a QueueID; the consumer loads every Speculating batch's tree,
39+
// runs the queue's Prioritizer over the candidate paths, applies the
40+
// resulting decisions — promoting paths into the build budget, or
41+
// cancelling in-flight paths a preemptive policy evicts — and republishes
42+
// to TopicKeyBuild for any path cleared to run.
43+
TopicKeyPrioritize TopicKey = "prioritize"
3644
// TopicKeyBuild is the pipeline stage where speculated batches are published for builds.
3745
TopicKeyBuild TopicKey = "build"
3846
// TopicKeyBuildSignal is the polling stage for triggered builds. Each
@@ -42,7 +50,7 @@ const (
4250
// PublishAfter when the build has not yet reached a terminal state.
4351
TopicKeyBuildSignal TopicKey = "buildsignal"
4452
// TopicKeyMerge is the pipeline stage where speculated batches are published for merging.
45-
TopicKeyMerge TopicKey = "merge"
53+
TopicKeyMerge TopicKey = "submitqueue-merge"
4654
// TopicKeyConclude is the pipeline stage where merged requests are published for conclusion.
4755
TopicKeyConclude TopicKey = "conclude"
4856
// TopicKeyLog is the pipeline stage where per-request logs are written.

0 commit comments

Comments
 (0)