Skip to content

Commit d8ee8ae

Browse files
committed
feat(runway): dlq reconciler for merge topics
## Summary ### Why? SubmitQueue records in-flight merge work before publishing and then waits for exactly one `MergeResult` echoing its correlation id. Runway is stateless and the sole responder on that id, so every request must resolve to a result — or the client waits forever. The primary controllers resolve what they can name: conflicts and invalid requests become a `FAILED` result, infrastructure faults are nacked for retry. But a fault that never recovers exhausts the retry budget and dead-letters. Nothing consumed those dead-letter topics, so the request produced no signal at all and the client's correlation id hung indefinitely. ### What? Adds `runway/controller/dlq`, a reconciler that subscribes to an inbound topic's `_dlq` queue and, for each dead-lettered `MergeRequest`, republishes a `FAILED` `MergeResult` echoing the correlation id to the corresponding signal topic. `dlq.TopicKey` derives the DLQ topic key from the primary one so the two stay in lockstep. Unlike the SubmitQueue and Stovepipe DLQ reconcilers this one writes no entity state — Runway has none, and the signal *is* the resolution. A payload that cannot be decoded carries no correlation id and is dropped rather than retried forever. Wires two instances in the server (one per inbound topic) on a dedicated consumer running under `errs.AlwaysRetryableProcessor`, so a transient publish failure retries indefinitely rather than dead-lettering the dead-letter. The DLQ consumer is started alongside the primary one and stopped with the same 30s drain on shutdown; both stop errors are joined into the exit status. ## Test Plan ✅ `bazel test //runway/...` — 5/5 pass, including new `//runway/controller/dlq` coverage for republish-on-dead-letter, the drop-undecodable-payload path, and publish-failure propagation ✅ `bazel build //service/runway/...` — wiring compiles ✅ `make gazelle`, `make fmt`
1 parent 930df02 commit d8ee8ae

8 files changed

Lines changed: 453 additions & 7 deletions

File tree

doc/rfc/runway/workflow.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,20 @@ The merge-conflict-check controller always publishes a result — even when all
6161

6262
The merge controller publishes a conflict result (and acks) when the merge detects a conflict; SubmitQueue handles rebatching. On infrastructure error it nacks for retry. On success it publishes per-step outcomes (output IDs of the revisions produced) so SubmitQueue can update its request state.
6363

64+
## Terminal failures and dead-lettering
65+
66+
Runway is stateless and the sole responder on the client's correlation id: SubmitQueue records the in-flight work before publishing and waits for exactly one `MergeResult` echoing that id. Every request must therefore resolve to a result — success or failure — or the client waits forever.
67+
68+
Failures split into two classes:
69+
70+
- **Named terminal outcomes** — a merge conflict or an invalid request (an unknown/unsupported strategy, a malformed change URI, or an invalid `PROMOTE` composition). These can never succeed on retry, so the controller publishes a `FAILED` `MergeResult` (with a reason) and acks, rather than nacking. The merger surfaces them as the `ErrConflict` / `ErrInvalidRequest` sentinels; `IsTerminal` is the single classification point.
71+
72+
- **Infrastructure faults** — fetch/network/auth failures, a push rejected for a reason other than a moved tip, and so on. These are nacked for retry.
73+
74+
An infrastructure fault that never recovers would exhaust retries and dead-letter. Because nothing consumed those dead-letter topics, such a request produced no signal and left the client's correlation id unresolved. Runway closes that gap with a **DLQ reconciler**: a dedicated consumer subscribes to the inbound topics' `_dlq` queues and, for each dead-lettered request, republishes a `FAILED` `MergeResult` (echoing the correlation id) to the corresponding signal topic. Unlike the SubmitQueue/Stovepipe DLQ reconcilers it writes no entity state — the signal is the resolution. It runs under an always-retryable error policy so a transient publish failure retries indefinitely rather than dead-lettering again. A payload that cannot even be decoded carries no correlation id and is dropped.
75+
76+
Together these guarantee the client's correlation id always resolves: the primary controllers handle what they can name, and the reconciler is the backstop for everything else.
77+
6478
## Idempotency
6579

6680
Runway has no persistent state — no request store, no job store, no database. Idempotency is achieved through the VCS contract: merge detects already-pushed changes (revisions reachable from HEAD) and treats them as already-landed. Merge-conflict check is read-only and naturally idempotent.

runway/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,5 @@ Each controller deserializes the `MergeRequest`, obtains a `Merger` for the requ
1616
## Failure handling
1717

1818
A merge outcome the controller can name is published as a `FAILED` result and acked, not retried: a merge conflict (`merger.ErrConflict`) or an invalid request (`merger.ErrInvalidRequest` — unknown strategy, malformed change URI, invalid PROMOTE composition). The `merger.IsTerminal` helper draws that line. Any other error is an infrastructure fault and is nacked for retry.
19+
20+
Because Runway is stateless and the sole responder on the client's correlation id, a request that exhausts retries (or hits an unexpected fault) must still resolve the client. The inbound topics dead-letter by default; the [`dlq`](controller/dlq) reconciler drains those `_dlq` topics and republishes a `FAILED` `MergeResult` to the signal topic so the correlation id never hangs.

runway/controller/dlq/BUILD.bazel

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["dlq.go"],
6+
importpath = "github.com/uber/submitqueue/runway/controller/dlq",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//api/runway/messagequeue:go_default_library",
10+
"//api/runway/messagequeue/protopb:go_default_library",
11+
"//platform/base/messagequeue:go_default_library",
12+
"//platform/consumer:go_default_library",
13+
"//platform/metrics: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 = ["dlq_test.go"],
22+
embed = [":go_default_library"],
23+
deps = [
24+
"//api/runway/messagequeue:go_default_library",
25+
"//api/runway/messagequeue/protopb:go_default_library",
26+
"//platform/base/messagequeue:go_default_library",
27+
"//platform/consumer:go_default_library",
28+
"//platform/extension/messagequeue/mock:go_default_library",
29+
"@com_github_stretchr_testify//assert:go_default_library",
30+
"@com_github_stretchr_testify//require:go_default_library",
31+
"@com_github_uber_go_tally//:go_default_library",
32+
"@org_uber_go_mock//gomock:go_default_library",
33+
"@org_uber_go_zap//zaptest:go_default_library",
34+
],
35+
)

runway/controller/dlq/dlq.go

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
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 reconciles dead-lettered merge requests back to the client.
16+
//
17+
// Runway's inbound merge topics dead-letter a message after the primary
18+
// controller returns a non-retryable error or exhausts retries on a retryable
19+
// one. Runway is stateless and the sole responder on the client's correlation
20+
// id, so a dead-lettered request that produced no signal would leave the client
21+
// (SubmitQueue) waiting forever. Expected outcomes — conflicts and invalid
22+
// requests — are already published as FAILED results by the primary controllers;
23+
// this reconciler is the backstop for everything else (unexpected faults, retry
24+
// exhaustion).
25+
//
26+
// On each delivery from a `{topic}_dlq` topic it decodes the same MergeRequest
27+
// payload the primary controller consumes (the queue preserves the bytes
28+
// verbatim), and republishes a FAILED MergeResult — echoing the correlation id
29+
// — to the corresponding signal topic. Unlike the stovepipe/orchestrator DLQ
30+
// reconcilers it writes no entity state (Runway has none); the signal is the
31+
// resolution. A payload that cannot be decoded carries no correlation id to
32+
// resolve, so it is logged and acked (dropped).
33+
//
34+
// Wire this controller on a dedicated consumer built with
35+
// errs.AlwaysRetryableProcessor so a transient publish failure retries forever
36+
// rather than dead-lettering again (the DLQ topic has no DLQ of its own).
37+
package dlq
38+
39+
import (
40+
"context"
41+
"fmt"
42+
43+
"github.com/uber-go/tally"
44+
runwaymq "github.com/uber/submitqueue/api/runway/messagequeue"
45+
runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb"
46+
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
47+
"github.com/uber/submitqueue/platform/consumer"
48+
"github.com/uber/submitqueue/platform/metrics"
49+
"go.uber.org/zap"
50+
)
51+
52+
// topicSuffix is appended to a primary topic key to derive its DLQ topic key.
53+
// It matches DefaultSubscriptionConfig's DLQ TopicSuffix so a registered DLQ
54+
// subscription's topic name matches the controller's TopicKey().
55+
const topicSuffix = "_dlq"
56+
57+
// TopicKey returns the DLQ topic key for a primary merge topic. Exported so the
58+
// wiring layer builds matching pairs without duplicating the suffix literal.
59+
func TopicKey(main consumer.TopicKey) consumer.TopicKey {
60+
return consumer.TopicKey(string(main) + topicSuffix)
61+
}
62+
63+
// Verify Controller implements consumer.Controller at compile time.
64+
var _ consumer.Controller = (*Controller)(nil)
65+
66+
// Controller consumes a merge topic's dead-letter queue and republishes a
67+
// terminal FAILED result to the corresponding signal topic.
68+
type Controller struct {
69+
logger *zap.SugaredLogger
70+
metricsScope tally.Scope
71+
registry consumer.TopicRegistry
72+
topicKey consumer.TopicKey
73+
signalTopicKey consumer.TopicKey
74+
consumerGroup string
75+
}
76+
77+
// Params are the parameters for creating a new DLQ reconciler.
78+
type Params struct {
79+
// TopicKey is the dead-letter topic this controller consumes
80+
// (typically dlq.TopicKey(<primary topic>)).
81+
TopicKey consumer.TopicKey
82+
// SignalTopicKey is the signal topic the FAILED result is published to.
83+
SignalTopicKey consumer.TopicKey
84+
ConsumerGroup string
85+
86+
Registry consumer.TopicRegistry
87+
88+
Scope tally.Scope
89+
Logger *zap.SugaredLogger
90+
}
91+
92+
// NewController creates a DLQ reconciler for a merge topic's dead-letter queue.
93+
func NewController(p Params) *Controller {
94+
return &Controller{
95+
logger: p.Logger.Named("merge_dlq_controller"),
96+
metricsScope: p.Scope.SubScope("merge_dlq_controller"),
97+
registry: p.Registry,
98+
topicKey: p.TopicKey,
99+
signalTopicKey: p.SignalTopicKey,
100+
consumerGroup: p.ConsumerGroup,
101+
}
102+
}
103+
104+
// Process decodes the dead-lettered merge request and republishes a FAILED
105+
// result to the signal topic so the client's correlation id resolves. Returns
106+
// nil to ack; an error to nack (retried indefinitely under
107+
// AlwaysRetryableProcessor).
108+
func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) error {
109+
const opName = "process"
110+
111+
msg := delivery.Message()
112+
meta := delivery.Metadata()
113+
114+
request := &runwaymq.MergeRequest{}
115+
if err := runwaymq.Unmarshal(msg.Payload, request); err != nil {
116+
// No correlation id is recoverable, so there is nothing to resolve for
117+
// the client. Log and ack (drop) rather than retry forever.
118+
metrics.NamedCounter(c.metricsScope, opName, "undecodable", 1)
119+
c.logger.Errorw("dlq reconcile: undecodable merge request, dropping",
120+
"err", err,
121+
"original_topic", meta["dlq.original_topic"],
122+
)
123+
return nil
124+
}
125+
126+
reason := meta["dlq.last_error"]
127+
if reason == "" {
128+
reason = "runway failed to process the merge request"
129+
}
130+
131+
c.logger.Warnw("dlq reconcile: publishing terminal failure",
132+
"id", request.GetId(),
133+
"queue_name", request.GetQueueName(),
134+
"original_topic", meta["dlq.original_topic"],
135+
"failure_count", meta["dlq.failure_count"],
136+
"last_error", reason,
137+
)
138+
139+
result := &runwaymq.MergeResult{
140+
Id: request.GetId(),
141+
Outcome: runwaypb.Outcome_FAILED,
142+
Reason: fmt.Sprintf("dead-lettered: %s", reason),
143+
}
144+
145+
if err := c.publish(ctx, result, msg.PartitionKey); err != nil {
146+
metrics.NamedCounter(c.metricsScope, opName, "publish_errors", 1)
147+
return fmt.Errorf("failed to publish dlq failure result for %s: %w", request.GetId(), err)
148+
}
149+
150+
metrics.NamedCounter(c.metricsScope, opName, "reconciled", 1)
151+
return nil
152+
}
153+
154+
// publish serializes a MergeResult and publishes it to the signal topic.
155+
func (c *Controller) publish(ctx context.Context, result *runwaymq.MergeResult, partitionKey string) error {
156+
payload, err := runwaymq.Marshal(result)
157+
if err != nil {
158+
return fmt.Errorf("failed to serialize merge result: %w", err)
159+
}
160+
161+
msg := entityqueue.NewMessage(result.GetId(), payload, partitionKey, nil)
162+
163+
q, ok := c.registry.Queue(c.signalTopicKey)
164+
if !ok {
165+
return fmt.Errorf("no queue registered for topic key %s", c.signalTopicKey)
166+
}
167+
168+
topicName, ok := c.registry.TopicName(c.signalTopicKey)
169+
if !ok {
170+
return fmt.Errorf("no topic name registered for topic key %s", c.signalTopicKey)
171+
}
172+
173+
if err := q.Publisher().Publish(ctx, topicName, msg); err != nil {
174+
return fmt.Errorf("failed to publish message: %w", err)
175+
}
176+
177+
return nil
178+
}
179+
180+
// Name returns the controller name for logging and metrics.
181+
func (c *Controller) Name() string {
182+
return string(c.topicKey)
183+
}
184+
185+
// TopicKey returns the dead-letter topic key this controller subscribes to.
186+
func (c *Controller) TopicKey() consumer.TopicKey {
187+
return c.topicKey
188+
}
189+
190+
// ConsumerGroup returns the consumer group for offset tracking.
191+
func (c *Controller) ConsumerGroup() string {
192+
return c.consumerGroup
193+
}

runway/controller/dlq/dlq_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
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
16+
17+
import (
18+
"context"
19+
"testing"
20+
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
"github.com/uber-go/tally"
24+
runwaymq "github.com/uber/submitqueue/api/runway/messagequeue"
25+
runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb"
26+
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
27+
"github.com/uber/submitqueue/platform/consumer"
28+
queuemock "github.com/uber/submitqueue/platform/extension/messagequeue/mock"
29+
"go.uber.org/mock/gomock"
30+
"go.uber.org/zap/zaptest"
31+
)
32+
33+
const (
34+
testID = "test-queue/1"
35+
testQueue = "test-queue"
36+
testPartitionKey = "test-queue"
37+
)
38+
39+
// publishedMsg captures a message published to the signal topic.
40+
type publishedMsg struct {
41+
topic string
42+
msg entityqueue.Message
43+
}
44+
45+
func newDelivery(t *testing.T, ctrl *gomock.Controller, payload []byte, meta map[string]string) *queuemock.MockDelivery {
46+
t.Helper()
47+
msg := entityqueue.NewMessage(testID, payload, testPartitionKey, nil)
48+
d := queuemock.NewMockDelivery(ctrl)
49+
d.EXPECT().Message().Return(msg).AnyTimes()
50+
d.EXPECT().Metadata().Return(meta).AnyTimes()
51+
d.EXPECT().Attempt().Return(1).AnyTimes()
52+
return d
53+
}
54+
55+
// newRegistry builds a registry whose signal-topic publisher records every
56+
// message it receives into the returned slice pointer.
57+
func newRegistry(t *testing.T, ctrl *gomock.Controller) (consumer.TopicRegistry, *[]publishedMsg) {
58+
t.Helper()
59+
var published []publishedMsg
60+
pub := queuemock.NewMockPublisher(ctrl)
61+
pub.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
62+
func(_ context.Context, topic string, msg entityqueue.Message) error {
63+
published = append(published, publishedMsg{topic: topic, msg: msg})
64+
return nil
65+
},
66+
).AnyTimes()
67+
68+
q := queuemock.NewMockQueue(ctrl)
69+
q.EXPECT().Publisher().Return(pub).AnyTimes()
70+
71+
registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{
72+
{Key: runwaymq.TopicKeyMergeSignal, Name: "merge-signal", Queue: q},
73+
})
74+
require.NoError(t, err)
75+
return registry, &published
76+
}
77+
78+
func newController(t *testing.T, registry consumer.TopicRegistry) *Controller {
79+
t.Helper()
80+
return NewController(Params{
81+
Logger: zaptest.NewLogger(t).Sugar(),
82+
Scope: tally.NoopScope,
83+
Registry: registry,
84+
TopicKey: TopicKey(runwaymq.TopicKeyMerge),
85+
SignalTopicKey: runwaymq.TopicKeyMergeSignal,
86+
ConsumerGroup: "runway-merge-dlq",
87+
})
88+
}
89+
90+
func TestProcess_DecodableRepublishesFailure(t *testing.T) {
91+
ctrl := gomock.NewController(t)
92+
registry, published := newRegistry(t, ctrl)
93+
controller := newController(t, registry)
94+
95+
req := &runwaymq.MergeRequest{
96+
Id: testID,
97+
QueueName: testQueue,
98+
Steps: []*runwaymq.MergeStep{{StepId: "step-1"}},
99+
}
100+
payload, err := runwaymq.Marshal(req)
101+
require.NoError(t, err)
102+
103+
meta := map[string]string{
104+
"dlq.last_error": "boom: connection refused",
105+
"dlq.original_topic": "runway-merge",
106+
}
107+
delivery := newDelivery(t, ctrl, payload, meta)
108+
109+
require.NoError(t, controller.Process(context.Background(), delivery))
110+
111+
require.Len(t, *published, 1)
112+
got := (*published)[0]
113+
assert.Equal(t, "merge-signal", got.topic)
114+
115+
result := &runwaymq.MergeResult{}
116+
require.NoError(t, runwaymq.Unmarshal(got.msg.Payload, result))
117+
assert.Equal(t, testID, result.Id)
118+
assert.Equal(t, runwaypb.Outcome_FAILED, result.Outcome)
119+
assert.Contains(t, result.Reason, "boom: connection refused")
120+
}
121+
122+
func TestProcess_UndecodableAcksAndPublishesNothing(t *testing.T) {
123+
ctrl := gomock.NewController(t)
124+
registry, published := newRegistry(t, ctrl)
125+
controller := newController(t, registry)
126+
127+
delivery := newDelivery(t, ctrl, []byte("{bad"), map[string]string{"dlq.original_topic": "runway-merge"})
128+
129+
require.NoError(t, controller.Process(context.Background(), delivery))
130+
assert.Empty(t, *published)
131+
}

0 commit comments

Comments
 (0)