Skip to content

Commit 39f1572

Browse files
committed
feat(stovepipe): persist repairs with queue-scoped discovery
1 parent 39202b1 commit 39f1572

26 files changed

Lines changed: 775 additions & 56 deletions

‎doc/rfc/stovepipe/fixer.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,4 @@ The fixer adds one table and uses primary-key operations. Build-analysis history
8585

8686
Both queue controllers follow [`consumer.Controller`](../../../platform/consumer/controller.go): `Process(context.Context, consumer.Delivery) error`, `Name`, `TopicKey`, and `ConsumerGroup`. Service wiring resolves per-queue evaluators/executors, registers fix/fixsignal and their recovery behavior, and supplies host integrations.
8787

88-
The API layer defines the contracts; the persistence layer supplies MySQL support. Runtime controller wiring and provider adapters follow as separate changes. Initial dispatch can run before complete recovery reconciliation, while leaving completion awaiting verification.
88+
The API layer defines the contracts; this persistence layer supplies MySQL support. Runtime controller wiring and provider adapters follow as separate changes. Initial dispatch can run before complete recovery reconciliation, while leaving completion awaiting verification.

‎stovepipe/extension/storage/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,4 @@ Resolution belongs to a later controller: a successful, authoritative final anal
5151

5252
`OpenBuildIssue.FixID` is the authoritative assignment for its opening. Updates replace last-observed build and assignment together under the opening/version guards. Analysis preserves assignments; fixer preserves observation fields. Resolution history and recovery evidence remain owned by build analysis; deleting a current row alone supplies no validating build reference.
5353

54-
The [repair RFC](../../../doc/rfc/stovepipe/fixer.md) defines persistence, publication, and observation ordering. Persistence implementations follow in the storage layer.
54+
The [repair RFC](../../../doc/rfc/stovepipe/fixer.md) defines persistence, publication, and observation ordering. MySQL persists these fields through complete, version-guarded snapshots. Before deployment, add/backfill `build.first_fix_id`, `open_build_issue.fix_id`, and `queue.active_fix_ids`; fresh schemas include them.

‎stovepipe/extension/storage/mock/storage_mock.go‎

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

‎stovepipe/extension/storage/mysql/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go_library(
55
srcs = [
66
"build_issue_store.go",
77
"build_store.go",
8+
"fix_store.go",
89
"open_build_issue_store.go",
910
"queue_log_store.go",
1011
"queue_store.go",
@@ -33,6 +34,7 @@ go_test(
3334
srcs = [
3435
"build_issue_store_test.go",
3536
"build_store_test.go",
37+
"fix_store_test.go",
3638
"open_build_issue_store_test.go",
3739
"queue_store_test.go",
3840
"queue_summary_store_test.go",

‎stovepipe/extension/storage/mysql/build_store.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@ func (b *buildStore) Create(ctx context.Context, build entity.Build) (retErr err
4646
defer func() { op.Complete(retErr) }()
4747

4848
_, err := b.db.ExecContext(ctx,
49-
`INSERT INTO build (queue, id, request_id, status, version)
50-
VALUES (?, ?, ?, ?, ?)`,
49+
`INSERT INTO build (queue, id, request_id, status, first_fix_id, version)
50+
VALUES (?, ?, ?, ?, ?, ?)`,
5151
b.queue,
5252
build.ID,
5353
build.RequestID,
5454
build.Status,
55+
build.FirstFixID,
5556
build.Version,
5657
)
5758
if err != nil {
@@ -71,13 +72,14 @@ func (b *buildStore) Get(ctx context.Context, id string) (ret entity.Build, retE
7172

7273
var build entity.Build
7374
err := b.db.QueryRowContext(ctx,
74-
`SELECT id, request_id, status, version
75+
`SELECT id, request_id, status, first_fix_id, version
7576
FROM build WHERE queue = ? AND id = ?`,
7677
b.queue, id,
7778
).Scan(
7879
&build.ID,
7980
&build.RequestID,
8081
&build.Status,
82+
&build.FirstFixID,
8183
&build.Version,
8284
)
8385

@@ -91,7 +93,7 @@ func (b *buildStore) Get(ctx context.Context, id string) (ret entity.Build, retE
9193
return build, nil
9294
}
9395

94-
// Update persists the mutable fields of build (status) if the stored version matches
96+
// Update persists the mutable build snapshot if the stored version matches
9597
// oldVersion, writing newVersion. Returns ErrVersionMismatch if the stored version does not
9698
// match (including when the build does not exist). This is a pure conditional write; the
9799
// caller owns version arithmetic.
@@ -101,9 +103,10 @@ func (b *buildStore) Update(ctx context.Context, build entity.Build, oldVersion,
101103

102104
result, err := b.db.ExecContext(ctx,
103105
`UPDATE build
104-
SET status = ?, version = ?
106+
SET status = ?, first_fix_id = ?, version = ?
105107
WHERE queue = ? AND id = ? AND version = ?`,
106108
build.Status,
109+
build.FirstFixID,
107110
newVersion,
108111
b.queue,
109112
build.ID,

‎stovepipe/extension/storage/mysql/build_store_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ func TestBuildStore_Create(t *testing.T) {
5858
name: "success",
5959
setup: func(mock sqlmock.Sqlmock) {
6060
mock.ExpectExec("INSERT INTO build").
61-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
61+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.FirstFixID, build.Version).
6262
WillReturnResult(sqlmock.NewResult(0, 1))
6363
},
6464
},
6565
{
6666
name: "duplicate id returns ErrAlreadyExists",
6767
setup: func(mock sqlmock.Sqlmock) {
6868
mock.ExpectExec("INSERT INTO build").
69-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
69+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.FirstFixID, build.Version).
7070
WillReturnError(&mysql.MySQLError{Number: mysqlErrDuplicateEntry})
7171
},
7272
wantErr: true,
@@ -76,7 +76,7 @@ func TestBuildStore_Create(t *testing.T) {
7676
name: "other exec error",
7777
setup: func(mock sqlmock.Sqlmock) {
7878
mock.ExpectExec("INSERT INTO build").
79-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
79+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.FirstFixID, build.Version).
8080
WillReturnError(fmt.Errorf("connection reset"))
8181
},
8282
wantErr: true,
@@ -124,9 +124,9 @@ func TestBuildStore_Get(t *testing.T) {
124124
name: "found",
125125
id: want.ID,
126126
setup: func(mock sqlmock.Sqlmock) {
127-
rows := sqlmock.NewRows([]string{"id", "request_id", "status", "version"}).
128-
AddRow(want.ID, want.RequestID, string(want.Status), want.Version)
129-
mock.ExpectQuery("SELECT id, request_id, status, version").
127+
rows := sqlmock.NewRows([]string{"id", "request_id", "status", "first_fix_id", "version"}).
128+
AddRow(want.ID, want.RequestID, string(want.Status), want.FirstFixID, want.Version)
129+
mock.ExpectQuery("SELECT id, request_id, status, first_fix_id, version").
130130
WithArgs("monorepo/main", want.ID).
131131
WillReturnRows(rows)
132132
},
@@ -136,7 +136,7 @@ func TestBuildStore_Get(t *testing.T) {
136136
name: "not found",
137137
id: "missing",
138138
setup: func(mock sqlmock.Sqlmock) {
139-
mock.ExpectQuery("SELECT id, request_id, status, version").
139+
mock.ExpectQuery("SELECT id, request_id, status, first_fix_id, version").
140140
WithArgs("monorepo/main", "missing").
141141
WillReturnError(sql.ErrNoRows)
142142
},
@@ -147,7 +147,7 @@ func TestBuildStore_Get(t *testing.T) {
147147
name: "query error",
148148
id: "bad",
149149
setup: func(mock sqlmock.Sqlmock) {
150-
mock.ExpectQuery("SELECT id, request_id, status, version").
150+
mock.ExpectQuery("SELECT id, request_id, status, first_fix_id, version").
151151
WithArgs("monorepo/main", "bad").
152152
WillReturnError(fmt.Errorf("connection reset"))
153153
},
@@ -191,15 +191,15 @@ func TestBuildStore_Update(t *testing.T) {
191191
name: "success",
192192
setup: func(mock sqlmock.Sqlmock) {
193193
mock.ExpectExec("UPDATE build").
194-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
194+
WithArgs(build.Status, build.FirstFixID, newVersion, "monorepo/main", build.ID, oldVersion).
195195
WillReturnResult(sqlmock.NewResult(0, 1))
196196
},
197197
},
198198
{
199199
name: "version mismatch",
200200
setup: func(mock sqlmock.Sqlmock) {
201201
mock.ExpectExec("UPDATE build").
202-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
202+
WithArgs(build.Status, build.FirstFixID, newVersion, "monorepo/main", build.ID, oldVersion).
203203
WillReturnResult(sqlmock.NewResult(0, 0))
204204
},
205205
wantErr: true,
@@ -209,7 +209,7 @@ func TestBuildStore_Update(t *testing.T) {
209209
name: "exec error",
210210
setup: func(mock sqlmock.Sqlmock) {
211211
mock.ExpectExec("UPDATE build").
212-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
212+
WithArgs(build.Status, build.FirstFixID, newVersion, "monorepo/main", build.ID, oldVersion).
213213
WillReturnError(fmt.Errorf("connection reset"))
214214
},
215215
wantErr: true,
@@ -218,7 +218,7 @@ func TestBuildStore_Update(t *testing.T) {
218218
name: "rows affected error",
219219
setup: func(mock sqlmock.Sqlmock) {
220220
mock.ExpectExec("UPDATE build").
221-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
221+
WithArgs(build.Status, build.FirstFixID, newVersion, "monorepo/main", build.ID, oldVersion).
222222
WillReturnResult(sqlmock.NewErrorResult(fmt.Errorf("driver error")))
223223
},
224224
wantErr: true,
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// Copyright (c) 2026 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 mysql
16+
17+
import (
18+
"context"
19+
"database/sql"
20+
"encoding/json"
21+
"errors"
22+
"fmt"
23+
24+
"github.com/uber-go/tally"
25+
"github.com/uber/submitqueue/platform/metrics"
26+
"github.com/uber/submitqueue/stovepipe/entity"
27+
"github.com/uber/submitqueue/stovepipe/extension/storage"
28+
)
29+
30+
type fixStore struct {
31+
db *sql.DB
32+
scope tally.Scope
33+
queue string
34+
}
35+
36+
// NewFixStore creates a MySQL-backed, queue-scoped FixStore.
37+
func NewFixStore(db *sql.DB, scope tally.Scope, queue string) storage.FixStore {
38+
return &fixStore{db: db, scope: scope, queue: queue}
39+
}
40+
41+
func (s *fixStore) Create(ctx context.Context, fix entity.Fix) (retErr error) {
42+
op := metrics.Begin(s.scope, "create", metrics.StorageLatencyBuckets)
43+
defer func() { op.Complete(retErr) }()
44+
45+
if fix.Queue != s.queue || fix.ID == "" {
46+
return fmt.Errorf("repair requires a nonempty ID and queue matching %q", s.queue)
47+
}
48+
issuesJSON, metadataJSON, err := marshalFixJSON(fix)
49+
if err != nil {
50+
return fmt.Errorf("failed to marshal repair data: %w", err)
51+
}
52+
53+
_, err = s.db.ExecContext(ctx,
54+
`INSERT INTO fix (queue, id, build_id, state, issues, execution_ref, metadata, created_at, updated_at, version)
55+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
56+
s.queue,
57+
fix.ID,
58+
fix.BuildID,
59+
fix.State,
60+
issuesJSON,
61+
fix.ExecutionRef,
62+
metadataJSON,
63+
fix.CreatedAt,
64+
fix.UpdatedAt,
65+
fix.Version,
66+
)
67+
if err != nil {
68+
if isDuplicateEntry(err) {
69+
return fmt.Errorf("fix queue=%q: %w", s.queue, storage.ErrAlreadyExists)
70+
}
71+
return fmt.Errorf("failed to insert fix queue=%q: %w", s.queue, err)
72+
}
73+
return nil
74+
}
75+
76+
func (s *fixStore) Get(ctx context.Context, id string) (ret entity.Fix, retErr error) {
77+
op := metrics.Begin(s.scope, "get", metrics.StorageLatencyBuckets)
78+
defer func() { op.Complete(retErr) }()
79+
80+
var fix entity.Fix
81+
fix.Queue = s.queue
82+
var issuesJSON []byte
83+
var metadataJSON []byte
84+
err := s.db.QueryRowContext(ctx,
85+
`SELECT id, build_id, state, issues, execution_ref, metadata, created_at, updated_at, version FROM fix
86+
WHERE queue = ? AND id = ?`,
87+
s.queue,
88+
id,
89+
).Scan(
90+
&fix.ID,
91+
&fix.BuildID,
92+
&fix.State,
93+
&issuesJSON,
94+
&fix.ExecutionRef,
95+
&metadataJSON,
96+
&fix.CreatedAt,
97+
&fix.UpdatedAt,
98+
&fix.Version,
99+
)
100+
if errors.Is(err, sql.ErrNoRows) {
101+
return entity.Fix{}, storage.WrapNotFound(err)
102+
}
103+
if err != nil {
104+
return entity.Fix{}, fmt.Errorf("failed to get fix queue=%q: %w", s.queue, err)
105+
}
106+
if err := json.Unmarshal(issuesJSON, &fix.Issues); err != nil {
107+
return entity.Fix{}, fmt.Errorf("failed to unmarshal repair issues: %w", err)
108+
}
109+
if err := json.Unmarshal(metadataJSON, &fix.Metadata); err != nil {
110+
return entity.Fix{}, fmt.Errorf("failed to unmarshal repair metadata: %w", err)
111+
}
112+
if fix.Metadata == nil {
113+
fix.Metadata = map[string]string{}
114+
}
115+
return fix, nil
116+
}
117+
118+
func (s *fixStore) Update(ctx context.Context, fix entity.Fix, oldVersion, newVersion int32) (retErr error) {
119+
op := metrics.Begin(s.scope, "update", metrics.StorageLatencyBuckets)
120+
defer func() { op.Complete(retErr) }()
121+
122+
if fix.Queue != s.queue || fix.ID == "" {
123+
return fmt.Errorf("repair requires a nonempty ID and queue matching %q", s.queue)
124+
}
125+
issuesJSON, metadataJSON, err := marshalFixJSON(fix)
126+
if err != nil {
127+
return fmt.Errorf("failed to marshal repair data: %w", err)
128+
}
129+
130+
result, err := s.db.ExecContext(ctx,
131+
`UPDATE fix SET build_id = ?, state = ?, issues = ?, execution_ref = ?, metadata = ?, created_at = ?, updated_at = ?, version = ?
132+
WHERE queue = ? AND id = ? AND version = ?`,
133+
fix.BuildID,
134+
fix.State,
135+
issuesJSON,
136+
fix.ExecutionRef,
137+
metadataJSON,
138+
fix.CreatedAt,
139+
fix.UpdatedAt,
140+
newVersion,
141+
s.queue,
142+
fix.ID,
143+
oldVersion,
144+
)
145+
if err != nil {
146+
return fmt.Errorf("failed to update fix queue=%q: %w", s.queue, err)
147+
}
148+
rowsAffected, err := result.RowsAffected()
149+
if err != nil {
150+
return fmt.Errorf("failed to get rows affected for fix queue=%q: %w", s.queue, err)
151+
}
152+
if rowsAffected != 1 {
153+
return fmt.Errorf("version mismatch for fix queue=%q expected_version=%d: %w", s.queue, oldVersion, storage.ErrVersionMismatch)
154+
}
155+
return nil
156+
}
157+
158+
func marshalFixJSON(fix entity.Fix) ([]byte, []byte, error) {
159+
issuesJSON, err := json.Marshal(fix.Issues)
160+
if err != nil {
161+
return nil, nil, err
162+
}
163+
metadata := fix.Metadata
164+
if metadata == nil {
165+
metadata = map[string]string{}
166+
}
167+
metadataJSON, err := json.Marshal(metadata)
168+
if err != nil {
169+
return nil, nil, err
170+
}
171+
return issuesJSON, metadataJSON, nil
172+
}

0 commit comments

Comments
 (0)