Skip to content

Commit c543478

Browse files
committed
feat(stovepipe): Save pending issue changes
1 parent 97ef3b1 commit c543478

9 files changed

Lines changed: 252 additions & 23 deletions

File tree

‎stovepipe/entity/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ go_library(
1111
"issue_tracking_state.go",
1212
"list.go",
1313
"open_issue.go",
14+
"pending_issue_change.go",
1415
"project_fact_by_degree.go",
1516
"project_status.go",
1617
"queue.go",
@@ -39,6 +40,7 @@ go_test(
3940
"issue_log_test.go",
4041
"issue_state_test.go",
4142
"issue_tracking_state_test.go",
43+
"pending_issue_change_test.go",
4244
"project_fact_by_degree_test.go",
4345
"queue_state_test.go",
4446
"request_id_test.go",

‎stovepipe/entity/issue_tracking_state.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ type IssueTrackingState struct {
4949
AfterTaskID string `json:"after_task_id"`
5050
// AfterUnitID is the unit component of the exclusive cursor; empty is task-wide.
5151
AfterUnitID string `json:"after_unit_id"`
52+
// PendingChange is the saved issue transition; its zero value means none.
53+
PendingChange PendingIssueChange `json:"pending_change"`
5254
// Phase distinguishes pending work from completed refresh or final passes.
5355
Phase IssueTrackingPhase `json:"phase"`
5456
// Version is the positive optimistic-lock version of this state.
@@ -60,6 +62,9 @@ func (s IssueTrackingState) IsPending() bool {
6062
return s.Phase == IssueTrackingPhaseRefreshing || s.Phase == IssueTrackingPhaseFinalizing
6163
}
6264

65+
// HasPendingChange reports whether a saved issue transition is present.
66+
func (s IssueTrackingState) HasPendingChange() bool { return s.PendingChange.TaskID != "" }
67+
6368
// Validate verifies the identity, phase, and version of a persisted state.
6469
func (s IssueTrackingState) Validate() error {
6570
if err := id.Validate(s.RequestID); err != nil {
@@ -77,6 +82,24 @@ func (s IssueTrackingState) Validate() error {
7782
if s.Version <= 0 {
7883
return fmt.Errorf("issue tracking version must be positive")
7984
}
85+
if err := s.PendingChange.Validate(); err != nil {
86+
return err
87+
}
88+
if s.HasPendingChange() {
89+
if !s.IsPending() {
90+
return fmt.Errorf("completed issue tracking cannot retain a pending change")
91+
}
92+
change := s.PendingChange
93+
if change.TaskID < s.AfterTaskID || (change.TaskID == s.AfterTaskID && change.UnitID <= s.AfterUnitID) {
94+
return fmt.Errorf("pending issue change must follow the completed cursor")
95+
}
96+
if change.Status == IssueStatusOpen && change.FirstSeenBuildID != s.BuildID {
97+
return fmt.Errorf("pending opening must identify its source build")
98+
}
99+
if change.Status == IssueStatusClosed && s.Phase != IssueTrackingPhaseFinalizing {
100+
return fmt.Errorf("only final issue tracking can close an issue")
101+
}
102+
}
80103
switch s.Phase {
81104
case IssueTrackingPhaseRefreshing, IssueTrackingPhaseRefreshed, IssueTrackingPhaseFinalizing, IssueTrackingPhaseFinalized:
82105
return nil
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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 entity
16+
17+
import (
18+
"fmt"
19+
"math"
20+
)
21+
22+
// PendingIssueChange is one saved issue transition with its original CAS guard.
23+
// The zero value means no change is pending.
24+
type PendingIssueChange struct {
25+
// TaskID identifies the issue's validation task.
26+
TaskID string `json:"task_id"`
27+
// UnitID identifies the unit; empty is task-wide.
28+
UnitID string `json:"unit_id"`
29+
// Status is the intended open or closed status.
30+
Status IssueStatus `json:"status"`
31+
// FirstSeenBuildID identifies the opening retained by the intended issue state.
32+
FirstSeenBuildID string `json:"first_seen_build_id"`
33+
// ExpectedVersion is the original issue version; zero requires absence.
34+
ExpectedVersion int32 `json:"expected_version"`
35+
// TimestampMs is the transition's retained history timestamp in Unix milliseconds.
36+
TimestampMs int64 `json:"timestamp_ms"`
37+
}
38+
39+
// Validate checks a complete change or the empty sentinel.
40+
func (c PendingIssueChange) Validate() error {
41+
if c == (PendingIssueChange{}) {
42+
return nil
43+
}
44+
if c.ExpectedVersion < 0 || c.ExpectedVersion == math.MaxInt32 {
45+
return fmt.Errorf("pending issue version must permit a positive successor")
46+
}
47+
target := IssueState{TaskID: c.TaskID, UnitID: c.UnitID, Status: c.Status, FirstSeenBuildID: c.FirstSeenBuildID, Version: c.ExpectedVersion + 1}
48+
if err := target.Validate(); err != nil {
49+
return err
50+
}
51+
if c.Status == IssueStatusClosed && c.ExpectedVersion == 0 {
52+
return fmt.Errorf("closing an issue requires an existing version")
53+
}
54+
if c.TimestampMs <= 0 {
55+
return fmt.Errorf("pending issue timestamp must be positive")
56+
}
57+
return nil
58+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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 entity
16+
17+
import (
18+
"math"
19+
"testing"
20+
21+
"github.com/stretchr/testify/require"
22+
)
23+
24+
func TestPendingIssueChangeValidate(t *testing.T) {
25+
require.NoError(t, (PendingIssueChange{}).Validate())
26+
opening := PendingIssueChange{TaskID: "task", Status: IssueStatusOpen, FirstSeenBuildID: "build", TimestampMs: 100}
27+
require.NoError(t, opening.Validate())
28+
closing := opening
29+
closing.Status = IssueStatusClosed
30+
closing.ExpectedVersion = 7
31+
require.NoError(t, closing.Validate())
32+
for _, tt := range []struct {
33+
name string
34+
change func(*PendingIssueChange)
35+
}{
36+
{"missing task", func(c *PendingIssueChange) { c.TaskID = "" }},
37+
{"missing opening", func(c *PendingIssueChange) { c.FirstSeenBuildID = "" }},
38+
{"unknown status", func(c *PendingIssueChange) { c.Status = IssueStatusUnknown }},
39+
{"negative guard", func(c *PendingIssueChange) { c.ExpectedVersion = -1 }},
40+
{"exhausted guard", func(c *PendingIssueChange) { c.ExpectedVersion = math.MaxInt32 }},
41+
{"closing absent issue", func(c *PendingIssueChange) { c.Status = IssueStatusClosed }},
42+
{"missing timestamp", func(c *PendingIssueChange) { c.TimestampMs = 0 }},
43+
} {
44+
t.Run(tt.name, func(t *testing.T) { value := opening; tt.change(&value); require.Error(t, value.Validate()) })
45+
}
46+
}
47+
48+
func TestIssueTrackingPendingChange(t *testing.T) {
49+
state := IssueTrackingState{RequestID: "10", BuildID: "build", NotificationID: "notice", Phase: IssueTrackingPhaseRefreshing, Version: 1,
50+
PendingChange: PendingIssueChange{TaskID: "task", UnitID: "unit", Status: IssueStatusOpen, FirstSeenBuildID: "build", TimestampMs: 100}}
51+
require.True(t, state.HasPendingChange())
52+
require.NoError(t, state.Validate())
53+
for _, tt := range []struct {
54+
name string
55+
change func(*IssueTrackingState)
56+
}{
57+
{"completed pass", func(s *IssueTrackingState) { s.Phase = IssueTrackingPhaseRefreshed }},
58+
{"behind cursor", func(s *IssueTrackingState) { s.AfterTaskID = "z" }},
59+
{"at cursor", func(s *IssueTrackingState) { s.AfterTaskID = "task"; s.AfterUnitID = "unit" }},
60+
{"opening from another build", func(s *IssueTrackingState) { s.PendingChange.FirstSeenBuildID = "other" }},
61+
{"refresh cannot close", func(s *IssueTrackingState) {
62+
s.PendingChange.Status = IssueStatusClosed
63+
s.PendingChange.ExpectedVersion = 1
64+
}},
65+
} {
66+
t.Run(tt.name, func(t *testing.T) { value := state; tt.change(&value); require.Error(t, value.Validate()) })
67+
}
68+
state.Phase = IssueTrackingPhaseFinalizing
69+
state.PendingChange.Status = IssueStatusClosed
70+
state.PendingChange.ExpectedVersion = 7
71+
state.PendingChange.FirstSeenBuildID = "older"
72+
require.NoError(t, state.Validate())
73+
}

‎stovepipe/extension/storage/README.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,14 @@ History persistence does not alter issue state or validate transitions against i
6262

6363
## Issue reconciliation progress
6464

65-
`IssueTrackingStore` retains one queue-scoped operational `IssueTrackingState`, independently of open-issue membership and user-facing summaries. It records the source request and build, whether refresh or final writes are pending or complete, the owning notification, an exclusive task/unit cursor, and its optimistic version. The zero cursor starts a pass. A cursor checkpoint follows the writes for every processed key; notification identity lets the controller distinguish its own continuation from helping another pass. Keeping it after the last open row is removed preserves the source-order boundary. The MySQL table is keyed only by queue; creates, reads, and updates use complete primary keys with no secondary indexes or cross-table transaction.
65+
`IssueTrackingStore` retains one queue-scoped operational `IssueTrackingState`, independently of open-issue membership and user-facing summaries. It records the source request and build, whether refresh or final writes are pending or complete, the owning notification, an exclusive task/unit cursor, and its optimistic version. The zero cursor starts a pass. A cursor checkpoint follows the writes for every processed key; notification identity lets the controller distinguish its own continuation from helping another pass. The record remains after all issues close to preserve the source-order boundary. The MySQL table is keyed only by queue; creates, reads, and updates use complete primary keys with no secondary indexes or cross-table transaction.
6666

6767
The store does not decide source ordering, apply issue transitions, or advance versions. Its caller supplies the guard version and replacement version. A later controller will own acceptance, resumption of pending writes, and completion; no runtime writer is enabled here.
6868

6969
The issue-tracking table must include the notification and cursor columns before updated readers or writers are deployed. No runtime writer is enabled by the storage layer. Notification IDs allow up to 1020 bytes, matching the queue message ID byte capacity; cursor limits match the issue stores.
70+
71+
### Saved issue changes
72+
73+
Progress can retain one `PendingIssueChange`: the task/unit, intended status and first-seen build, original expected issue version, and history timestamp. Expected version zero requires create-if-absent; a positive value guards an update of an existing record. The expected version is never replaced with a newer value when a retry resumes old instructions.
74+
75+
The pending change is saved with progress CAS before any issue write. A replacement worker must finish that same issue write and its idempotent history event before clearing the change and advancing the cursor. Pending changes are bounded fields, not a serialized batch. Closing retains the issue record; neither issue versions nor the queue's source boundary may be reset while old deliveries could resume.

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

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,17 @@ func (s *issueTrackingStore) Get(ctx context.Context) (state entity.IssueTrackin
4141
op := metrics.Begin(s.scope, "get", metrics.StorageLatencyBuckets)
4242
defer func() { op.Complete(retErr) }()
4343
err := s.db.QueryRowContext(ctx,
44-
`SELECT request_id, build_id, notification_id, after_task_id, after_unit_id, phase, version FROM issue_tracking WHERE queue = ?`, s.queue).
45-
Scan(&state.RequestID, &state.BuildID, &state.NotificationID, &state.AfterTaskID, &state.AfterUnitID, &state.Phase, &state.Version)
44+
`SELECT request_id, build_id, notification_id,
45+
after_task_id, after_unit_id,
46+
pending_task_id, pending_unit_id, pending_status, pending_first_seen_build_id,
47+
pending_expected_version, pending_timestamp_ms, phase, version
48+
FROM issue_tracking WHERE queue = ?`, s.queue).
49+
Scan(
50+
&state.RequestID, &state.BuildID, &state.NotificationID,
51+
&state.AfterTaskID, &state.AfterUnitID,
52+
&state.PendingChange.TaskID, &state.PendingChange.UnitID, &state.PendingChange.Status, &state.PendingChange.FirstSeenBuildID,
53+
&state.PendingChange.ExpectedVersion, &state.PendingChange.TimestampMs, &state.Phase, &state.Version,
54+
)
4655
if errors.Is(err, sql.ErrNoRows) {
4756
return entity.IssueTrackingState{}, storage.WrapNotFound(err)
4857
}
@@ -59,8 +68,15 @@ func (s *issueTrackingStore) Create(ctx context.Context, state entity.IssueTrack
5968
return err
6069
}
6170
_, err := s.db.ExecContext(ctx,
62-
`INSERT INTO issue_tracking (queue, request_id, build_id, notification_id, after_task_id, after_unit_id, phase, version) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
63-
s.queue, state.RequestID, state.BuildID, state.NotificationID, state.AfterTaskID, state.AfterUnitID, state.Phase, state.Version)
71+
`INSERT INTO issue_tracking (queue, request_id, build_id, notification_id,
72+
after_task_id, after_unit_id,
73+
pending_task_id, pending_unit_id, pending_status, pending_first_seen_build_id,
74+
pending_expected_version, pending_timestamp_ms, phase, version) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
75+
s.queue, state.RequestID, state.BuildID, state.NotificationID,
76+
state.AfterTaskID, state.AfterUnitID,
77+
state.PendingChange.TaskID, state.PendingChange.UnitID, state.PendingChange.Status, state.PendingChange.FirstSeenBuildID,
78+
state.PendingChange.ExpectedVersion, state.PendingChange.TimestampMs, state.Phase, state.Version,
79+
)
6480
if isDuplicateEntry(err) {
6581
return storage.ErrAlreadyExists
6682
}
@@ -82,8 +98,17 @@ func (s *issueTrackingStore) Update(ctx context.Context, state entity.IssueTrack
8298
return fmt.Errorf("issue tracking guard version must be positive")
8399
}
84100
result, err := s.db.ExecContext(ctx,
85-
`UPDATE issue_tracking SET request_id = ?, build_id = ?, notification_id = ?, after_task_id = ?, after_unit_id = ?, phase = ?, version = ? WHERE queue = ? AND version = ?`,
86-
state.RequestID, state.BuildID, state.NotificationID, state.AfterTaskID, state.AfterUnitID, state.Phase, newVersion, s.queue, oldVersion)
101+
`UPDATE issue_tracking SET request_id = ?, build_id = ?, notification_id = ?,
102+
after_task_id = ?, after_unit_id = ?,
103+
pending_task_id = ?, pending_unit_id = ?, pending_status = ?, pending_first_seen_build_id = ?,
104+
pending_expected_version = ?, pending_timestamp_ms = ?, phase = ?, version = ?
105+
WHERE queue = ? AND version = ?`,
106+
state.RequestID, state.BuildID, state.NotificationID,
107+
state.AfterTaskID, state.AfterUnitID,
108+
state.PendingChange.TaskID, state.PendingChange.UnitID, state.PendingChange.Status, state.PendingChange.FirstSeenBuildID,
109+
state.PendingChange.ExpectedVersion, state.PendingChange.TimestampMs, state.Phase, newVersion,
110+
s.queue, oldVersion,
111+
)
87112
if err != nil {
88113
return fmt.Errorf("update issue tracking state: %w", err)
89114
}
@@ -105,7 +130,11 @@ func (s *issueTrackingStore) validateState(state entity.IssueTrackingState) erro
105130
if len(state.NotificationID) > 1020 {
106131
return fmt.Errorf("issue tracking notification ID exceeds 1020-byte limit")
107132
}
108-
return validateBuildIssueKey(s.queue, state.BuildID, state.AfterTaskID, state.AfterUnitID)
133+
if err := validateBuildIssueKey(s.queue, state.BuildID, state.AfterTaskID, state.AfterUnitID); err != nil {
134+
return err
135+
}
136+
change := state.PendingChange
137+
return validateBuildIssueKey(s.queue, change.FirstSeenBuildID, change.TaskID, change.UnitID)
109138
}
110139

111140
var _ storage.IssueTrackingStore = (*issueTrackingStore)(nil)

0 commit comments

Comments
 (0)