Skip to content

Commit 91ddcce

Browse files
authored
feat(submitqueue): cut over internal queues to protojson (#703)
## Summary ### Why? SubmitQueue's pipeline still serialized Go structs with encoding/json, while Stovepipe and the queue-contract RFC use proto3 + protojson with one message per topic key. That left the internal contract without a language-neutral schema and made additive field growth a shared-struct change. ### What? Add submitqueue/core/messagequeue (proto payloads, protojson glue, TopicKey constants, and entity mapping). Gateway and orchestrator publish and consume the message bound to each topic. MarshalID/UnmarshalID take the topic key so a later field is not discarded onto the wrong type. Proto filenames that collide in the protobuf registry are prefixed (submitqueuemerge, submitqueuebuild, submitqueuebuildsignal). Queue-only ToBytes/FromBytes helpers are removed from entities. Hard cutover: drain or drop in-flight start and log messages; id-only topics stay {id,queue}. ## Test Plan ✅ `bazel test` of `//submitqueue/core/messagequeue:go_default_test` plus gateway and orchestrator/DLQ controller tests ✅ `bazel test //test/e2e/submitqueue:go_default_test --test_filter=TestE2EIntegration` ✅ `bazel test //test/e2e/submitqueue:go_default_test --test_filter=TestGitMergeE2E`
1 parent 9746526 commit 91ddcce

108 files changed

Lines changed: 3292 additions & 609 deletions

File tree

Some content is hidden

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

AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ Paths follow the directory layout: shared packages live under `platform/` at the
168168
- Queue contracts: external `github.com/uber/submitqueue/api/{domain}/messagequeue`; internal `github.com/uber/submitqueue/{domain}/core/messagequeue`
169169
- Domain entities: `github.com/uber/submitqueue/{domain}/entity` (e.g. `.../submitqueue/entity`)
170170
- Domain extensions: `github.com/uber/submitqueue/{domain}/extension/{ext}[/{impl}]` (e.g. `.../submitqueue/extension/storage/mysql`)
171-
- Cross-domain consumer framework: `github.com/uber/submitqueue/platform/consumer`; internal topic keys live with the owning domain contract (for example `submitqueue/core/topickey` and `stovepipe/core/messagequeue`); external queue topic keys live with their published contract (for example `api/runway/messagequeue`)
171+
- Cross-domain consumer framework: `github.com/uber/submitqueue/platform/consumer`; internal topic keys live with the owning domain contract (for example `submitqueue/core/messagequeue` and `stovepipe/core/messagequeue`); external queue topic keys live with their published contract (for example `api/runway/messagequeue`)
172172
- Domain-internal infra: `github.com/uber/submitqueue/{domain}/core/{pkg}` (e.g. `.../submitqueue/core/request`)
173173
- Shared entities: `github.com/uber/submitqueue/platform/base/{pkg}` (e.g. `.../platform/base/messagequeue`)
174174
- Shared extensions: `github.com/uber/submitqueue/platform/extension/{ext}[/{impl}]` (e.g. `.../platform/extension/messagequeue/mysql`)
@@ -200,7 +200,7 @@ New queue contracts are defined in **proto3** (`.proto` under `proto/`, generate
200200
201201
The message types are generated; the contract package adds only generic `protojson` glue — `Marshal(m)` / `Unmarshal[T](b, m)` — owning the wire conventions: `UseProtoNames` (snake_case fields), UPPER_SNAKE enum values, int64-as-string, unknown fields discarded on read (additive evolution). The topic key(s) carrying a message are declared on the message via the `topic_keys` proto option — a `google.protobuf.MessageOptions` extension defined in `api/base/messagequeue`. A topic key is a stable logical name, not a concrete wire topic; each implementer maps it to its backend's topic name, and a `TopicKeys(msg)` reflection helper reads the option back. It is contract metadata, not the hot path — publish/consume still routes on `consumer.TopicKey` + `TopicRegistry`. The contract package owns both halves: the proto payload and the `TopicKey` constants for its topic keys. A contract test round-trips the payloads and asserts every topic key is bound to exactly one message. Shared field types (`Change`, `Strategy`) are shared protos under `api/base/{change,mergestrategy}`. `api/runway/messagequeue/` is the reference example.
202202
203-
SubmitQueue's internal pipeline predates the proto-backed convention. It continues to serialize domain entities with `encoding/json` and declares its logical keys in `submitqueue/core/topickey/`. Do not convert or mix these wire formats incidentally; treat migration as an explicit compatibility change.
203+
SubmitQueue's internal pipeline contract lives at `submitqueue/core/messagequeue/` (proto3 + protojson, one message per topic key). `MarshalID`/`UnmarshalID` take the topic key so consume uses the bound message. Proto filenames that would collide with another domain in the protobuf registry are prefixed (`submitqueuemerge.proto`, `submitqueuebuild.proto`, `submitqueuebuildsignal.proto`). `submitqueue/core/topickey` re-exports the topic-key constants from that package.
204204
205205
### Naming Conventions
206206

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ GOIMPORTS_VERSION ?= v0.33.0
3636
# (the out_dir convention in tool/proto/BUILD.bazel) and copied back here. A
3737
# package may hold multiple .proto files (e.g. an RPC contract plus messagequeue
3838
# contracts); all generated stubs land in the same protopb/ dir.
39-
PROTO_PACKAGES = api/base/change api/base/hook api/base/mergestrategy api/base/messagequeue api/runway/messagequeue api/runway api/submitqueue/gateway api/submitqueue/orchestrator api/stovepipe stovepipe/core/messagequeue
39+
PROTO_PACKAGES = api/base/change api/base/hook api/base/mergestrategy api/base/messagequeue api/runway/messagequeue api/runway api/submitqueue/gateway api/submitqueue/orchestrator api/stovepipe stovepipe/core/messagequeue submitqueue/core/messagequeue
4040

4141
# Set REPO_ROOT for docker-compose
4242
export REPO_ROOT := $(shell pwd)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = [
6+
"id.go",
7+
"map.go",
8+
"messagequeue.go",
9+
"topics.go",
10+
],
11+
importpath = "github.com/uber/submitqueue/submitqueue/core/messagequeue",
12+
visibility = [
13+
"//service/submitqueue:__subpackages__",
14+
"//submitqueue:__subpackages__",
15+
"//test:__subpackages__",
16+
],
17+
deps = [
18+
"//api/base/change/protopb:go_default_library",
19+
"//api/base/mergestrategy/protopb:go_default_library",
20+
"//api/base/messagequeue/protopb:go_default_library",
21+
"//platform/base/change:go_default_library",
22+
"//platform/base/mergestrategy:go_default_library",
23+
"//platform/consumer:go_default_library",
24+
"//submitqueue/core/messagequeue/protopb:go_default_library",
25+
"//submitqueue/entity:go_default_library",
26+
"@org_golang_google_protobuf//encoding/protojson:go_default_library",
27+
"@org_golang_google_protobuf//proto:go_default_library",
28+
],
29+
)
30+
31+
go_test(
32+
name = "go_default_test",
33+
srcs = ["messagequeue_test.go"],
34+
embed = [":go_default_library"],
35+
deps = [
36+
"//platform/base/change:go_default_library",
37+
"//platform/base/mergestrategy:go_default_library",
38+
"//submitqueue/entity:go_default_library",
39+
"@com_github_stretchr_testify//assert:go_default_library",
40+
"@com_github_stretchr_testify//require:go_default_library",
41+
"@org_golang_google_protobuf//proto:go_default_library",
42+
],
43+
)
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# SubmitQueue internal message-queue contract
2+
3+
Wire payloads for the queues internal to the SubmitQueue pipeline (gateway and orchestrator). It is **internal** — used only within the SubmitQueue domain — so it lives under `submitqueue/core` rather than `api/` (Bazel visibility keeps it domain-scoped).
4+
5+
Payloads are defined in proto3 (`proto/`, generated into `protopb/`) and serialized as **protobuf JSON** (protojson), so the MySQL-backed queue keeps storing self-describing JSON. The contract package adds protojson glue (`Marshal`/`Unmarshal`), `TopicKeys`, the pipeline `TopicKey` constants, and helpers that map generated payloads to `submitqueue/entity` types. `MarshalID`/`UnmarshalID` take the topic key so the bound message is used; unmarshalling through a different type would drop fields added later (`DiscardUnknown`). Each payload declares the topic key that carries it via the `topic_keys` proto option (defined in `api/base/messagequeue`); a contract test round-trips every payload and asserts each topic key is bound to exactly one message.
6+
7+
Shared field types `Change` and `Strategy` come from `api/base/change` and `api/base/mergestrategy`.
8+
9+
## Stages
10+
11+
Each topic key has its own message, even when the first version is only an id and a queue, so a stage can grow fields without touching others.
12+
13+
- **start** (`TopicKeyStart`, `Start`) — gateway publishes the minted request id and land inputs; start persists a `Request`. Full payload: this seam crosses services.
14+
- **cancel** (`TopicKeyCancel`, `Cancel`) — gateway publishes the request id to cancel; cancel reloads the `Request`. Full payload across the gateway/orchestrator seam.
15+
- **validate** (`TopicKeyValidate`, `Validate`) — start publishes the request id; validate reloads the `Request`.
16+
- **batch** (`TopicKeyBatch`, `Batch`) — landconflictsignal publishes the request id; batch reloads the `Request`.
17+
- **dependency-analysis** (`TopicKeyDependencyAnalysis`, `DependencyAnalysis`) — batch publishes the batch id; partitioned by queue.
18+
- **speculate** (`TopicKeySpeculate`, `Speculate`) — dependency-analysis (and later stages) publish the batch id.
19+
- **build** (`TopicKeyBuild`, `Build` in `submitqueuebuild.proto`) — speculate publishes a **batch** id; build reloads the `Batch`. The proto filename is not `build.proto` so it does not collide with Stovepipe's `stovepipe/core/messagequeue/proto/build.proto` in the protobuf filename registry.
20+
- **buildsignal** (`TopicKeyBuildSignal`, `BuildSignal` in `submitqueuebuildsignal.proto`) — build publishes a **build** id; buildsignal polls and may hold the delivery. Same filename-registry reason as build.
21+
- **submitqueue-land** (`TopicKeyLand`, `Merge` in `submitqueuemerge.proto`) — speculate publishes a batch id; land reloads the `Batch` before handing work to Runway. The proto filename is not `merge.proto`/`land.proto` so it does not collide with Runway's `api/runway/messagequeue/proto/merge.proto` in the protobuf filename registry.
22+
- **conclude** (`TopicKeyConclude`, `Conclude`) — speculate/landsignal publish a batch id. A failed batch's reason travels in message metadata (`MetadataKeyFailureReason`), not the payload.
23+
- **log** (`TopicKeyLog`, `Log`) — orchestrator publishes a full request-log entry; the gateway materializes it. `type`, `status`, and `event` are open strings matching the domain vocabularies.
24+
25+
In-boundary stages (validate through conclude, except start/cancel/log) put only an id on the queue because producer and consumer share storage.
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
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 messagequeue
16+
17+
import (
18+
"fmt"
19+
20+
"google.golang.org/protobuf/proto"
21+
22+
"github.com/uber/submitqueue/submitqueue/entity"
23+
)
24+
25+
type idPayload interface {
26+
proto.Message
27+
GetId() string
28+
GetQueue() string
29+
}
30+
31+
func idOnlyMessage(key TopicKey, id, queue string) (idPayload, error) {
32+
switch key {
33+
case TopicKeyValidate:
34+
return &Validate{Id: id, Queue: queue}, nil
35+
case TopicKeyBatch:
36+
return &Batch{Id: id, Queue: queue}, nil
37+
case TopicKeyDependencyAnalysis:
38+
return &DependencyAnalysis{Id: id, Queue: queue}, nil
39+
case TopicKeySpeculate:
40+
return &Speculate{Id: id, Queue: queue}, nil
41+
case TopicKeyBuild:
42+
return &Build{Id: id, Queue: queue}, nil
43+
case TopicKeyBuildSignal:
44+
return &BuildSignal{Id: id, Queue: queue}, nil
45+
case TopicKeyLand:
46+
return &Merge{Id: id, Queue: queue}, nil
47+
case TopicKeyConclude:
48+
return &Conclude{Id: id, Queue: queue}, nil
49+
default:
50+
return nil, fmt.Errorf("topic %q does not carry an id-only payload", key)
51+
}
52+
}
53+
54+
// MarshalID serializes the id-only payload bound to key. Start, cancel, and log
55+
// are not id-only; callers marshal those messages directly.
56+
func MarshalID(key TopicKey, id, queue string) ([]byte, error) {
57+
m, err := idOnlyMessage(key, id, queue)
58+
if err != nil {
59+
return nil, err
60+
}
61+
return Marshal(m)
62+
}
63+
64+
// UnmarshalID reads id and queue from the id-only payload bound to key.
65+
// Consumers pass the topic they subscribe to so a field added to that message
66+
// is decoded rather than discarded as unknown on a different type.
67+
func UnmarshalID(key TopicKey, b []byte) (id, queue string, err error) {
68+
m, err := idOnlyMessage(key, "", "")
69+
if err != nil {
70+
return "", "", err
71+
}
72+
if err := Unmarshal(b, m); err != nil {
73+
return "", "", err
74+
}
75+
return m.GetId(), m.GetQueue(), nil
76+
}
77+
78+
// UnmarshalLandRequest reads a start payload into the gateway-owned land request.
79+
func UnmarshalLandRequest(b []byte) (entity.LandRequest, error) {
80+
m := &Start{}
81+
if err := Unmarshal(b, m); err != nil {
82+
return entity.LandRequest{}, err
83+
}
84+
return LandRequestFromStart(m), nil
85+
}
86+
87+
// UnmarshalCancelRequest reads a cancel payload into the domain cancellation.
88+
func UnmarshalCancelRequest(b []byte) (entity.CancelRequest, error) {
89+
m := &Cancel{}
90+
if err := Unmarshal(b, m); err != nil {
91+
return entity.CancelRequest{}, err
92+
}
93+
return CancelToEntity(m), nil
94+
}
95+
96+
// UnmarshalRequestLog reads a log payload into a request-log entry.
97+
func UnmarshalRequestLog(b []byte) (entity.RequestLog, error) {
98+
m := &Log{}
99+
if err := Unmarshal(b, m); err != nil {
100+
return entity.RequestLog{}, err
101+
}
102+
return LogToEntity(m), nil
103+
}
104+
105+
// UnmarshalRequestID reads a request-scoped id-only payload (validate or batch).
106+
func UnmarshalRequestID(key TopicKey, b []byte) (entity.RequestID, error) {
107+
switch key {
108+
case TopicKeyValidate, TopicKeyBatch:
109+
default:
110+
return entity.RequestID{}, fmt.Errorf("topic %q does not carry a request-id payload", key)
111+
}
112+
id, queue, err := UnmarshalID(key, b)
113+
if err != nil {
114+
return entity.RequestID{}, err
115+
}
116+
return entity.RequestID{ID: id, Queue: queue}, nil
117+
}
118+
119+
// UnmarshalBatchID reads a batch-scoped id-only payload.
120+
func UnmarshalBatchID(key TopicKey, b []byte) (entity.BatchID, error) {
121+
switch key {
122+
case TopicKeyDependencyAnalysis, TopicKeySpeculate, TopicKeyBuild, TopicKeyLand, TopicKeyConclude:
123+
default:
124+
return entity.BatchID{}, fmt.Errorf("topic %q does not carry a batch-id payload", key)
125+
}
126+
id, queue, err := UnmarshalID(key, b)
127+
if err != nil {
128+
return entity.BatchID{}, err
129+
}
130+
return entity.BatchID{ID: id, Queue: queue}, nil
131+
}
132+
133+
// UnmarshalBuildID reads a buildsignal payload.
134+
func UnmarshalBuildID(key TopicKey, b []byte) (entity.BuildID, error) {
135+
if key != TopicKeyBuildSignal {
136+
return entity.BuildID{}, fmt.Errorf("topic %q does not carry a build-id payload", key)
137+
}
138+
id, queue, err := UnmarshalID(key, b)
139+
if err != nil {
140+
return entity.BuildID{}, err
141+
}
142+
return entity.BuildID{ID: id, Queue: queue}, nil
143+
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
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 messagequeue
16+
17+
import (
18+
changepb "github.com/uber/submitqueue/api/base/change/protopb"
19+
strategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
20+
"github.com/uber/submitqueue/platform/base/change"
21+
"github.com/uber/submitqueue/platform/base/mergestrategy"
22+
"github.com/uber/submitqueue/submitqueue/entity"
23+
)
24+
25+
// StartFromLandRequest copies a gateway-owned land request onto the start payload.
26+
func StartFromLandRequest(r entity.LandRequest) *Start {
27+
return &Start{
28+
Id: r.ID,
29+
Queue: r.Queue,
30+
Change: &changepb.Change{Uris: append([]string{}, r.Change.URIs...)},
31+
LandStrategy: landStrategyToProto(r.LandStrategy),
32+
}
33+
}
34+
35+
// LandRequestFromStart copies a start payload onto the gateway-owned land request.
36+
func LandRequestFromStart(m *Start) entity.LandRequest {
37+
var uris []string
38+
if m.GetChange() != nil {
39+
uris = append([]string{}, m.GetChange().GetUris()...)
40+
}
41+
return entity.LandRequest{
42+
ID: m.GetId(),
43+
Queue: m.GetQueue(),
44+
Change: change.Change{URIs: uris},
45+
LandStrategy: landStrategyFromProto(m.GetLandStrategy()),
46+
}
47+
}
48+
49+
// CancelFromEntity copies a cancellation onto the cancel payload.
50+
func CancelFromEntity(r entity.CancelRequest) *Cancel {
51+
return &Cancel{Id: r.ID, Queue: r.Queue, Reason: r.Reason}
52+
}
53+
54+
// CancelToEntity copies a cancel payload onto the domain cancellation.
55+
func CancelToEntity(m *Cancel) entity.CancelRequest {
56+
return entity.CancelRequest{ID: m.GetId(), Queue: m.GetQueue(), Reason: m.GetReason()}
57+
}
58+
59+
// LogFromEntity copies a request-log entry onto the log payload.
60+
func LogFromEntity(r entity.RequestLog) *Log {
61+
return &Log{
62+
RequestId: r.RequestID,
63+
Queue: r.Queue,
64+
TimestampMs: r.TimestampMs,
65+
Type: string(r.Type),
66+
Status: string(r.Status),
67+
Event: string(r.Event),
68+
RequestVersion: r.RequestVersion,
69+
LastError: r.LastError,
70+
Metadata: r.Metadata,
71+
}
72+
}
73+
74+
// LogToEntity copies a log payload onto a request-log entry. An empty type is
75+
// treated as a status entry, matching entries written before the type field
76+
// existed. A nil metadata map becomes an empty map.
77+
func LogToEntity(m *Log) entity.RequestLog {
78+
meta := m.GetMetadata()
79+
if meta == nil {
80+
meta = make(map[string]string)
81+
}
82+
typ := entity.RequestLogType(m.GetType())
83+
if typ == "" {
84+
typ = entity.RequestLogTypeStatus
85+
}
86+
return entity.RequestLog{
87+
RequestID: m.GetRequestId(),
88+
Queue: m.GetQueue(),
89+
TimestampMs: m.GetTimestampMs(),
90+
Type: typ,
91+
Status: entity.RequestStatus(m.GetStatus()),
92+
Event: entity.RequestEvent(m.GetEvent()),
93+
RequestVersion: m.GetRequestVersion(),
94+
LastError: m.GetLastError(),
95+
Metadata: meta,
96+
}
97+
}
98+
99+
func landStrategyToProto(s mergestrategy.MergeStrategy) strategypb.Strategy {
100+
switch s {
101+
case mergestrategy.MergeStrategyRebase:
102+
return strategypb.Strategy_REBASE
103+
case mergestrategy.MergeStrategySquashRebase:
104+
return strategypb.Strategy_SQUASH_REBASE
105+
case mergestrategy.MergeStrategyMerge:
106+
return strategypb.Strategy_MERGE
107+
case mergestrategy.MergeStrategyPromote:
108+
return strategypb.Strategy_PROMOTE
109+
default:
110+
return strategypb.Strategy_DEFAULT
111+
}
112+
}
113+
114+
func landStrategyFromProto(s strategypb.Strategy) mergestrategy.MergeStrategy {
115+
switch s {
116+
case strategypb.Strategy_REBASE:
117+
return mergestrategy.MergeStrategyRebase
118+
case strategypb.Strategy_SQUASH_REBASE:
119+
return mergestrategy.MergeStrategySquashRebase
120+
case strategypb.Strategy_MERGE:
121+
return mergestrategy.MergeStrategyMerge
122+
case strategypb.Strategy_PROMOTE:
123+
return mergestrategy.MergeStrategyPromote
124+
default:
125+
return mergestrategy.MergeStrategyUnknown
126+
}
127+
}

0 commit comments

Comments
 (0)