Skip to content

Commit f75843b

Browse files
authored
feat(stovepipe): List open issues in pages (#924)
## Summary **What**: - List currently open issues in bounded, cursor-based pages within each validation queue, preserving byte-exact task and unit ordering. - Reject open issues without a task identity while preserving task-wide issues with no unit. **Why**: - Enable issue reconciliation to discover existing open issues through primary-key reads without secondary indexes or loading every issue at once. ## Test Plan - [x] Add unit tests. - [x] Verify page boundaries, queue isolation, identifier limits, and continuation after deletion against MySQL. ## Revert Plan - Revert this PR. It changes storage reads and input validation without a schema migration or enabling runtime writers. ## Issue Links - [CODEM-607](https://linear.app/uber/issue/CODEM-607/build-analysis) ## Stack 1. #923 1. #924 1. #925
1 parent fae9097 commit f75843b

11 files changed

Lines changed: 412 additions & 5 deletions

File tree

‎stovepipe/entity/open_issue.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package entity
1818
// Its task and unit identify the condition; the first-seen build identifies
1919
// this opening and is immutable until removal.
2020
type OpenIssue struct {
21-
// TaskID identifies the logical validation task.
21+
// TaskID identifies the logical validation task and must be nonempty.
2222
TaskID string `json:"task_id"`
2323
// UnitID identifies the validation unit; empty for a task-wide issue.
2424
UnitID string `json:"unit_id"`

‎stovepipe/extension/storage/README.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,17 @@ The MySQL primary key contains the queue, build ID, task ID, and unit ID directl
3636

3737
## Currently open issues
3838

39-
`OpenIssueStore` contains only currently open issues, keyed by task and unit within the bound queue. Removing an open row never removes immutable per-build observations. There is no open/resolved flag; a resolved issue has no row here.
39+
`OpenIssueStore` contains only currently open issues, keyed by task and unit within the bound queue. Removing an open row never removes immutable per-build observations. Task IDs must be nonempty; an empty unit identifies a task-wide issue. There is no open/resolved flag; a resolved issue has no row here.
4040

4141
An open row is immutable. Its first-seen build identifies the current opening, not its latest observation. Repeated observations remain in the per-build store. Reopening after removal uses a different first-seen build ID; no last-observed build, version counter, or update operation is stored here.
4242

43+
Listing scans the bound queue's primary-key prefix in ascending bytewise task/unit order, returning at most the requested positive limit. Continue after the last returned task/unit key; the zero cursor starts the range. A cursor with a unit but no task is invalid. Empty units remain task-wide issues and can be used as continuation boundaries. No secondary index or schema change is required.
44+
45+
Pages are not a snapshot across calls. Concurrent changes can affect which rows appear, and the cursor row need not still exist. Readers must not treat listing as an atomic replacement or resolution decision; guarded deletion still protects a later opening.
46+
4347
Deletion matches the task, unit, and first-seen build within the bound queue. It is safe to repeat: an absent opening is a no-op, and a stale deletion never removes a different opening of the same condition. Database failures are returned for retry.
4448

45-
MySQL identifiers use the same binary byte limits as per-build observations. Build IDs are opaque context, not ordering tokens. The store does not choose resolution, enumerate issues, write lifecycle history, or reconcile analysis responses. Lifecycle occurrences are retained separately by `IssueLogStore`. Source-order reconciliation belongs to later controllers, not this membership store.
49+
MySQL identifiers use the same binary byte limits as per-build observations. Build IDs are opaque context, not ordering tokens. The store does not choose resolution, write lifecycle history, or reconcile analysis responses. Lifecycle occurrences are retained separately by `IssueLogStore`. Source-order reconciliation belongs to later controllers, not this membership store.
4650

4751
## Issue lifecycle history
4852

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

Lines changed: 16 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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ go_test(
3636
"build_issue_store_test.go",
3737
"build_store_test.go",
3838
"issue_log_store_test.go",
39+
"open_issue_list_test.go",
3940
"open_issue_store_test.go",
4041
"project_fact_by_degree_store_test.go",
4142
"queue_store_test.go",
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
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/driver"
20+
"regexp"
21+
"strings"
22+
"testing"
23+
24+
"github.com/DATA-DOG/go-sqlmock"
25+
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/require"
27+
"github.com/uber/submitqueue/stovepipe/entity"
28+
"github.com/uber/submitqueue/stovepipe/extension/storage"
29+
)
30+
31+
func TestOpenIssueList(t *testing.T) {
32+
for _, tt := range []struct {
33+
name string
34+
after storage.OpenIssueCursor
35+
empty bool
36+
dbErr error
37+
scanError bool
38+
rowError bool
39+
}{
40+
{name: "first page"},
41+
{name: "after task-wide issue", after: storage.OpenIssueCursor{TaskID: "task"}},
42+
{name: "empty page", empty: true},
43+
{name: "query error", dbErr: assert.AnError},
44+
{name: "cancelled", dbErr: context.Canceled},
45+
{name: "scan error", scanError: true},
46+
{name: "iteration error", rowError: true},
47+
} {
48+
t.Run(tt.name, func(t *testing.T) {
49+
db, mock, err := sqlmock.New()
50+
require.NoError(t, err)
51+
defer db.Close()
52+
store := NewOpenIssueStore(db, testMetrics(), "queue")
53+
issue := openIssueFixture()
54+
query := listOpenIssuesQuery
55+
args := []driver.Value{"queue", int64(2)}
56+
if tt.after.TaskID != "" {
57+
query = listOpenIssuesAfterCursorQuery
58+
args = []driver.Value{"queue", tt.after.TaskID, tt.after.TaskID, tt.after.UnitID, int64(2)}
59+
}
60+
expectation := mock.ExpectQuery(regexp.QuoteMeta(query)).WithArgs(args...)
61+
if tt.dbErr != nil {
62+
expectation.WillReturnError(tt.dbErr)
63+
} else {
64+
rows := sqlmock.NewRows([]string{"task_id", "unit_id", "first_seen_build_id"})
65+
if !tt.empty {
66+
var firstBuild any = issue.FirstSeenBuildID
67+
if tt.scanError {
68+
firstBuild = nil
69+
}
70+
rows.AddRow(issue.TaskID, issue.UnitID, firstBuild)
71+
if tt.rowError {
72+
rows.AddRow("task", "unit-2", "build/2").RowError(1, assert.AnError)
73+
}
74+
}
75+
expectation.WillReturnRows(rows).RowsWillBeClosed()
76+
}
77+
78+
got, err := store.List(context.Background(), storage.OpenIssueRange{After: tt.after, Limit: 2})
79+
switch {
80+
case tt.dbErr != nil:
81+
require.ErrorIs(t, err, tt.dbErr)
82+
require.Nil(t, got)
83+
case tt.scanError:
84+
require.Error(t, err)
85+
require.Nil(t, got)
86+
case tt.rowError:
87+
require.ErrorIs(t, err, assert.AnError)
88+
require.Nil(t, got)
89+
case tt.empty:
90+
require.NoError(t, err)
91+
require.NotNil(t, got)
92+
require.Empty(t, got)
93+
default:
94+
require.NoError(t, err)
95+
require.Equal(t, []entity.OpenIssue{issue}, got)
96+
}
97+
require.NoError(t, mock.ExpectationsWereMet())
98+
})
99+
}
100+
}
101+
102+
func TestOpenIssueListRejectsInvalidRange(t *testing.T) {
103+
for _, tt := range []struct {
104+
name string
105+
queue string
106+
bounds storage.OpenIssueRange
107+
}{
108+
{name: "zero limit", bounds: storage.OpenIssueRange{}},
109+
{name: "negative limit", bounds: storage.OpenIssueRange{Limit: -1}},
110+
{name: "unit without task", bounds: storage.OpenIssueRange{Limit: 1,
111+
After: storage.OpenIssueCursor{UnitID: "unit"}}},
112+
{name: "oversized task", bounds: storage.OpenIssueRange{Limit: 1,
113+
After: storage.OpenIssueCursor{TaskID: strings.Repeat("t", 256)}}},
114+
{name: "oversized unit", bounds: storage.OpenIssueRange{Limit: 1,
115+
After: storage.OpenIssueCursor{TaskID: "task", UnitID: strings.Repeat("😀", 257)}}},
116+
{name: "oversized queue", queue: strings.Repeat("q", 256), bounds: storage.OpenIssueRange{Limit: 1}},
117+
} {
118+
t.Run(tt.name, func(t *testing.T) {
119+
db, mock, err := sqlmock.New()
120+
require.NoError(t, err)
121+
defer db.Close()
122+
queue := tt.queue
123+
if queue == "" {
124+
queue = "queue"
125+
}
126+
store := NewOpenIssueStore(db, testMetrics(), queue)
127+
mock.ExpectQuery("SELECT").WillReturnRows(sqlmock.NewRows([]string{"task_id", "unit_id", "first_seen_build_id"}))
128+
129+
got, err := store.List(context.Background(), tt.bounds)
130+
require.Error(t, err)
131+
require.Nil(t, got)
132+
})
133+
}
134+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ import (
2626
"github.com/uber/submitqueue/stovepipe/extension/storage"
2727
)
2828

29+
const listOpenIssuesQuery = `
30+
SELECT task_id, unit_id, first_seen_build_id
31+
FROM open_issue WHERE queue = ?
32+
ORDER BY task_id ASC, unit_id ASC LIMIT ?`
33+
34+
const listOpenIssuesAfterCursorQuery = `
35+
SELECT task_id, unit_id, first_seen_build_id
36+
FROM open_issue WHERE queue = ?
37+
AND (task_id > ? OR (task_id = ? AND unit_id > ?))
38+
ORDER BY task_id ASC, unit_id ASC LIMIT ?`
39+
2940
type openIssueStore struct {
3041
db *sql.DB
3142
scope tally.Scope
@@ -64,6 +75,9 @@ func (s *openIssueStore) Get(ctx context.Context, taskID, unitID string) (ret en
6475
op := metrics.Begin(s.scope, "get", metrics.StorageLatencyBuckets)
6576
defer func() { op.Complete(retErr) }()
6677

78+
if taskID == "" {
79+
return entity.OpenIssue{}, fmt.Errorf("open issue task ID must not be empty")
80+
}
6781
if err := validateBuildIssueKey(s.queue, "", taskID, unitID); err != nil {
6882
return entity.OpenIssue{}, err
6983
}
@@ -84,6 +98,51 @@ func (s *openIssueStore) Get(ctx context.Context, taskID, unitID string) (ret en
8498
return issue, nil
8599
}
86100

101+
func (s *openIssueStore) List(ctx context.Context, bounds storage.OpenIssueRange) (ret []entity.OpenIssue, retErr error) {
102+
op := metrics.Begin(s.scope, "list", metrics.StorageLatencyBuckets)
103+
defer func() { op.Complete(retErr) }()
104+
105+
cursor := bounds.After
106+
if bounds.Limit <= 0 {
107+
return nil, fmt.Errorf("open issue range requires a positive limit")
108+
}
109+
if cursor.TaskID == "" && cursor.UnitID != "" {
110+
return nil, fmt.Errorf("open issue cursor requires a task ID when a unit ID is supplied")
111+
}
112+
if err := validateBuildIssueKey(s.queue, "", cursor.TaskID, cursor.UnitID); err != nil {
113+
return nil, err
114+
}
115+
116+
var (
117+
rows *sql.Rows
118+
err error
119+
)
120+
if cursor.TaskID != "" {
121+
rows, err = s.db.QueryContext(ctx, listOpenIssuesAfterCursorQuery,
122+
s.queue, cursor.TaskID, cursor.TaskID, cursor.UnitID, bounds.Limit)
123+
} else {
124+
rows, err = s.db.QueryContext(ctx, listOpenIssuesQuery, s.queue, bounds.Limit)
125+
}
126+
if err != nil {
127+
return nil, fmt.Errorf("failed to list open issues: %w", err)
128+
}
129+
defer rows.Close()
130+
131+
issues := make([]entity.OpenIssue, 0)
132+
for rows.Next() {
133+
var issue entity.OpenIssue
134+
if err := rows.Scan(&issue.TaskID, &issue.UnitID, &issue.FirstSeenBuildID); err != nil {
135+
return nil, fmt.Errorf("failed to scan open issue: %w", err)
136+
}
137+
issues = append(issues, issue)
138+
}
139+
if err := rows.Err(); err != nil {
140+
return nil, fmt.Errorf("failed to iterate open issues: %w", err)
141+
}
142+
143+
return issues, nil
144+
}
145+
87146
func (s *openIssueStore) Delete(ctx context.Context, issue entity.OpenIssue) (retErr error) {
88147
op := metrics.Begin(s.scope, "delete", metrics.StorageLatencyBuckets)
89148
defer func() { op.Complete(retErr) }()
@@ -104,6 +163,9 @@ func (s *openIssueStore) Delete(ctx context.Context, issue entity.OpenIssue) (re
104163
}
105164

106165
func (s *openIssueStore) validateIssue(issue entity.OpenIssue) error {
166+
if issue.TaskID == "" {
167+
return fmt.Errorf("open issue task ID must not be empty")
168+
}
107169
if err := validateBuildIssueKey(s.queue, issue.FirstSeenBuildID, issue.TaskID, issue.UnitID); err != nil {
108170
return err
109171
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func TestOpenIssueDelete(t *testing.T) {
166166
}
167167

168168
func TestOpenIssueRejectsInvalidIdentifiers(t *testing.T) {
169-
for _, kind := range []string{"queue", "task", "unit", "first build", "empty first build"} {
169+
for _, kind := range []string{"queue", "task", "empty task", "unit", "first build", "empty first build"} {
170170
t.Run(kind, func(t *testing.T) {
171171
db, mock, err := sqlmock.New()
172172
require.NoError(t, err)
@@ -179,6 +179,8 @@ func TestOpenIssueRejectsInvalidIdentifiers(t *testing.T) {
179179
queue = strings.Repeat("q", 256)
180180
case "task":
181181
issue.TaskID = strings.Repeat("t", 256)
182+
case "empty task":
183+
issue.TaskID = ""
182184
case "unit":
183185
issue.UnitID = strings.Repeat("😀", 257)
184186
case "first build":
@@ -200,7 +202,7 @@ func TestOpenIssueRejectsInvalidIdentifiers(t *testing.T) {
200202
require.Error(t, store.Delete(context.Background(), issue))
201203

202204
switch kind {
203-
case "queue", "task", "unit":
205+
case "queue", "task", "empty task", "unit":
204206
_, err = store.Get(context.Background(), issue.TaskID, issue.UnitID)
205207
require.Error(t, err)
206208
}

‎stovepipe/extension/storage/open_issue_store.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,25 @@ import (
2222
"github.com/uber/submitqueue/stovepipe/entity"
2323
)
2424

25+
// OpenIssueCursor is an exclusive position in ascending bytewise task/unit order.
26+
type OpenIssueCursor struct {
27+
// TaskID is the task component; empty with an empty UnitID starts the range.
28+
TaskID string
29+
// UnitID is the unit component of the last returned key; empty is a task-wide issue.
30+
UnitID string
31+
}
32+
33+
// OpenIssueRange bounds a primary-key scan within the store's queue.
34+
type OpenIssueRange struct {
35+
// After excludes this key and earlier keys; the zero cursor starts the range.
36+
After OpenIssueCursor
37+
// Limit is the positive maximum number of issues to return.
38+
Limit int
39+
}
40+
2541
// OpenIssueStore contains only currently open issues, keyed by task and unit
2642
// within the bound queue. A reopened issue must use a new first-seen build ID.
43+
// Issue keys require a nonempty task ID; an empty unit is a task-wide issue.
2744
type OpenIssueStore interface {
2845
// Create requires a nonempty first-seen build ID.
2946
// An existing task/unit returns ErrAlreadyExists.
@@ -32,6 +49,11 @@ type OpenIssueStore interface {
3249
// Get returns ErrNotFound when the task/unit is not currently open.
3350
Get(ctx context.Context, taskID, unitID string) (entity.OpenIssue, error)
3451

52+
// List returns at most Limit issues in ascending bytewise (task, unit) order.
53+
// An empty slice ends the range; invalid bounds return an error.
54+
// Pages are not a snapshot: concurrent creates/deletes may change membership.
55+
List(ctx context.Context, bounds OpenIssueRange) ([]entity.OpenIssue, error)
56+
3557
// Delete removes only the matching opening. It succeeds if that opening is
3658
// already absent and never removes a row with a different first-seen build.
3759
Delete(ctx context.Context, issue entity.OpenIssue) error

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go_test(
55
srcs = [
66
"build_issue_test.go",
77
"issue_log_test.go",
8+
"open_issue_list_test.go",
89
"open_issue_test.go",
910
"project_fact_by_degree_test.go",
1011
"queue_state_transitions_test.go",

0 commit comments

Comments
 (0)