Skip to content

Commit 6a6bf07

Browse files
refactor(messagequeue): make publish tenant explicit
## Summary ### Why? Tenant selects the physical MQ shard and is therefore a first-class publish input, not metadata to infer implicitly. Parsing MQ_TENANTS is service wiring behavior rather than a responsibility of the MySQL backend. ### What? - Require tenant explicitly in platform publish and hook APIs, propagate it to queue_name metadata, and reject conflicting metadata. - Pass each producer's authoritative queue through SubmitQueue, Runway, Stovepipe, hooks, and tests independently from partition keys. - Move required-tenant parsing into a shared service/messagequeue wiring package. ## Test Plan ✅ Focused unit tests across publish, hook, service wiring, Runway, Stovepipe, and SubmitQueue controllers ✅ `./tool/bazel build //...` ✅ `make fmt && make gazelle` Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 718d045 commit 6a6bf07

68 files changed

Lines changed: 227 additions & 191 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

doc/rfc/messagequeue-tenant-sharding.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Fair-share, orphan sweep, and idle-lease release run per `(tenant, topic)`, not
7676

7777
## Publish
7878

79-
`platform/publish` stamps `Message.Tenant` from context metadata (`queue_name`). Empty tenant on publish is rejected. `PartitionKey` is unchanged.
79+
Every `platform/publish` call supplies tenant explicitly. The package stamps `Message.Tenant` and mirrors it into `queue_name` delivery metadata, rejecting empty tenants and conflicting caller metadata. `PartitionKey` is unchanged.
8080

8181
## Wiring
8282

platform/extension/messagequeue/mysql/BUILD.bazel

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ go_library(
1616
"stores.go",
1717
"subscriber.go",
1818
"subscriber_heartbeat_store.go",
19-
"tenants.go",
2019
],
2120
importpath = "github.com/uber/submitqueue/platform/extension/messagequeue/mysql",
2221
visibility = ["//visibility:public"],
@@ -43,7 +42,6 @@ go_test(
4342
"sql_test.go",
4443
"subscriber_heartbeat_store_test.go",
4544
"subscriber_test.go",
46-
"tenants_test.go",
4745
],
4846
embed = [":go_default_library"],
4947
deps = [

platform/extension/messagequeue/mysql/tenants.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

platform/hook/publisher.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ import (
2323
"github.com/uber/submitqueue/platform/publish"
2424
)
2525

26-
// Publish sends one hook event to the domain's hook topic, partitioned by
27-
// partitionKey. The topic key is not a parameter: a domain runs a single hook
28-
// topic, and the caller's registry is what binds that key to a wire topic.
26+
// Publish sends one hook event to the domain's hook topic for tenant,
27+
// partitioned by partitionKey. The topic key is not a parameter: a domain runs
28+
// a single hook topic, and the caller's registry binds that key to a wire topic.
2929
//
3030
// The event id is the message id, so a redelivery republishing the same event
3131
// dedups into the original message instead of enqueuing a second one. Callers
@@ -37,6 +37,7 @@ import (
3737
func Publish(
3838
ctx context.Context,
3939
registry consumer.TopicRegistry,
40+
tenant string,
4041
event *basehook.HookEvent,
4142
partitionKey string,
4243
) error {
@@ -50,7 +51,7 @@ func Publish(
5051
}
5152

5253
if err := publish.Message(
53-
ctx, registry, basehook.TopicKeyHook, publish.IntentID(event.GetId()), body, partitionKey,
54+
ctx, registry, basehook.TopicKeyHook, tenant, publish.IntentID(event.GetId()), body, partitionKey,
5455
); err != nil {
5556
return fmt.Errorf("failed to publish hook event %s: %w", event.GetId(), err)
5657
}

platform/hook/publisher_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
const (
3232
testEventID = "stovepipe/validation.repository.recorded/request/7/0"
3333
testPartitionKey = "request/7"
34+
testTenant = "monorepo/main"
3435
)
3536

3637
func testEvent() *basehook.HookEvent {
@@ -69,12 +70,14 @@ func TestPublish(t *testing.T) {
6970
ctrl := gomock.NewController(t)
7071
registry, published := registryWithHookTopic(t, ctrl, nil)
7172

72-
require.NoError(t, Publish(context.Background(), registry, testEvent(), testPartitionKey))
73+
require.NoError(t, Publish(context.Background(), registry, testTenant, testEvent(), testPartitionKey))
7374

7475
// The event id is the message id, so a redelivery republishing the same
7576
// event dedups into the original message.
7677
assert.Equal(t, testEventID, published.ID)
7778
assert.Equal(t, testPartitionKey, published.PartitionKey)
79+
assert.Equal(t, testTenant, published.Tenant)
80+
assert.Equal(t, testTenant, published.Metadata[entityqueue.MetadataKeyQueueName])
7881

7982
decoded := &basehook.HookEvent{}
8083
require.NoError(t, basehook.Unmarshal(published.Payload, decoded))
@@ -105,7 +108,7 @@ func TestPublish_RejectsMalformedEvent(t *testing.T) {
105108
require.NoError(t, err)
106109

107110
// No Publish expectation: a malformed event must not reach the queue.
108-
require.Error(t, Publish(context.Background(), registry, tt.event, testPartitionKey))
111+
require.Error(t, Publish(context.Background(), registry, testTenant, tt.event, testPartitionKey))
109112
})
110113
}
111114
}
@@ -114,12 +117,12 @@ func TestPublish_PropagatesPublishFailure(t *testing.T) {
114117
ctrl := gomock.NewController(t)
115118
registry, _ := registryWithHookTopic(t, ctrl, errors.New("boom"))
116119

117-
require.Error(t, Publish(context.Background(), registry, testEvent(), testPartitionKey))
120+
require.Error(t, Publish(context.Background(), registry, testTenant, testEvent(), testPartitionKey))
118121
}
119122

120123
func TestPublish_FailsWhenHookTopicIsUnregistered(t *testing.T) {
121124
registry, err := consumer.NewTopicRegistry(nil)
122125
require.NoError(t, err)
123126

124-
require.Error(t, Publish(context.Background(), registry, testEvent(), testPartitionKey))
127+
require.Error(t, Publish(context.Background(), registry, testTenant, testEvent(), testPartitionKey))
125128
}

platform/publish/publish.go

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ import (
3535
"github.com/uber/submitqueue/platform/consumer"
3636
)
3737

38-
// Message publishes payload to the topic registered for key. Allowlisted
39-
// delivery context is propagated as message metadata.
38+
// Message publishes payload to the topic registered for key. Tenant selects
39+
// the shard and is propagated as the delivery's queue-name metadata.
4040
//
4141
// msgID selects the dedup behavior, so the caller must choose it deliberately.
42-
// The queue deduplicates on (topic, partition key, message ID) against every
42+
// The queue deduplicates on (tenant, topic, partition key, message ID) against every
4343
// row it has not garbage-collected yet, consumed ones included — a window with
4444
// no upper bound on a busy partition. A publish that collides is reported as a
4545
// success and writes nothing, and nothing retries it.
@@ -48,16 +48,18 @@ import (
4848
// this particular message exists for. A retry of the same cause then dedups,
4949
// which is what makes redelivery safe, while a new cause about the same entity
5050
// can never be swallowed by an older row.
51-
func Message(ctx context.Context, registry consumer.TopicRegistry, key consumer.TopicKey, msgID string, payload []byte, partitionKey string) error {
52-
return MessageWithMetadata(ctx, registry, key, msgID, payload, partitionKey, nil)
51+
func Message(ctx context.Context, registry consumer.TopicRegistry, key consumer.TopicKey, tenant, msgID string, payload []byte, partitionKey string) error {
52+
return MessageWithMetadata(ctx, registry, key, tenant, msgID, payload, partitionKey, nil)
5353
}
5454

5555
// MessageWithMetadata is Message with side-band message metadata (headers/attributes)
5656
// attached to the delivery. Use it to carry diagnostic context that is not part of
5757
// the payload — the backend persists and redelivers metadata alongside the message.
58-
// Allowlisted delivery context, currently only the queue name, is propagated unless
59-
// the caller supplies that metadata key explicitly.
60-
func MessageWithMetadata(ctx context.Context, registry consumer.TopicRegistry, key consumer.TopicKey, msgID string, payload []byte, partitionKey string, metadata map[string]string) error {
58+
// Tenant is also propagated as the delivery's queue-name context.
59+
func MessageWithMetadata(ctx context.Context, registry consumer.TopicRegistry, key consumer.TopicKey, tenant, msgID string, payload []byte, partitionKey string, metadata map[string]string) error {
60+
if tenant == "" {
61+
return fmt.Errorf("tenant is required")
62+
}
6163
q, ok := registry.Queue(key)
6264
if !ok {
6365
return fmt.Errorf("no queue registered for topic key %s", key)
@@ -67,29 +69,18 @@ func MessageWithMetadata(ctx context.Context, registry consumer.TopicRegistry, k
6769
return fmt.Errorf("no topic name registered for topic key %s", key)
6870
}
6971

70-
msg := entityqueue.NewMessage(msgID, payload, partitionKey, metadataFromContext(ctx, metadata))
71-
if tenant := msg.Metadata[entityqueue.MetadataKeyQueueName]; tenant != "" {
72-
msg.Tenant = tenant
73-
} else if queueName, ok := entityqueue.QueueName(ctx); ok {
74-
msg.Tenant = queueName
72+
if queueName, exists := metadata[entityqueue.MetadataKeyQueueName]; exists && queueName != tenant {
73+
return fmt.Errorf("queue-name metadata %q does not match tenant %q", queueName, tenant)
7574
}
76-
return q.Publisher().Publish(ctx, topicName, msg)
77-
}
78-
79-
func metadataFromContext(ctx context.Context, metadata map[string]string) map[string]string {
8075
metadata = maps.Clone(metadata)
81-
if _, exists := metadata[entityqueue.MetadataKeyQueueName]; exists {
82-
return metadata
83-
}
84-
queueName, ok := entityqueue.QueueName(ctx)
85-
if !ok || queueName == "" {
86-
return metadata
87-
}
8876
if metadata == nil {
8977
metadata = make(map[string]string)
9078
}
91-
metadata[entityqueue.MetadataKeyQueueName] = queueName
92-
return metadata
79+
metadata[entityqueue.MetadataKeyQueueName] = tenant
80+
81+
msg := entityqueue.NewMessage(msgID, payload, partitionKey, metadata)
82+
msg.Tenant = tenant
83+
return q.Publisher().Publish(ctx, topicName, msg)
9384
}
9485

9586
// IntentID names the occasion to publish rather than the entity published

platform/publish/publish_test.go

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,16 @@ func TestMessage(t *testing.T) {
5555
return nil
5656
})
5757

58-
err := Message(context.Background(), registry, testKey, "msg-1", []byte("payload"), "partition-1")
58+
err := Message(context.Background(), registry, testKey, "tenant-1", "msg-1", []byte("payload"), "partition-1")
5959
require.NoError(t, err)
60+
assert.Equal(t, "tenant-1", published.Tenant)
6061
assert.Equal(t, "msg-1", published.ID)
6162
assert.Equal(t, []byte("payload"), published.Payload)
6263
assert.Equal(t, "partition-1", published.PartitionKey)
63-
assert.Empty(t, published.Metadata)
64+
assert.Equal(t, "tenant-1", published.Metadata[entityqueue.MetadataKeyQueueName])
6465
}
6566

66-
func TestMessage_PropagatesQueueNameFromContext(t *testing.T) {
67+
func TestMessage_PropagatesTenantAsQueueName(t *testing.T) {
6768
ctrl := gomock.NewController(t)
6869
registry, publisher := newTestRegistry(t, ctrl)
6970

@@ -75,8 +76,7 @@ func TestMessage_PropagatesQueueNameFromContext(t *testing.T) {
7576
return nil
7677
})
7778

78-
ctx := entityqueue.WithQueueName(context.Background(), "monorepo/main")
79-
require.NoError(t, Message(ctx, registry, testKey, "msg-1", []byte("payload"), "partition-1"))
79+
require.NoError(t, Message(context.Background(), registry, testKey, "monorepo/main", "msg-1", []byte("payload"), "partition-1"))
8080
assert.Equal(t, "monorepo/main", published.Metadata[entityqueue.MetadataKeyQueueName])
8181
assert.Equal(t, "monorepo/main", published.Tenant)
8282
}
@@ -94,8 +94,7 @@ func TestMessageWithMetadata_MergesContextWithoutMutatingInput(t *testing.T) {
9494
})
9595

9696
metadata := map[string]string{"failure_reason": "build failed"}
97-
ctx := entityqueue.WithQueueName(context.Background(), "monorepo/main")
98-
require.NoError(t, MessageWithMetadata(ctx, registry, testKey, "msg-1", []byte("payload"), "partition-1", metadata))
97+
require.NoError(t, MessageWithMetadata(context.Background(), registry, testKey, "monorepo/main", "msg-1", []byte("payload"), "partition-1", metadata))
9998
assert.Equal(t, map[string]string{
10099
"failure_reason": "build failed",
101100
entityqueue.MetadataKeyQueueName: "monorepo/main",
@@ -104,30 +103,28 @@ func TestMessageWithMetadata_MergesContextWithoutMutatingInput(t *testing.T) {
104103
assert.Equal(t, map[string]string{"failure_reason": "build failed"}, metadata)
105104
}
106105

107-
func TestMessageWithMetadata_ExplicitQueueNameWins(t *testing.T) {
106+
func TestMessageWithMetadata_RejectsQueueNameDifferentFromTenant(t *testing.T) {
108107
ctrl := gomock.NewController(t)
109-
registry, publisher := newTestRegistry(t, ctrl)
110-
111-
var published entityqueue.Message
112-
publisher.EXPECT().
113-
Publish(gomock.Any(), "test-topic", gomock.Any()).
114-
DoAndReturn(func(_ context.Context, _ string, msg entityqueue.Message) error {
115-
published = msg
116-
return nil
117-
})
108+
registry, _ := newTestRegistry(t, ctrl)
118109

119-
ctx := entityqueue.WithQueueName(context.Background(), "inbound")
120110
metadata := map[string]string{entityqueue.MetadataKeyQueueName: "outbound"}
121-
require.NoError(t, MessageWithMetadata(ctx, registry, testKey, "msg-1", []byte("payload"), "partition-1", metadata))
122-
assert.Equal(t, "outbound", published.Metadata[entityqueue.MetadataKeyQueueName])
123-
assert.Equal(t, "outbound", published.Tenant)
111+
err := MessageWithMetadata(context.Background(), registry, testKey, "inbound", "msg-1", []byte("payload"), "partition-1", metadata)
112+
require.Error(t, err)
124113
}
125114

126115
func TestMessage_UnregisteredKey(t *testing.T) {
127116
ctrl := gomock.NewController(t)
128117
registry, _ := newTestRegistry(t, ctrl)
129118

130-
err := Message(context.Background(), registry, "unregistered-key", "msg-1", []byte("payload"), "partition-1")
119+
err := Message(context.Background(), registry, "unregistered-key", "tenant-1", "msg-1", []byte("payload"), "partition-1")
120+
require.Error(t, err)
121+
}
122+
123+
func TestMessage_RequiresTenant(t *testing.T) {
124+
ctrl := gomock.NewController(t)
125+
registry, _ := newTestRegistry(t, ctrl)
126+
127+
err := Message(context.Background(), registry, testKey, "", "msg-1", []byte("payload"), "partition-1")
131128
require.Error(t, err)
132129
}
133130

runway/controller/dlq/dlq.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func (c *Controller) publish(ctx context.Context, result *runwaymq.MergeResult,
165165
}
166166

167167
if err := publish.Message(ctx, c.registry, c.signalTopicKey,
168-
publish.IntentID(result.GetId(), "dlq"), payload, partitionKey); err != nil {
168+
result.GetQueueName(), publish.IntentID(result.GetId(), "dlq"), payload, partitionKey); err != nil {
169169
return fmt.Errorf("failed to publish message: %w", err)
170170
}
171171

runway/controller/dlq/dlq_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func TestProcess_DecodableRepublishesFailure(t *testing.T) {
112112
require.Len(t, *published, 1)
113113
got := (*published)[0]
114114
assert.Equal(t, "merge-signal", got.topic)
115+
assert.Equal(t, testQueue, got.msg.Tenant)
115116

116117
result := &runwaymq.MergeResult{}
117118
require.NoError(t, runwaymq.Unmarshal(got.msg.Payload, result))

runway/controller/merge/merge.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func (c *Controller) publish(ctx context.Context, key consumer.TopicKey, result
153153
return fmt.Errorf("failed to serialize merge result: %w", err)
154154
}
155155

156-
if err := publish.Message(ctx, c.registry, key, publish.IntentID(result.GetId()), payload, partitionKey); err != nil {
156+
if err := publish.Message(ctx, c.registry, key, result.GetQueueName(), publish.IntentID(result.GetId()), payload, partitionKey); err != nil {
157157
return fmt.Errorf("failed to publish message: %w", err)
158158
}
159159

0 commit comments

Comments
 (0)