Skip to content

Commit b7ffb44

Browse files
gsturgesclaude
andauthored
feat(stovepipe): add DLQ reconciliation for the process stage (#347)
## Summary A request whose `process`-stage message exhausts retries would previously sit in a non-terminal state forever — indistinguishable from "not yet validated" to callers gating deployments on greenness. Per the fail-closed policy in [doc/rfc/stovepipe/workflow.md](doc/rfc/stovepipe/workflow.md), such requests must be driven to a conservative not-green terminal state instead. - **Entity**: add `RequestStateFailed` terminal state and `IsRequestStateTerminal` helper. - **New `stovepipe/controller/dlq` package**: a `consumer.Controller` that drains the `process_dlq` topic and CAS-transitions the affected request to `failed`. If the request had already been admitted (`processing`), the queue's `in_flight_count` slot is released first, per the integrity rule in [doc/rfc/stovepipe/steps/process.md](doc/rfc/stovepipe/steps/process.md). - **Server wiring**: second consumer using `errs.AlwaysRetryableProcessor` (a reconciliation failure retries in place — the DLQ subscription is a final destination with `DLQ.Enabled = false`, so nothing can cascade to a `_dlq_dlq`); `process_dlq` registered in the topic registry with raised retry attempts; DLQ consumer starts first and stops last. Reconciliation is idempotent: already-terminal requests are skipped, and optimistic locking lets a late legitimate pipeline transition win. The design mirrors SubmitQueue's proven `submitqueue/orchestrator/controller/dlq` pattern, simplified for stovepipe's single payload shape. Only `process` is a queue-consuming step today (`ingest` is synchronous RPC); future stages (`build`, `buildsignal`, `record`) can reuse the same pattern. ## Test plan - [x] New table-driven unit tests in `stovepipe/controller/dlq/dlq_test.go`: accepted→failed, processing→failed with slot release, already-terminal no-ops, not-found no-op, CAS version-mismatch paths, malformed/empty payloads - [x] `go build ./...` and `go test ./stovepipe/... ./service/stovepipe/...` pass - [x] `make gazelle`, `make fmt`, `make lint-license` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5063ddd commit b7ffb44

9 files changed

Lines changed: 612 additions & 22 deletions

File tree

platform/extension/messagequeue/subscription_config.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ type DLQConfig struct {
7878
TopicSuffix string
7979
}
8080

81+
// DLQSubscriptionConfig returns a SubscriptionConfig for consuming a dead-letter
82+
// topic (DLQ reconciliation). It starts from DefaultSubscriptionConfig and applies
83+
// the two overrides every DLQ consumer needs:
84+
//
85+
// - DLQ.Enabled is false, so a reconciliation failure retries in place instead of
86+
// cascading to a second-level "_dlq_dlq" topic that nobody consumes.
87+
// - Retry.MaxAttempts is a very high backstop so the per-message retry budget
88+
// effectively never runs out. This pairs with errs.AlwaysRetryableProcessor
89+
// wired into the DLQ consumer: reconciliation converges eventually instead of
90+
// being silently dropped after the default retry count.
91+
func DLQSubscriptionConfig(subscriberName, consumerGroup string) SubscriptionConfig {
92+
config := DefaultSubscriptionConfig(subscriberName, consumerGroup)
93+
config.DLQ.Enabled = false
94+
config.Retry.MaxAttempts = 1000
95+
return config
96+
}
97+
8198
// DefaultSubscriptionConfig returns a SubscriptionConfig with sensible defaults.
8299
func DefaultSubscriptionConfig(subscriberName, consumerGroup string) SubscriptionConfig {
83100
return SubscriptionConfig{

platform/extension/messagequeue/subscription_config_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,18 @@ func TestSubscriptionConfig_CustomValues(t *testing.T) {
7373
assert.Equal(t, "_dead", config.DLQ.TopicSuffix)
7474
}
7575

76+
func TestDLQSubscriptionConfig(t *testing.T) {
77+
config := DLQSubscriptionConfig("worker-1", "consumer-1-dlq")
78+
79+
assert.Equal(t, "worker-1", config.SubscriberName)
80+
assert.Equal(t, "consumer-1-dlq", config.ConsumerGroup)
81+
82+
// The DLQ consumer must not dead-letter its own failures (no "_dlq_dlq"
83+
// cascade) and needs a far larger retry budget than a primary consumer.
84+
assert.False(t, config.DLQ.Enabled)
85+
assert.Greater(t, config.Retry.MaxAttempts, DefaultSubscriptionConfig("worker-1", "consumer-1").Retry.MaxAttempts)
86+
}
87+
7688
func TestSubscriptionConfig_DifferentConsumerGroups(t *testing.T) {
7789
// Test that different consumer groups get independent configs
7890
tests := []struct {

service/stovepipe/server/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ go_library(
1515
"//platform/extension/messagequeue/mysql:go_default_library",
1616
"//service/stovepipe/server/mapper:go_default_library",
1717
"//stovepipe/controller:go_default_library",
18+
"//stovepipe/controller/dlq:go_default_library",
1819
"//stovepipe/controller/process:go_default_library",
1920
"//stovepipe/core/messagequeue:go_default_library",
2021
"//stovepipe/extension/queueconfig/default:go_default_library",

service/stovepipe/server/main.go

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
queueMySQL "github.com/uber/submitqueue/platform/extension/messagequeue/mysql"
3838
"github.com/uber/submitqueue/service/stovepipe/server/mapper"
3939
"github.com/uber/submitqueue/stovepipe/controller"
40+
"github.com/uber/submitqueue/stovepipe/controller/dlq"
4041
"github.com/uber/submitqueue/stovepipe/controller/process"
4142
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
4243
queueconfigdefault "github.com/uber/submitqueue/stovepipe/extension/queueconfig/default"
@@ -208,13 +209,21 @@ func run() error {
208209
return fmt.Errorf("failed to create topic registry: %w", err)
209210
}
210211

211-
// Consumer running the process stage.
212+
// Two consumers share the topic registry but apply different error classification
213+
// policies. The primary consumer runs the standard classifier walk. The DLQ consumer
214+
// uses AlwaysRetryableProcessor so every non-nil error from a DLQ controller is
215+
// forced retryable — reconciliation must redeliver on any failure because the DLQ
216+
// subscription is a final destination (DLQ.Enabled is false on it, so there is no
217+
// further DLQ to fall back on).
212218
primaryConsumer := consumer.New(logger.Sugar(), scope.SubScope("consumer"), registry,
213219
errs.NewClassifierProcessor(
214220
genericerrs.Classifier,
215221
mysqlerrs.Classifier,
216222
),
217223
)
224+
dlqConsumer := consumer.New(logger.Sugar(), scope.SubScope("consumer-dlq"), registry,
225+
errs.AlwaysRetryableProcessor,
226+
)
218227

219228
processController := process.NewController(
220229
logger.Sugar(),
@@ -230,10 +239,23 @@ func run() error {
230239
return fmt.Errorf("failed to register process controller: %w", err)
231240
}
232241

242+
processDLQController := dlq.NewController(logger.Sugar(), scope, store, dlq.TopicKey(stovepipemq.TopicKeyProcess), "stovepipe-process-dlq")
243+
if err := dlqConsumer.Register(processDLQController); err != nil {
244+
return fmt.Errorf("failed to register process dlq controller: %w", err)
245+
}
246+
247+
// Start consumers. DLQ first because Start begins processing messages
248+
// immediately; if the primary consumer then fails to start, the half we
249+
// already started is the DLQ side, whose work is idempotent reconciliation
250+
// and is safe to interrupt mid-flight for rollback.
251+
if err := dlqConsumer.Start(ctx); err != nil {
252+
return fmt.Errorf("failed to start dlq consumer: %w", err)
253+
}
233254
if err := primaryConsumer.Start(ctx); err != nil {
234-
return fmt.Errorf("failed to start consumer: %w", err)
255+
stopErr := dlqConsumer.Stop(30000)
256+
return errors.Join(fmt.Errorf("failed to start consumer: %w", err), stopErr)
235257
}
236-
logger.Info("consumer started")
258+
logger.Info("consumers started")
237259

238260
// Create gRPC server
239261
grpcServer := grpc.NewServer()
@@ -298,9 +320,15 @@ func run() error {
298320
serverErr = fmt.Errorf("GRPC server exited with error: %w", serverErr)
299321
}
300322

301-
consumerStopErr := primaryConsumer.Stop(30000)
323+
// Stop consumers in reverse start order: primary first, then DLQ. The primary
324+
// pipeline writes the state that DLQ reconciliation reads, so draining primary
325+
// first means in-flight DLQ reconciliation finishes against a settled primary
326+
// rather than racing its shutdown.
327+
primaryStopErr := primaryConsumer.Stop(30000)
328+
dlqStopErr := dlqConsumer.Stop(30000)
329+
consumerStopErr := errors.Join(primaryStopErr, dlqStopErr)
302330
if consumerStopErr != nil {
303-
consumerStopErr = fmt.Errorf("failed to stop consumer: %w", consumerStopErr)
331+
consumerStopErr = fmt.Errorf("failed to stop consumers: %w", consumerStopErr)
304332
}
305333

306334
if consumerStopErr != nil || serverErr != nil {
@@ -312,6 +340,8 @@ func run() error {
312340

313341
// newTopicRegistry builds the TopicRegistry for Stovepipe's internal pipeline queues. ingest
314342
// publishes to process; process publishes admitted requests to the publish-only build topic.
343+
// The process_dlq topic is the dead-letter destination the queue backend routes to (per
344+
// DefaultSubscriptionConfig's DLQ.TopicSuffix) when the process controller exhausts retries.
315345
func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRegistry, error) {
316346
return consumer.NewTopicRegistry([]consumer.TopicConfig{
317347
{
@@ -327,5 +357,11 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
327357
Name: "build",
328358
Queue: q,
329359
},
360+
{
361+
Key: dlq.TopicKey(stovepipemq.TopicKeyProcess),
362+
Name: "process_dlq",
363+
Queue: q,
364+
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-process-dlq"),
365+
},
330366
})
331367
}

service/submitqueue/orchestrator/server/main.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -403,27 +403,15 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
403403
subscriberName, t.groupSuffix,
404404
),
405405
})
406-
// DLQ subscription for the same primary stage. DLQ is disabled here
407-
// to avoid a "_dlq_dlq" cascade: if DLQ reconciliation itself fails,
408-
// the consumer retries forever and the failure is surfaced via logs
409-
// and metrics rather than being moved to a second-level dead-letter
410-
// topic that nobody consumes.
411-
//
412-
// MaxAttempts is bumped to a very high value so the per-message
413-
// retry budget effectively never runs out — this pairs with the
414-
// AlwaysRetryableProcessor wired into the DLQ consumer to guarantee
415-
// reconciliation eventually converges instead of being silently
416-
// dropped after the default retry count.
417-
dlqSub := extqueue.DefaultSubscriptionConfig(
418-
subscriberName, t.groupSuffix+"-dlq",
419-
)
420-
dlqSub.DLQ.Enabled = false
421-
dlqSub.Retry.MaxAttempts = 1000
406+
// DLQ subscription for the same primary stage. DLQSubscriptionConfig
407+
// disables the subscription's own DLQ (no "_dlq_dlq" cascade) and sets
408+
// an effectively unlimited retry budget to pair with the
409+
// AlwaysRetryableProcessor wired into the DLQ consumer.
422410
configs = append(configs, consumer.TopicConfig{
423411
Key: dlq.TopicKey(t.key),
424412
Name: t.name + "_dlq",
425413
Queue: q,
426-
Subscription: dlqSub,
414+
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, t.groupSuffix+"-dlq"),
427415
})
428416
}
429417

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

stovepipe/controller/dlq/dlq.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright (c) 2025 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 dlq contains controllers that consume messages from a pipeline stage's
16+
// dead-letter topic and reconcile the affected request into a terminal state.
17+
//
18+
// Background. The consumer framework moves a message to its DLQ after the controller
19+
// for the original topic returns a non-retryable error or exhausts retries on a
20+
// retryable error. Without DLQ reconciliation the affected request would remain stuck
21+
// in a non-terminal state (accepted, processing) forever — a caller gating deployments
22+
// on greenness would see that indistinguishably from "not yet validated" (see
23+
// doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work).
24+
//
25+
// Reconciliation strategy. Each DLQ topic carries the same payload as its originating
26+
// topic (the queue framework preserves the bytes verbatim under a new `{topic}_dlq`
27+
// name). The DLQ controller decodes that payload to recover the affected request, then
28+
// transitions it to RequestStateRecordedNotGreen — the conservative not-green verdict
29+
// for gating (see entity.RequestState) — with an idempotent optimistic-locking write so
30+
// concurrent activity (a late successful pipeline transition) wins cleanly. If the request had
31+
// already been admitted (processing) and was holding a concurrency slot, the
32+
// reconciler also releases it by CAS-decrementing the queue's in_flight_count, per
33+
// doc/rfc/stovepipe/steps/process.md#in_flight_count-integrity.
34+
package dlq
35+
36+
import (
37+
"context"
38+
"errors"
39+
"fmt"
40+
41+
"github.com/uber/submitqueue/platform/consumer"
42+
"github.com/uber/submitqueue/stovepipe/entity"
43+
"github.com/uber/submitqueue/stovepipe/extension/storage"
44+
"go.uber.org/zap"
45+
)
46+
47+
// topicSuffix is appended to a primary topic key to derive the corresponding DLQ topic
48+
// key. The queue extension's DefaultSubscriptionConfig also uses "_dlq" as the DLQ
49+
// topic suffix; keeping both in sync is intentional so a registered DLQ subscription's
50+
// topic name matches the controller's TopicKey().
51+
const topicSuffix = "_dlq"
52+
53+
// TopicKey returns the DLQ topic key for the given primary pipeline topic. It is
54+
// exported so the stovepipe wiring layer can build matching pairs without duplicating
55+
// the suffix literal.
56+
func TopicKey(main consumer.TopicKey) consumer.TopicKey {
57+
return consumer.TopicKey(string(main) + topicSuffix)
58+
}
59+
60+
// failRequest transitions request to RequestStateRecordedNotGreen if it is not already
61+
// in a terminal state. If the request had reached RequestStateProcessing — meaning process's
62+
// admit step already CAS-incremented the queue's in_flight_count for it — the queue's
63+
// slot is released first. Queue and Request are separate entities with no cross-entity
64+
// transaction, so the two writes cannot be atomic and the ordering picks which crash
65+
// failure mode we accept: a crash between the writes leaves the request non-terminal,
66+
// redelivery re-runs reconciliation, and releaseSlot (which tracks no per-request slot
67+
// ownership) decrements again — transiently over-admitting by one slot until the
68+
// under-count re-converges at releaseSlot's zero clamp. The reverse order would leak
69+
// the slot instead: redelivery skips terminal requests, permanently shrinking the
70+
// queue's capacity toward a wedge. Over-admission is the failure mode we prefer. See
71+
// doc/rfc/stovepipe/steps/process.md#in_flight_count-integrity for the broader
72+
// counter-drift story.
73+
func failRequest(ctx context.Context, store storage.Storage, logger *zap.SugaredLogger, requestID string) error {
74+
request, err := store.GetRequestStore().Get(ctx, requestID)
75+
if err != nil {
76+
if errors.Is(err, storage.ErrNotFound) {
77+
logger.Warnw("dlq reconcile: request not found, skipping",
78+
"request_id", requestID,
79+
)
80+
return nil
81+
}
82+
return fmt.Errorf("failed to get request %s: %w", requestID, err)
83+
}
84+
85+
if request.State.IsTerminal() {
86+
logger.Infow("dlq reconcile: request already terminal, skipping",
87+
"request_id", requestID,
88+
"state", string(request.State),
89+
)
90+
return nil
91+
}
92+
93+
if request.State == entity.RequestStateProcessing {
94+
if err := releaseSlot(ctx, store, logger, request.Queue); err != nil {
95+
return fmt.Errorf("failed to release queue slot for request %s: %w", requestID, err)
96+
}
97+
}
98+
99+
updated := request
100+
updated.State = entity.RequestStateRecordedNotGreen
101+
newVersion := request.Version + 1
102+
if err := store.GetRequestStore().Update(ctx, updated, request.Version, newVersion); err != nil {
103+
return fmt.Errorf("failed to update request %s state to recorded_not_green: %w", requestID, err)
104+
}
105+
logger.Infow("dlq reconcile: request forced terminal not-green",
106+
"request_id", requestID,
107+
"previous_state", string(request.State),
108+
)
109+
return nil
110+
}
111+
112+
// releaseSlot CAS-decrements the queue's in_flight_count, retrying on version
113+
// conflicts, mirroring process.Controller's own CAS-retry loop for queue updates.
114+
func releaseSlot(ctx context.Context, store storage.Storage, logger *zap.SugaredLogger, queueName string) error {
115+
queueStore := store.GetQueueStore()
116+
117+
for {
118+
queueRow, err := queueStore.Get(ctx, queueName)
119+
if err != nil {
120+
if errors.Is(err, storage.ErrNotFound) {
121+
logger.Warnw("dlq reconcile: queue not found, skipping slot release",
122+
"queue", queueName,
123+
)
124+
return nil
125+
}
126+
return fmt.Errorf("failed to get queue %s: %w", queueName, err)
127+
}
128+
129+
if queueRow.InFlightCount <= 0 {
130+
logger.Warnw("dlq reconcile: queue in_flight_count already at zero, skipping slot release",
131+
"queue", queueName,
132+
)
133+
return nil
134+
}
135+
136+
updated := queueRow
137+
updated.InFlightCount--
138+
newVersion := queueRow.Version + 1
139+
if err := queueStore.Update(ctx, updated, queueRow.Version, newVersion); err != nil {
140+
if errors.Is(err, storage.ErrVersionMismatch) {
141+
continue
142+
}
143+
return fmt.Errorf("failed to release slot for queue %s: %w", queueName, err)
144+
}
145+
logger.Infow("dlq reconcile: released queue slot",
146+
"queue", queueName,
147+
"in_flight_count", updated.InFlightCount,
148+
)
149+
return nil
150+
}
151+
}

0 commit comments

Comments
 (0)