Skip to content

Commit 9971247

Browse files
committed
feat(entity,storage): rework speculation tree model and store
Reshape the speculation tree data model around the Base/Head path that the build stage actually consumes, and align its store with the speculation design. entity: SpeculationPath{Base, Head} becomes the unit; SpeculationPathInfo carries the path plus its Score, controller-owned Status (candidate/selected/prioritized/building/passed/failed/cancelling/cancelled), and BuildID — Path is immutable once persisted, Score/Status/BuildID are updateable by the controller. Score is scorer-computed and controller-persisted dynamic state — recomputed on every respeculate, not set at enumeration. Selected/Prioritized and Cancelling/Cancelled split desire vs budget-cleared and cancel-intent vs terminal, so the persisted status carries the cross-stage contract. Adds SpeculationPathAction (Promote/Cancel) and SpeculationPathDecision for the selector and prioritizer seams. SpeculationTree.Speculations becomes Paths, and the tree gains a Version for optimistic locking (starts at 1, incremented per change). The Build entity uses the shared SpeculationPath (Head = the batch under verification) and drops its own Score, which was never populated or read. storage: SpeculationTreeStore.UpdateSpeculations(batchID, []SpeculationInfo) becomes a pure conditional write — Update(ctx, batchID, oldVersion, newVersion, paths) — returning ErrVersionMismatch when the persisted version does not match, per the repo's optimistic-locking pattern (version arithmetic owned by the controller). The MySQL impl treats rowsAffected == 1 as success, 0 as version mismatch, and anything else as a generic error. The MySQL column speculations is renamed paths and the schema gains a version column. MySQL impl, mock, and storage integration coverage (create/get round-trip, duplicate, versioned whole-tree overwrite, stale-write rejection, not-found) added/updated.
1 parent e1b7c7b commit 9971247

11 files changed

Lines changed: 319 additions & 77 deletions

File tree

submitqueue/entity/build.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,6 @@ func (s BuildStatus) IsTerminal() bool {
5353
return s == BuildStatusSucceeded || s == BuildStatusFailed || s == BuildStatusCancelled
5454
}
5555

56-
// SpeculationPathInfo represents the base and head commits of a speculation path used in a build.
57-
type SpeculationPathInfo struct {
58-
// Base is a list of batchIDs(in order) that form the base of this speculation path.
59-
Base []string
60-
}
61-
6256
// Build represents a build scheduled for a batch along a specific speculation path.
6357
// All fields except the Status are immutable after creation.
6458
type Build struct {
@@ -69,10 +63,9 @@ type Build struct {
6963
BatchID string
7064
// SpeculationPath is the speculation path that represents this build. For
7165
// a given batch this path is crafted from the graph that is generated from the
72-
// dependencies of this batch.
73-
SpeculationPath SpeculationPathInfo
74-
// Score represents the build prediction score for this speculation path.
75-
Score float32
66+
// dependencies of this batch. Its Head is the batch being verified (equal to
67+
// BatchID) and its Base is the assumed-good prefix of predecessor batches.
68+
SpeculationPath SpeculationPath
7669
// Status represents the state of the build lifecycle this build is in.
7770
Status BuildStatus
7871
}

submitqueue/entity/build_test.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ func TestBuild_ToBytes(t *testing.T) {
7070
build := Build{
7171
ID: "build-1",
7272
BatchID: "batch-1",
73-
SpeculationPath: SpeculationPathInfo{
73+
SpeculationPath: SpeculationPath{
7474
Base: []string{"batch-0", "batch-prev"},
75+
Head: "batch-1",
7576
},
76-
Score: 0.85,
7777
Status: BuildStatusAccepted,
7878
}
7979

@@ -92,10 +92,10 @@ func TestBuildFromBytes(t *testing.T) {
9292
original := Build{
9393
ID: "build-42",
9494
BatchID: "batch-7",
95-
SpeculationPath: SpeculationPathInfo{
95+
SpeculationPath: SpeculationPath{
9696
Base: []string{"batch-5", "batch-6"},
97+
Head: "batch-7",
9798
},
98-
Score: 0.92,
9999
Status: BuildStatusAccepted,
100100
}
101101

@@ -111,7 +111,6 @@ func TestBuildFromBytes(t *testing.T) {
111111
assert.Equal(t, original.ID, deserialized.ID)
112112
assert.Equal(t, original.BatchID, deserialized.BatchID)
113113
assert.Equal(t, original.SpeculationPath.Base, deserialized.SpeculationPath.Base)
114-
assert.Equal(t, original.Score, deserialized.Score)
115114
assert.Equal(t, original.Status, deserialized.Status)
116115
}
117116

@@ -132,7 +131,6 @@ func TestBuildFromBytes_EmptyData(t *testing.T) {
132131
assert.Empty(t, build.ID)
133132
assert.Empty(t, build.BatchID)
134133
assert.Equal(t, BuildStatusUnknown, build.Status)
135-
assert.Equal(t, float32(0), build.Score)
136134
}
137135

138136
func TestBuild_SerializationRoundTrip(t *testing.T) {
@@ -145,10 +143,10 @@ func TestBuild_SerializationRoundTrip(t *testing.T) {
145143
build: Build{
146144
ID: "build-100",
147145
BatchID: "batch-50",
148-
SpeculationPath: SpeculationPathInfo{
146+
SpeculationPath: SpeculationPath{
149147
Base: []string{"batch-48", "batch-49"},
148+
Head: "batch-50",
150149
},
151-
Score: 0.75,
152150
Status: BuildStatusAccepted,
153151
},
154152
},
@@ -157,19 +155,18 @@ func TestBuild_SerializationRoundTrip(t *testing.T) {
157155
build: Build{
158156
ID: "build-200",
159157
BatchID: "batch-60",
160-
Score: 1.0,
161158
Status: BuildStatusSucceeded,
162159
},
163160
},
164161
{
165-
name: "failed build with zero score",
162+
name: "failed build",
166163
build: Build{
167164
ID: "build-300",
168165
BatchID: "batch-70",
169-
SpeculationPath: SpeculationPathInfo{
166+
SpeculationPath: SpeculationPath{
170167
Base: []string{"batch-65"},
168+
Head: "batch-70",
171169
},
172-
Score: 0,
173170
Status: BuildStatusFailed,
174171
},
175172
},

submitqueue/entity/speculation_tree.go

Lines changed: 131 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,38 +14,151 @@
1414

1515
package entity
1616

17-
// SpeculationPathAction defines the possible actions for a speculation path.
17+
// SpeculationPath is a single speculation path: an assumed-good prefix of
18+
// predecessor batches (Base) on top of which the batch under verification
19+
// (Head) is built and validated.
20+
//
21+
// This is the unit the build stage consumes: Base maps to the build runner's
22+
// base changes (an assumed-good prefix to apply) and Head maps to the changes
23+
// being validated.
24+
type SpeculationPath struct {
25+
// Base is the ordered list of predecessor batch IDs assumed to have passed.
26+
// Empty means the path builds the head batch directly on the target branch.
27+
Base []string
28+
// Head is the batch ID being verified by this path.
29+
Head string
30+
}
31+
32+
// SpeculationPathStatus is the observed lifecycle state of a speculation path.
33+
// It is written only by the orchestrator's speculate controller (into the
34+
// speculation tree store) and read by the decision seams (selector, prioritizer)
35+
// as input; the seams never write it.
36+
type SpeculationPathStatus string
37+
38+
const (
39+
// SpeculationPathStatusUnknown is the unreachable zero value, set by default
40+
// on init. A persisted path always carries a real status (candidate onward),
41+
// so this should never be seen in the store.
42+
SpeculationPathStatusUnknown SpeculationPathStatus = ""
43+
// SpeculationPathStatusCandidate is a freshly enumerated path the controller
44+
// has persisted but not yet acted on.
45+
SpeculationPathStatusCandidate SpeculationPathStatus = "candidate"
46+
// SpeculationPathStatusSelected is a path the selector has promoted — a
47+
// per-batch desire to build it — that the queue-wide prioritizer has not yet
48+
// cleared. It is not sent to build while Selected: it waits on the build
49+
// budget until it is prioritized (or dropped).
50+
SpeculationPathStatusSelected SpeculationPathStatus = "selected"
51+
// SpeculationPathStatusPrioritized is a path the prioritizer has admitted
52+
// under the queue's build budget; it is cleared to run. The build effector
53+
// triggers only Prioritized paths.
54+
SpeculationPathStatusPrioritized SpeculationPathStatus = "prioritized"
55+
// SpeculationPathStatusBuilding is a path a build signal has confirmed is in
56+
// flight; its BuildID is known.
57+
SpeculationPathStatusBuilding SpeculationPathStatus = "building"
58+
// SpeculationPathStatusPassed is a path whose build succeeded.
59+
SpeculationPathStatusPassed SpeculationPathStatus = "passed"
60+
// SpeculationPathStatusFailed is a path whose build failed.
61+
SpeculationPathStatusFailed SpeculationPathStatus = "failed"
62+
// SpeculationPathStatusCancelling is a path whose in-flight build the
63+
// controller has asked to stop but whose cancellation is not yet confirmed. It
64+
// mirrors the batch-level cancelling intent (BatchStateCancelling): the cancel
65+
// decision is recorded here and the effector drives it to terminal Cancelled.
66+
// A path with no build in flight is dropped straight to Cancelled instead.
67+
SpeculationPathStatusCancelling SpeculationPathStatus = "cancelling"
68+
// SpeculationPathStatusCancelled is the terminal state for a path that is no
69+
// longer pursued — its in-flight build was confirmed stopped, or the path was
70+
// abandoned before a build started.
71+
SpeculationPathStatusCancelled SpeculationPathStatus = "cancelled"
72+
)
73+
74+
// SpeculationPathAction is the decision a seam (the selector or the prioritizer)
75+
// asks the controller to take for a path. It names the decision, not its effect:
76+
// it is ephemeral (recomputed every time a seam runs) and never persisted. The
77+
// controller maps it to the corresponding SpeculationPathStatus transition —
78+
// applied under the tree's optimistic lock (Version) — and records the result;
79+
// the seams never write status themselves.
1880
type SpeculationPathAction string
1981

2082
const (
21-
// SpeculationPathActionUnknown is the default zero value for SpeculationPathAction.
83+
// SpeculationPathActionUnknown is the unreachable zero value. A seam
84+
// expresses "leave this path as-is" by omitting the path from its decisions,
85+
// not by returning this.
2286
SpeculationPathActionUnknown SpeculationPathAction = ""
23-
// TODO: Add comprehensive list of actions
87+
// SpeculationPathActionPromote asks the controller to advance this path one
88+
// stage toward running. The target status depends on which seam decided it:
89+
// the selector's promote moves a path to Selected; the prioritizer's promote
90+
// moves it to Prioritized (cleared to build).
91+
SpeculationPathActionPromote SpeculationPathAction = "promote"
92+
// SpeculationPathActionCancel asks the controller to stop pursuing this path:
93+
// it moves to Cancelling if a build is in flight (the effector then confirms
94+
// the stop and drives it to terminal Cancelled), or straight to Cancelled if
95+
// no build has started.
96+
SpeculationPathActionCancel SpeculationPathAction = "cancel"
2497
)
2598

26-
// SpeculationInfo represents metadata about a single speculation path, including the path through the dependency graph, its current state, and the predicted build score.
27-
type SpeculationInfo struct {
28-
// Path represents the speculation path; which is an ordered list of batches.
29-
Path []string
30-
// Action is a state that this path is in.
31-
Action SpeculationPathAction
32-
// Score is score for this speculation path.
99+
// SpeculationPathInfo is the per-path entry in a speculation tree: a path, its
100+
// latest predicted-success score, its controller-owned status, and a link to
101+
// the build dispatched for it (if any). Path is immutable once the entry is
102+
// persisted; Score, Status, and BuildID are updateable, written only by the
103+
// controller under the tree's Version optimistic lock.
104+
type SpeculationPathInfo struct {
105+
// Path is the Base/Head split this entry covers. Immutable: it identifies
106+
// the entry and never changes after the path is first persisted.
107+
Path SpeculationPath
108+
// Score is the path's predicted-success score. Updateable: it is computed by
109+
// the scorer and persisted by the controller, not set at enumeration — the
110+
// enumerator produces structure only. It is dynamic: the controller re-runs
111+
// the scorer on every respeculate (as dependencies land, dependency builds
112+
// pass, or sibling paths fail), so the value tracks the latest state rather
113+
// than a figure frozen when the path was first enumerated (~0 until the
114+
// first pass).
33115
Score float32
116+
// Status is the observed lifecycle state of the path. Updateable: written
117+
// only by the controller; read by the decision seams (scorer, selector,
118+
// prioritizer).
119+
Status SpeculationPathStatus
120+
// BuildID links this path to its build. Updateable: empty until a build
121+
// signal confirms the build and the controller records it (Prioritized ->
122+
// Building); the controller never knows the ID at send time.
123+
BuildID string
124+
}
125+
126+
// SpeculationPathDecision is a seam's decision for a single path: the action the
127+
// controller should take for it. It is the output of both the selector (per
128+
// batch) and the prioritizer (queue-wide), and is not persisted. A seam returns
129+
// a decision only for the paths it wants to act on; omitted paths are left
130+
// as-is.
131+
type SpeculationPathDecision struct {
132+
// Path identifies the speculation path the action applies to.
133+
Path SpeculationPath
134+
// Action is what the controller should do for the path.
135+
Action SpeculationPathAction
34136
}
35137

36-
// SpeculationTree represents the set of speculation paths constructed for a batch based on its dependency graph.
138+
// SpeculationTree is the set of candidate speculation paths for a batch, built
139+
// from its dependency graph. BatchID is immutable; Paths is updateable (the
140+
// controller overwrites it wholesale on every respeculate), guarded by Version.
37141
type SpeculationTree struct {
38142
// BatchID is the batch for which this speculation tree is constructed.
143+
// Immutable: it identifies the tree.
39144
BatchID string
40-
// Speculations is a list of speculation paths for this batch based on a graph of its
41-
// dependents.
145+
// Paths is the candidate speculation paths for this batch, derived from a
146+
// graph of its dependencies. Each entry's per-path dynamic state (Score,
147+
// Status, BuildID) is documented on SpeculationPathInfo.
42148
//
43149
// For e.g - Consider batches - queueA/batch/1, queueA/batch/2, queueA/batch/3
44-
// such that - queueA/batch/2 and queueA/batch/3 depend on queueA/batch/1
150+
// such that - queueA/batch/2 and queueA/batch/3 depend on queueA/batch/1.
151+
// Each dependent batch gets two paths: build alone, or build on the
152+
// assumed-good predecessor. Just after enumeration every path is a candidate:
45153
//
46-
// Speculations for queueA/batch/1 - [{Path: []string{"queueA/batch/1"}, State: "scheduled", Score: 0.1}]
47-
// Speculations for queueA/batch/2 - [{Path: []string{"queueA/batch/2"}, State: "scheduled", Score: 0.9}, {Path: []string{"queueA/batch/1", "queueA/batch/2"}, State: "scheduled", Score: 0.3}]
48-
// Speculations for queueA/batch/3 - [{Path: []string{"queueA/batch/3"}, State: "scheduled", Score: 0.9}, {Path: []string{"queueA/batch/1", "queueA/batch/3"}, State: "scheduled", Score: 0.3}]
154+
// Paths for queueA/batch/2 - [{Path: {Base: [], Head: "queueA/batch/2"}, Status: "candidate"}, {Path: {Base: ["queueA/batch/1"], Head: "queueA/batch/2"}, Status: "candidate"}]
155+
// Paths for queueA/batch/3 - [{Path: {Base: [], Head: "queueA/batch/3"}, Status: "candidate"}, {Path: {Base: ["queueA/batch/1"], Head: "queueA/batch/3"}, Status: "candidate"}]
49156
//
50-
Speculations []SpeculationInfo
157+
Paths []SpeculationPathInfo
158+
// Version is the version of the object. It is used for optimistic locking:
159+
// updates are conditional on the persisted version matching the caller's
160+
// expected version. Versioning starts at 1 and is incremented for each
161+
// change to the object; version arithmetic is owned by the controller, the
162+
// store performs a pure conditional write.
163+
Version int32
51164
}

submitqueue/extension/storage/mock/speculation_tree_store_mock.go

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

submitqueue/extension/storage/mysql/build_store.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ func (s *buildStore) Get(ctx context.Context, id string) (ret entity.Build, retE
4848
var speculationPathJSON []byte
4949

5050
err := s.db.QueryRowContext(ctx,
51-
"SELECT id, batch_id, speculation_path, score, status FROM build WHERE id = ?",
51+
"SELECT id, batch_id, speculation_path, status FROM build WHERE id = ?",
5252
id,
53-
).Scan(&build.ID, &build.BatchID, &speculationPathJSON, &build.Score, &build.Status)
53+
).Scan(&build.ID, &build.BatchID, &speculationPathJSON, &build.Status)
5454

5555
if errors.Is(err, sql.ErrNoRows) {
5656
return entity.Build{}, storage.WrapNotFound(err)
@@ -77,8 +77,8 @@ func (s *buildStore) Create(ctx context.Context, build entity.Build) (retErr err
7777
}
7878

7979
_, err = s.db.ExecContext(ctx,
80-
"INSERT INTO build (id, batch_id, speculation_path, score, status) VALUES (?, ?, ?, ?, ?)",
81-
build.ID, build.BatchID, speculationPathJSON, build.Score, build.Status,
80+
"INSERT INTO build (id, batch_id, speculation_path, status) VALUES (?, ?, ?, ?)",
81+
build.ID, build.BatchID, speculationPathJSON, build.Status,
8282
)
8383
if err != nil {
8484
var mysqlErr *mysql.MySQLError

submitqueue/extension/storage/mysql/schema/build.sql

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ CREATE TABLE IF NOT EXISTS build (
22
id VARCHAR(255) NOT NULL,
33
batch_id VARCHAR(255) NOT NULL,
44
speculation_path JSON NOT NULL,
5-
score FLOAT NOT NULL,
65
status VARCHAR(64) NOT NULL,
76
PRIMARY KEY (id)
87
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
CREATE TABLE IF NOT EXISTS speculation_tree (
22
batch_id VARCHAR(255) NOT NULL,
3-
speculations JSON NOT NULL,
3+
paths JSON NOT NULL,
4+
version INT NOT NULL,
45
PRIMARY KEY (batch_id)
56
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

0 commit comments

Comments
 (0)