Skip to content

Commit 61ff6cc

Browse files
committed
feat(orchestrator): add queue-wide prioritize stage
## Summary ### Why? Selection is per batch and blind to other batches, so it cannot ration a shared build budget: if every batch promoted generously, their combined demand could swamp CI. The speculation design closes that gap with a prioritization step that sees every candidate path across a queue's in-flight batches and admits only what fits the queue's concurrent-build budget. That reconcile is queue-scoped, not batch-scoped, so it gets its own pipeline stage rather than piggybacking on the per-batch speculate flow. ### What? New `prioritize` topic (payload: `entity.QueueID`, partitioned by queue name) and controller. `entity.QueueID` is introduced here with ToBytes/QueueIDFromBytes, mirroring the BatchID payload pattern for queue-scoped stages. Each invocation is a full budget round for one queue: load every Speculating batch's speculation tree (skipping batches not yet speculated), flatten the queue-wide candidates (Selected / Prioritized / Building paths), hand them to the queue's `prioritizer.Prioritizer`, and apply the sparse decisions as captured intent — Promote flips Selected→Prioritized; Cancel on a Building path flips it to Cancelling (this controller never talks to the build system: the build stage owns runner interaction and enacts the persisted intent, retried on every build message until the build terminates); Cancel on a not-yet-building Prioritized path drops it straight to Cancelled; illegal decisions are logged and skipped so a policy bug cannot corrupt tree state. Path lookup for decision application lives on the entity (`SpeculationTree.PathIndex`), keyed by the path ID decisions carry; the controller maps each ID back to its tree via the candidates it loaded this round (never by parsing the ID), and treats a duplicate decision for the same path as a policy bug — logged and skipped like any other illegal decision. Each affected tree is persisted under its own optimistic lock (version arithmetic in the controller); any conflict nacks the round, which is safe to replay because decisions are recomputed from freshly read state — a redelivered round carries no memory of what a previous attempt picked and needs none, since already-promoted paths are counted as slot holders rather than re-promoted. After applying, the controller republishes to build for every batch whose tree carries work the build stage still has to enact — a Prioritized path with no build, or a Cancelling intent — healing dropped build messages idempotently. Wiring: topic registered with an automatic DLQ pair. The DLQ reconciler re-arms the queue rather than terminalizing an entity: it logs the failure and republishes a fresh prioritize round under a distinct, deterministic message ID, so a queue whose only pending work is waiting on prioritization is not stranded when a round dead-letters. The requeue cannot poison-loop — a round carries only the queue name and recomputes from live state — and a persistently failing round cycles retry-ladder→DLQ→requeue at full-ladder cadence, visible in metrics, converging once the fault heals. The sticky prioritizer (never preempts) backed by a static admit-all parity limit is wired as the default per-queue profile. Tree-entity docs are cleaned to describe the data itself (states, invariants, uniqueness) rather than narrating which stage reads or writes what. Nothing publishes to the topic yet — the speculate rework turns it on. ## Test Plan ✅ `make gazelle && make fmt && make test` and `bazel build //service/submitqueue/orchestrator/...`. Controller unit tests cover: empty queue ack, missing-tree skip, promote transition + versioned update + build republish, illegal-decision skip, cancel-on-building capturing intent (Cancelling persisted, build republished, no runner dependency at all), cancel-on-prioritized dropping straight to Cancelled, version-mismatch and prioritizer errors nacking, and republish for pre-existing prioritized paths with zero new decisions. DLQ reconciler tests cover the requeued round's ID/partition/payload, publish-failure nack, and malformed/empty payload rejection. Entity tests cover `SpeculationTree.PathIndex` (order-sensitive base matching, empty tree).
1 parent 112d263 commit 61ff6cc

14 files changed

Lines changed: 1439 additions & 97 deletions

File tree

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: 53 additions & 9 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,6 +385,7 @@ 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"},
385391
{topickey.TopicKeyMerge, "merge", "orchestrator-merge"},
@@ -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

submitqueue/core/topickey/topickey.go

Lines changed: 8 additions & 0 deletions
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

submitqueue/entity/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ go_library(
1414
"land_request.go",
1515
"merge_result.go",
1616
"push_result.go",
17+
"queue.go",
1718
"queue_config.go",
1819
"request.go",
1920
"request_log.go",
@@ -36,6 +37,7 @@ go_test(
3637
"build_test.go",
3738
"cancel_request_test.go",
3839
"land_request_test.go",
40+
"queue_test.go",
3941
"request_log_test.go",
4042
"request_test.go",
4143
"speculation_tree_test.go",

submitqueue/entity/queue.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package entity
16+
17+
import "encoding/json"
18+
19+
// QueueID is the queue-message payload for queue-scoped pipeline stages. It
20+
// carries only the queue name.
21+
type QueueID struct {
22+
// Name is the merge-queue name the message targets.
23+
Name string `json:"name"`
24+
}
25+
26+
// ToBytes serializes the QueueID to JSON bytes for queue message payload.
27+
func (q QueueID) ToBytes() ([]byte, error) {
28+
return json.Marshal(q)
29+
}
30+
31+
// QueueIDFromBytes deserializes a QueueID from JSON bytes.
32+
func QueueIDFromBytes(data []byte) (QueueID, error) {
33+
var qid QueueID
34+
err := json.Unmarshal(data, &qid)
35+
return qid, err
36+
}

submitqueue/entity/queue_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package entity
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
22+
)
23+
24+
func TestQueueID_SerializationRoundTrip(t *testing.T) {
25+
tests := []struct {
26+
name string
27+
queueID QueueID
28+
}{
29+
{
30+
name: "simple queue name",
31+
queueID: QueueID{Name: "queueA"},
32+
},
33+
{
34+
name: "another queue name",
35+
queueID: QueueID{Name: "queueB"},
36+
},
37+
}
38+
39+
for _, tt := range tests {
40+
t.Run(tt.name, func(t *testing.T) {
41+
data, err := tt.queueID.ToBytes()
42+
require.NoError(t, err)
43+
44+
deserialized, err := QueueIDFromBytes(data)
45+
require.NoError(t, err)
46+
47+
assert.Equal(t, tt.queueID, deserialized)
48+
})
49+
}
50+
}
51+
52+
func TestQueueIDFromBytes_InvalidJSON(t *testing.T) {
53+
_, err := QueueIDFromBytes([]byte(`{"invalid": json"}`))
54+
assert.Error(t, err)
55+
}
56+
57+
func TestQueueIDFromBytes_EmptyJSON(t *testing.T) {
58+
queueID, err := QueueIDFromBytes([]byte(`{}`))
59+
require.NoError(t, err)
60+
61+
assert.Empty(t, queueID.Name)
62+
}
63+
64+
func TestQueueIDFromBytes_EmptyBytes(t *testing.T) {
65+
_, err := QueueIDFromBytes([]byte{})
66+
assert.Error(t, err)
67+
}
68+
69+
func TestQueueIDFromBytes_NilBytes(t *testing.T) {
70+
_, err := QueueIDFromBytes(nil)
71+
assert.Error(t, err)
72+
}

0 commit comments

Comments
 (0)