Skip to content

Commit 86a421d

Browse files
committed
feat(stovepipe): select and submit one repair per build
Scan build observations for existing fix IDs, submit an eligible open issue, and associate the returned ID through entity CAS. Return submission errors without an ID for normal queue handling; document caller-owned retry policy. Test Plan: Run ./tool/bazel test //stovepipe/controller/fix:go_default_test and make lint check-tidy check-gazelle. Revert Plan: Revert this commit.
1 parent 3ca80ca commit 86a421d

6 files changed

Lines changed: 534 additions & 6 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["fix.go"],
6+
importpath = "github.com/uber/submitqueue/stovepipe/controller/fix",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//platform/consumer:go_default_library",
10+
"//platform/metrics:go_default_library",
11+
"//stovepipe/entity:go_default_library",
12+
"//stovepipe/extension/fixer:go_default_library",
13+
"//stovepipe/extension/storage:go_default_library",
14+
"@com_github_uber_go_tally//:go_default_library",
15+
"@org_uber_go_zap//:go_default_library",
16+
],
17+
)
18+
19+
go_test(
20+
name = "go_default_test",
21+
srcs = ["fix_test.go"],
22+
embed = [":go_default_library"],
23+
deps = [
24+
"//platform/consumer:go_default_library",
25+
"//platform/errs:go_default_library",
26+
"//stovepipe/entity:go_default_library",
27+
"//stovepipe/extension/fixer:go_default_library",
28+
"//stovepipe/extension/fixer/mock:go_default_library",
29+
"//stovepipe/extension/storage:go_default_library",
30+
"//stovepipe/extension/storage/mock:go_default_library",
31+
"@com_github_stretchr_testify//assert:go_default_library",
32+
"@com_github_stretchr_testify//require:go_default_library",
33+
"@com_github_uber_go_tally//:go_default_library",
34+
"@org_uber_go_mock//gomock:go_default_library",
35+
"@org_uber_go_zap//:go_default_library",
36+
"@org_uber_go_zap//zaptest/observer:go_default_library",
37+
],
38+
)

‎stovepipe/controller/fix/README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Fix submission
2+
3+
The controller pages through a build's durable issue records and skips submission when any record has a fix ID, including recovered issues. Otherwise, it selects one currently open issue whose first-seen build matches this build and calls `RepairExecutor.Submit`.
4+
5+
A returned fix ID is associated with the issue through `BuildIssueStore.Update`, using the version read during selection. A concurrent association is preserved. A submission error without an ID leaves the issue eligible for normal queue retries; an ID returned alongside an error is retained.
6+
7+
The one-per-build limit is controller policy. Submission precedes persistence, so overlapping deliveries, crashes, and failed writes can still produce duplicate executor calls. Issue recovery remains separate from repair association.

‎stovepipe/controller/fix/fix.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
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 fix submits build-scoped repair work.
16+
package fix
17+
18+
import (
19+
"context"
20+
"errors"
21+
"fmt"
22+
23+
"github.com/uber-go/tally"
24+
"github.com/uber/submitqueue/platform/consumer"
25+
"github.com/uber/submitqueue/platform/metrics"
26+
"github.com/uber/submitqueue/stovepipe/entity"
27+
"github.com/uber/submitqueue/stovepipe/extension/fixer"
28+
"github.com/uber/submitqueue/stovepipe/extension/storage"
29+
"go.uber.org/zap"
30+
)
31+
32+
const buildIssuePageSize = 100
33+
34+
type Controller struct {
35+
logger *zap.SugaredLogger
36+
metricsScope tally.Scope
37+
stores storage.Factory
38+
executors fixer.Factory
39+
topicKey consumer.TopicKey
40+
consumerGroup string
41+
}
42+
43+
func NewController(
44+
logger *zap.SugaredLogger,
45+
scope tally.Scope,
46+
stores storage.Factory,
47+
executors fixer.Factory,
48+
topicKey consumer.TopicKey,
49+
consumerGroup string,
50+
) *Controller {
51+
return &Controller{
52+
logger: logger.Named("fix_controller"),
53+
metricsScope: scope.SubScope("fix_controller"),
54+
stores: stores,
55+
executors: executors,
56+
topicKey: topicKey,
57+
consumerGroup: consumerGroup,
58+
}
59+
}
60+
61+
func (c *Controller) processBuild(ctx context.Context, queueName, buildID string) error {
62+
store, err := c.stores.For(storage.Config{QueueName: queueName})
63+
if err != nil {
64+
return fmt.Errorf("failed to resolve repair storage: %w", err)
65+
}
66+
observation, found, err := findIssueToFix(ctx, store.GetBuildIssueStore(), store.GetOpenIssueStore(), buildID)
67+
if err != nil {
68+
return fmt.Errorf("failed to select an issue for build %q: %w", buildID, err)
69+
}
70+
if !found {
71+
return nil
72+
}
73+
74+
executor, err := c.executors.For(fixer.Config{QueueName: queueName})
75+
if err != nil {
76+
return fmt.Errorf("failed to resolve repair executor: %w", err)
77+
}
78+
79+
op := metrics.Begin(c.metricsScope, "submit", metrics.LongLatencyBuckets, metrics.TagsFromContext(ctx)...)
80+
issue := entity.OpenIssue{TaskID: observation.TaskID, UnitID: observation.UnitID, FirstSeenBuildID: buildID}
81+
fixID, err := executor.Submit(ctx, issue)
82+
if err == nil && fixID == "" {
83+
err = fmt.Errorf("repair executor returned an empty fix ID")
84+
}
85+
op.Complete(err)
86+
if err != nil {
87+
c.logger.Errorw("repair submission failed",
88+
"queue", queueName, "build_id", buildID,
89+
"task_id", issue.TaskID, "unit_id", issue.UnitID,
90+
"fix_id", fixID, "error", err)
91+
if fixID == "" {
92+
return fmt.Errorf("failed to submit repair for build %q: %w", buildID, err)
93+
}
94+
} else {
95+
c.logger.Infow("repair submitted",
96+
"queue", queueName, "build_id", buildID,
97+
"task_id", issue.TaskID, "unit_id", issue.UnitID, "fix_id", fixID)
98+
}
99+
100+
// Submission precedes persistence; a crash or write failure can allow duplicates.
101+
updated := observation
102+
updated.FixID = fixID
103+
observations := store.GetBuildIssueStore()
104+
if err := observations.Update(ctx, buildID, updated, observation.Version, observation.Version+1); err != nil {
105+
if !errors.Is(err, storage.ErrVersionMismatch) {
106+
return fmt.Errorf("failed to associate repair with build issue: %w", err)
107+
}
108+
current, readErr := observations.Get(ctx, buildID, observation.TaskID, observation.UnitID)
109+
if readErr != nil {
110+
return fmt.Errorf("failed to reload build issue after concurrent update: %w", readErr)
111+
}
112+
if current.FixID == "" {
113+
return err
114+
}
115+
c.logger.Warnw("preserving previously recorded repair association",
116+
"queue", queueName, "build_id", buildID,
117+
"task_id", issue.TaskID, "unit_id", issue.UnitID,
118+
"fix_id", current.FixID, "submitted_fix_id", fixID)
119+
}
120+
return nil
121+
}
122+
123+
func (c *Controller) Name() string { return "fix" }
124+
125+
func (c *Controller) TopicKey() consumer.TopicKey { return c.topicKey }
126+
127+
func (c *Controller) ConsumerGroup() string { return c.consumerGroup }
128+
129+
func findIssueToFix(ctx context.Context, observations storage.BuildIssueStore, openIssues storage.OpenIssueStore, buildID string) (entity.BuildIssue, bool, error) {
130+
bounds := storage.BuildIssueRange{Limit: buildIssuePageSize}
131+
var candidate entity.BuildIssue
132+
found := false
133+
for {
134+
page, err := observations.List(ctx, buildID, bounds)
135+
if err != nil {
136+
return entity.BuildIssue{}, false, err
137+
}
138+
if len(page) == 0 {
139+
return candidate, found, nil
140+
}
141+
for _, record := range page {
142+
if record.FixID != "" {
143+
return entity.BuildIssue{}, false, nil
144+
}
145+
if found {
146+
continue
147+
}
148+
issue, err := openIssues.Get(ctx, record.TaskID, record.UnitID)
149+
if errors.Is(err, storage.ErrNotFound) {
150+
continue
151+
}
152+
if err != nil {
153+
return entity.BuildIssue{}, false, err
154+
}
155+
if issue.FirstSeenBuildID == buildID {
156+
candidate, found = record, true
157+
}
158+
}
159+
last := page[len(page)-1]
160+
bounds.After = storage.BuildIssueCursor{TaskID: last.TaskID, UnitID: last.UnitID}
161+
}
162+
}

0 commit comments

Comments
 (0)