Skip to content

Commit 4606cc7

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/building/passed/failed/cancelled), and BuildID. Score is scorer-computed and controller-persisted dynamic state — recomputed on every respeculate, not set at enumeration. Adds SpeculationPathAction and SpeculationPathDecision for the selector seam. SpeculationTree.Speculations becomes Paths. The Build entity uses the shared SpeculationPath (Head = the batch under verification) and drops its own Score, which was never populated or read; the prioritizer seam re-adds a build priority when it lands. storage: SpeculationTreeStore.UpdateSpeculations(batchID, []SpeculationInfo) becomes Update(ctx, SpeculationTree) — symmetric with Create, keyed by tree.BatchID. The MySQL column speculations is renamed paths to match the entity field. MySQL impl, mock, and storage integration coverage (create/get round-trip, duplicate, whole-tree overwrite, not-found) added/updated.
1 parent e1b7c7b commit 4606cc7

11 files changed

Lines changed: 245 additions & 75 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: 100 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,38 +14,120 @@
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 path selector as input; enumerators
35+
// and selectors 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 sent to build.
45+
SpeculationPathStatusCandidate SpeculationPathStatus = "candidate"
46+
// SpeculationPathStatusSelected is a path the controller has sent to the build
47+
// controller (in response to a selector Build action) but for which no build
48+
// signal has arrived yet — the build system may not have started it
49+
// (resource-gated), so whether it is actually building is not yet known.
50+
SpeculationPathStatusSelected SpeculationPathStatus = "selected"
51+
// SpeculationPathStatusBuilding is a path a build signal has confirmed is in
52+
// flight; its BuildID is known.
53+
SpeculationPathStatusBuilding SpeculationPathStatus = "building"
54+
// SpeculationPathStatusPassed is a path whose build succeeded.
55+
SpeculationPathStatusPassed SpeculationPathStatus = "passed"
56+
// SpeculationPathStatusFailed is a path whose build failed.
57+
SpeculationPathStatusFailed SpeculationPathStatus = "failed"
58+
// SpeculationPathStatusCancelled is a path that is no longer pursued — its
59+
// base was invalidated, its build was cancelled, or the selector dropped it.
60+
SpeculationPathStatusCancelled SpeculationPathStatus = "cancelled"
61+
)
62+
63+
// SpeculationPathAction is the action a path selector asks the controller to
64+
// take for a path. It is the selector's only output: ephemeral (recomputed
65+
// every time the selector runs) and never persisted. The controller enacts it
66+
// and records the resulting SpeculationPathStatus.
1867
type SpeculationPathAction string
1968

2069
const (
21-
// SpeculationPathActionUnknown is the default zero value for SpeculationPathAction.
70+
// SpeculationPathActionUnknown is the unreachable zero value. A real decision
71+
// always carries Build or Cancel; the selector expresses "leave this path
72+
// as-is" by omitting it from its decisions, not by returning this.
2273
SpeculationPathActionUnknown SpeculationPathAction = ""
23-
// TODO: Add comprehensive list of actions
74+
// SpeculationPathActionBuild asks the controller to send this path to the
75+
// build controller (which triggers a build subject to resources). The path moves
76+
// to Selected on send, then Building once a build signal confirms it.
77+
SpeculationPathActionBuild SpeculationPathAction = "build"
78+
// SpeculationPathActionCancel asks the controller to drop this path and
79+
// cancel any build in flight for it.
80+
SpeculationPathActionCancel SpeculationPathAction = "cancel"
2481
)
2582

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.
83+
// SpeculationPathInfo is the per-path entry in a speculation tree: a path, its
84+
// latest predicted-success score, its controller-owned status, and a link to
85+
// the build dispatched for it (if any).
86+
type SpeculationPathInfo struct {
87+
// Path is the Base/Head split this entry covers.
88+
Path SpeculationPath
89+
// Score is the path's predicted-success score. It is computed by the scorer
90+
// and persisted by the controller, not set at enumeration — the enumerator
91+
// produces structure only. It is dynamic: the controller re-runs the scorer
92+
// on every respeculate (as dependencies land, dependency builds pass, or
93+
// sibling paths fail), so the value tracks the latest state rather than a
94+
// figure frozen when the path was first enumerated (~0 until the first pass).
3395
Score float32
96+
// Status is the observed lifecycle state of the path. Written only by the
97+
// controller; read by the selector.
98+
Status SpeculationPathStatus
99+
// BuildID links this path to its build. Empty until a build signal confirms
100+
// the build and the controller records it (Selected -> Building); the
101+
// controller never knows the ID at send time.
102+
BuildID string
103+
}
104+
105+
// SpeculationPathDecision is a path selector's decision for a single path: the
106+
// action the controller should take for it. It is the selector's output and is
107+
// not persisted.
108+
type SpeculationPathDecision struct {
109+
// Path identifies the speculation path the action applies to.
110+
Path SpeculationPath
111+
// Action is what the controller should do for the path.
112+
Action SpeculationPathAction
34113
}
35114

36-
// SpeculationTree represents the set of speculation paths constructed for a batch based on its dependency graph.
115+
// SpeculationTree is the set of candidate speculation paths for a batch, built
116+
// from its dependency graph.
37117
type SpeculationTree struct {
38118
// BatchID is the batch for which this speculation tree is constructed.
39119
BatchID string
40-
// Speculations is a list of speculation paths for this batch based on a graph of its
41-
// dependents.
120+
// Paths is the candidate speculation paths for this batch, derived from a
121+
// graph of its dependencies. Each entry's per-path dynamic state (Score,
122+
// Status, BuildID) is documented on SpeculationPathInfo.
42123
//
43124
// 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
125+
// such that - queueA/batch/2 and queueA/batch/3 depend on queueA/batch/1.
126+
// Each dependent batch gets two paths: build alone, or build on the
127+
// assumed-good predecessor. Just after enumeration every path is a candidate:
45128
//
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}]
129+
// Paths for queueA/batch/2 - [{Path: {Base: [], Head: "queueA/batch/2"}, Status: "candidate"}, {Path: {Base: ["queueA/batch/1"], Head: "queueA/batch/2"}, Status: "candidate"}]
130+
// Paths for queueA/batch/3 - [{Path: {Base: [], Head: "queueA/batch/3"}, Status: "candidate"}, {Path: {Base: ["queueA/batch/1"], Head: "queueA/batch/3"}, Status: "candidate"}]
49131
//
50-
Speculations []SpeculationInfo
132+
Paths []SpeculationPathInfo
51133
}

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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
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,
44
PRIMARY KEY (batch_id)
55
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

submitqueue/extension/storage/mysql/speculation_tree_store.go

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ func (s *speculationTreeStore) Get(ctx context.Context, batchID string) (ret ent
4545
defer func() { op.Complete(retErr) }()
4646

4747
var st entity.SpeculationTree
48-
var speculationsJSON []byte
48+
var pathsJSON []byte
4949

5050
err := s.db.QueryRowContext(ctx,
51-
"SELECT batch_id, speculations FROM speculation_tree WHERE batch_id = ?",
51+
"SELECT batch_id, paths FROM speculation_tree WHERE batch_id = ?",
5252
batchID,
53-
).Scan(&st.BatchID, &speculationsJSON)
53+
).Scan(&st.BatchID, &pathsJSON)
5454

5555
if errors.Is(err, sql.ErrNoRows) {
5656
return entity.SpeculationTree{}, storage.WrapNotFound(err)
@@ -59,8 +59,8 @@ func (s *speculationTreeStore) Get(ctx context.Context, batchID string) (ret ent
5959
return entity.SpeculationTree{}, fmt.Errorf("failed to get speculation tree entity batchID=%s from the database: %w", batchID, err)
6060
}
6161

62-
if err := json.Unmarshal(speculationsJSON, &st.Speculations); err != nil {
63-
return entity.SpeculationTree{}, fmt.Errorf("failed to unmarshal speculations for speculation tree entity batchID=%s from the database: %w", batchID, err)
62+
if err := json.Unmarshal(pathsJSON, &st.Paths); err != nil {
63+
return entity.SpeculationTree{}, fmt.Errorf("failed to unmarshal paths for speculation tree entity batchID=%s from the database: %w", batchID, err)
6464
}
6565

6666
return st, nil
@@ -71,14 +71,14 @@ func (s *speculationTreeStore) Create(ctx context.Context, speculationTree entit
7171
op := metrics.Begin(s.scope, "create")
7272
defer func() { op.Complete(retErr) }()
7373

74-
speculationsJSON, err := json.Marshal(speculationTree.Speculations)
74+
pathsJSON, err := json.Marshal(speculationTree.Paths)
7575
if err != nil {
76-
return fmt.Errorf("failed to marshal speculations batchID=%s for Create speculation tree entity: %w", speculationTree.BatchID, err)
76+
return fmt.Errorf("failed to marshal paths batchID=%s for Create speculation tree entity: %w", speculationTree.BatchID, err)
7777
}
7878

7979
_, err = s.db.ExecContext(ctx,
80-
"INSERT INTO speculation_tree (batch_id, speculations) VALUES (?, ?)",
81-
speculationTree.BatchID, speculationsJSON,
80+
"INSERT INTO speculation_tree (batch_id, paths) VALUES (?, ?)",
81+
speculationTree.BatchID, pathsJSON,
8282
)
8383
if err != nil {
8484
var mysqlErr *mysql.MySQLError
@@ -91,31 +91,32 @@ func (s *speculationTreeStore) Create(ctx context.Context, speculationTree entit
9191
return nil
9292
}
9393

94-
// UpdateSpeculations updates the speculations of a speculation tree. Returns ErrNotFound if the speculation tree is not found.
95-
func (s *speculationTreeStore) UpdateSpeculations(ctx context.Context, batchID string, speculations []entity.SpeculationInfo) (retErr error) {
96-
op := metrics.Begin(s.scope, "update_speculations")
94+
// Update overwrites the paths of an existing speculation tree, identified by
95+
// speculationTree.BatchID. Returns ErrNotFound if the speculation tree is not found.
96+
func (s *speculationTreeStore) Update(ctx context.Context, speculationTree entity.SpeculationTree) (retErr error) {
97+
op := metrics.Begin(s.scope, "update")
9798
defer func() { op.Complete(retErr) }()
9899

99-
speculationsJSON, err := json.Marshal(speculations)
100+
pathsJSON, err := json.Marshal(speculationTree.Paths)
100101
if err != nil {
101-
return fmt.Errorf("failed to marshal speculations batchID=%s for UpdateSpeculations: %w", batchID, err)
102+
return fmt.Errorf("failed to marshal paths batchID=%s for Update: %w", speculationTree.BatchID, err)
102103
}
103104

104105
result, err := s.db.ExecContext(ctx,
105-
"UPDATE speculation_tree SET speculations = ? WHERE batch_id = ?",
106-
speculationsJSON, batchID,
106+
"UPDATE speculation_tree SET paths = ? WHERE batch_id = ?",
107+
pathsJSON, speculationTree.BatchID,
107108
)
108109
if err != nil {
109-
return fmt.Errorf("failed to update speculations for batchID=%q: %w", batchID, err)
110+
return fmt.Errorf("failed to update speculation tree for batchID=%q: %w", speculationTree.BatchID, err)
110111
}
111112

112113
rowsAffected, err := result.RowsAffected()
113114
if err != nil {
114-
return fmt.Errorf("failed to get rows affected from update for batchID=%q: %w", batchID, err)
115+
return fmt.Errorf("failed to get rows affected from update for batchID=%q: %w", speculationTree.BatchID, err)
115116
}
116117

117118
if rowsAffected != 1 {
118-
return storage.WrapNotFound(fmt.Errorf("speculation tree entity batchID=%s", batchID))
119+
return storage.WrapNotFound(fmt.Errorf("speculation tree entity batchID=%s", speculationTree.BatchID))
119120
}
120121

121122
return nil

0 commit comments

Comments
 (0)