Skip to content

Commit 562d335

Browse files
committed
refactor: separate land workflow from runway merge
## Summary ### Why? SubmitQueue owns the user-facing land workflow, while Runway owns the merge execution contract. Renaming Runway's established wire types and topics would blur that boundary and introduce an unnecessary contract migration. ### What? Keep SubmitQueue APIs, lifecycle states, controllers, internal topics, CLI, and documentation in land terminology. Preserve Runway's `MergeRequest`, `MergeStep`, `MergeResult`, merge controllers, extensions, configuration, and existing merge topic names, with explicit translation at the validation, land-dispatch, and result-signal boundaries. Restore the shared `mergestrategy` contract and preserve every strategy mapping, including `PROMOTE`. Tighten the dependent-batch E2E fixture so it deterministically exercises wake-up from a Runway merge result rather than complete-coverage bypass. ## Test Plan - ✅ `make proto` - ✅ `make mocks` - ✅ `make gazelle` - ✅ `make fmt` - ✅ `make tidy` - ✅ `make build` - ✅ `make test` - ✅ `make lint` - ✅ `make check-tidy` - ✅ `make check-gazelle` - ✅ Targeted Runway contract, controller, extension, DLQ, and SubmitQueue boundary tests - ✅ Runway E2E suite - ✅ SubmitQueue E2E suite
1 parent c9ee7a8 commit 562d335

126 files changed

Lines changed: 932 additions & 792 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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ submitqueue/ # repo root (Go module github.com/uber/submi
7171
└── doc/ # Documentation
7272
```
7373

74-
The `platform/` tree holds code reused across domains (infrastructure, shared entities, shared extension contracts). A multi-service **domain** (e.g. `submitqueue/`) keeps the same internal layout (`gateway/`, `orchestrator/`, `entity/`, `extension/`, `core/`); a domain's own `core/` (e.g. `submitqueue/core/`) holds infra shared only between that domain's services. A **single-service domain** collapses that split — the domain *is* the service, so its controllers live directly under the domain root (e.g. `runway/controller/`, `stovepipe/controller/`) with no `gateway/`/`orchestrator/` segment, and its wire contract is service-segment-free (`api/{domain}/`). `runway` is a consumer-only landing service with no gateway. `stovepipe` exposes ingestion RPC behavior and runs its own process, build, build-signal, record, hook, and DLQ queue stages.
74+
The `platform/` tree holds code reused across domains (infrastructure, shared entities, shared extension contracts). A multi-service **domain** (e.g. `submitqueue/`) keeps the same internal layout (`gateway/`, `orchestrator/`, `entity/`, `extension/`, `core/`); a domain's own `core/` (e.g. `submitqueue/core/`) holds infra shared only between that domain's services. A **single-service domain** collapses that split — the domain *is* the service, so its controllers live directly under the domain root (e.g. `runway/controller/`, `stovepipe/controller/`) with no `gateway/`/`orchestrator/` segment, and its wire contract is service-segment-free (`api/{domain}/`). `runway` is a consumer-only merge execution service with no gateway. `stovepipe` exposes ingestion RPC behavior and runs its own process, build, build-signal, record, hook, and DLQ queue stages.
7575

7676
The `api/` tree holds **published** wire contracts — those depended on from outside the owning domain. RPC contracts live at `api/{domain}/{service}/` (`proto/` for `.proto` sources, `protopb/` for committed generated Go); for a single-service domain the service segment is dropped, so the contract lives directly at `api/{domain}/` (e.g. `api/runway/{proto,protopb}/`). A service package may hold multiple `.proto` files, all generating into the same `protopb/`. External message-queue contracts live at `api/{domain}/messagequeue/` (see Message Queue Contracts below). Internal queue contracts do **not** go here — they live under `{domain}/core/messagequeue/`.
7777

@@ -113,7 +113,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
113113

114114
Controllers receive `consumer.Delivery` (a subset interface without Ack/Nack) to enforce separation of business logic from queue mechanics. `delivery.Hold(delayMs)` requests delayed redelivery without consuming retry budget; the controller must then return `nil`.
115115

116-
**Queue payloads: IDs within a boundary, full payloads across one.** When producer and consumer share a store (for example SubmitQueue's `build`→`buildsignal` flow), put only the entity **ID** on the queue and reload from storage (the store is the source of truth, messages stay small, redelivery is idempotent). Reloading from storage is what makes the publish ordering load-bearing — see "Persist before you publish" above. When a queue **crosses a service boundary** (for example SubmitQueue validation or merge handing work to Runway), publish the **full payload** the consumer needs, and have the **client own the correlation ID** so it can match the asynchronous result back to the work it is tracking. The queue's **owner defines the wire contract and topic keys** (in its own domain package); the other side imports them.
116+
**Queue payloads: IDs within a boundary, full payloads across one.** When producer and consumer share a store (same service — e.g. `build`→`buildsignal`, `validate`→`landconflict`), put only the entity **ID** on the queue and reload from storage (the store is the source of truth, messages stay small, redelivery is idempotent). Reloading from storage is what makes the publish ordering load-bearing — see "Persist before you publish" above. When a queue **crosses a service boundary** (the consumer cannot read the producer's store — e.g. orchestrator→runway), publish the **full payload** the consumer needs, and have the **client own the correlation ID** so it can match the async result back to the work it is tracking. The queue's **owner defines the wire contract and topic keys** (in its own domain package); the other side imports them.
117117

118118
### Entities
119119

@@ -198,7 +198,7 @@ To add a new `.proto` to a service, drop it in the service's `api/{domain}/{serv
198198
199199
New queue contracts are defined in **proto3** (`.proto` under `proto/`, generated Go in `protopb/` as the binding) and serialized as **protobuf JSON** (protojson) so the queue keeps storing self-describing JSON. Location follows audience: external/cross-domain contracts go under `api/{domain}/messagequeue/`; internal contracts (used only within the owning domain) go under `{domain}/core/messagequeue/`. Bazel `visibility` enforces the split — internal targets are domain-scoped, `api/` targets are public.
200200
201-
For proto-backed contracts, the message types are generated and the contract package adds generic `protojson` glue — `Marshal(m)` / `Unmarshal[T](b, m)` — owning the wire conventions: `UseProtoNames` (snake_case fields), UPPER_SNAKE enum values, int64-as-string, and 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/` and `stovepipe/core/messagequeue/` are current examples.
201+
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
203203
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.
204204
@@ -376,6 +376,6 @@ Errors are classified by origin (user vs infra) and retryability. The framework
376376
**Key rules:**
377377
1. **Non-retryable by default** — a plain `fmt.Errorf(...)` is non-retryable. Retryability is opted into explicitly, but that decision is almost always made by a classifier, not a controller (see rule 4).
378378
2. **Infra by default** — any error not wrapped with `NewUserError` is infra. There is no `NewInfraError`.
379-
3. **Extensions return plain errors** — extension interfaces (`MergeChecker`, `Storage`, `Publisher`) return standard `error` values with their own domain sentinels (e.g. `storage.ErrNotFound`). They do NOT classify errors as user or infra.
379+
3. **Extensions return plain errors** — extension interfaces (`LandChecker`, `Storage`, `Publisher`) return standard `error` values with their own domain sentinels (e.g. `storage.ErrNotFound`). They do NOT classify errors as user or infra.
380380
4. **Classifiers do the bulk of classification; controllers override only with knowledge a classifier lacks** — primary pipeline consumers compose per-backend classifiers into `errs.NewClassifierProcessor(...)`; the processor runs once per chain in the consumer and decides retryability from the raw error. So the common case is a controller returning the raw error (`fmt.Errorf("...: %w", err)`) and letting the classifier verdict stand. Reserve an explicit `errs.New*Error` wrap for the rare case where the controller knows something the classifier cannot infer from the error value alone (e.g. `storage.ErrNotFound` meaning "user asked for a missing resource" *in this call site*). Do **not** wrap a failure as retryable just because replaying it is convenient (e.g. a failed queue publish) — that turns permanent failures into infinite retries instead of dead-lettering. DLQ reconciliation consumers use `errs.AlwaysRetryableProcessor` instead. See [platform/errs/README.md](platform/errs/README.md).
381381
5. **Error chain works end-to-end** — extensions wrap custom errors, controllers wrap with `errs.New*Error`, and `errors.Is`/`errors.As` walks the full chain.

Makefile

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export REPO_ROOT := $(shell pwd)
4646
# path, so adding a provider is mostly adding a directory — see
4747
# service/submitqueue/demo/provider/README.md.
4848
#
49-
# fake a change is a URI; nothing merges anywhere. Needs nothing.
49+
# fake a change is a URI; nothing lands anywhere. Needs nothing.
5050
# git branches in a bare repository on disk; real fetch, cherry-pick, push.
5151
# github real pull requests. Needs a repository and GITHUB_TOKEN.
5252
PROVIDER ?= fake
@@ -60,7 +60,7 @@ PROVIDER_COMPOSE_FILE_git = service/submitqueue/docker-compose.git.yml
6060
PROVIDER_COMPOSE_FILE_github = service/submitqueue/docker-compose.provider.yml
6161
PROVIDER_COMPOSE_FILE = $(PROVIDER_COMPOSE_FILE_$(PROVIDER))
6262

63-
# Where PROVIDER=git keeps the bare repository it merges into. Outside the
63+
# Where PROVIDER=git keeps the bare repository it lands into. Outside the
6464
# repository, so a demo leaves nothing in a checkout, and bind-mounted rather
6565
# than kept in a volume so `git log` on the host can show what landed.
6666
#
@@ -266,7 +266,7 @@ deps: tidy-go ## Download and tidy Go dependencies
266266
e2e-git-test: ## Run the hermetic git E2E (real merger against a bare repo; no credentials)
267267
@echo "Running hermetic git end-to-end tests..."
268268
@$(BAZEL) test //test/e2e/submitqueue:go_default_test --test_output=errors \
269-
--test_filter='TestGitMergeE2E'
269+
--test_filter='TestGitLandE2E'
270270

271271
e2e-test: ## Run end-to-end tests (hermetic; Bazel builds all inputs; runs in parallel)
272272
@echo "Running end-to-end tests (parallel)..."
@@ -519,7 +519,7 @@ local-submitqueue-start: build-all-linux ## Start full stack (PROVIDER=fake|git|
519519
@echo ""
520520
@echo "Gateway gRPC port: $$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'unknown')"
521521
@if [ "$(PROVIDER)" = "git" ]; then \
522-
echo "Merge target: $(SQ_GIT_SANDBOX_DIR)/sandbox.git"; \
522+
echo "Land target: $(SQ_GIT_SANDBOX_DIR)/sandbox.git"; \
523523
fi
524524
@echo ""
525525
@echo "Generate traffic with:"
@@ -579,7 +579,7 @@ local-stovepipe-stop: ## Stop the Stovepipe service
579579

580580
mocks: ## Generate mock files using mockgen
581581
@echo "Generating mocks..."
582-
@$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/hook/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/mergechecker/... ./submitqueue/extension/conflict/... ./submitqueue/extension/speculation/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/core/requestlog/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/...
582+
@$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/hook/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/landchecker/... ./runway/extension/merger/... ./submitqueue/extension/conflict/... ./submitqueue/extension/speculation/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/core/requestlog/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/...
583583
@echo "Mocks generated successfully!"
584584

585585
proto: ## Generate protobuf files from .proto definitions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE)
66
[![Slack](https://img.shields.io/badge/Slack-join%20the%20community-4A154B?logo=slack&logoColor=white)](https://join.slack.com/t/submitqueue/shared_invite/zt-46gkqj682-7zcQphxm2pYqkjDo9lbmYA)
77

8-
SubmitQueue is a high-performance speculative merge queue that keeps your trunk consistently green at scale. Rather than validating changes one at a time, SubmitQueue speculatively rebases and validates multiple changes in parallel against predicted future states of HEAD. When validations pass, changes land automatically. When they fail, SubmitQueue isolates the offending change and retries the rest — all without human intervention.
8+
SubmitQueue is a high-performance speculative submission queue that keeps your trunk consistently green at scale. Rather than validating changes one at a time, SubmitQueue speculatively rebases and validates multiple changes in parallel against predicted future states of HEAD. When validations pass, changes land automatically. When they fail, SubmitQueue isolates the offending change and retries the rest — all without human intervention.
99

1010
Designed for large monorepos and fast-moving teams where concurrent changes can introduce subtle conflicts and destabilize builds.
1111

api/base/hook/protopb/hook.pb.go

Lines changed: 4 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/base/messagequeue/proto/messagequeue.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ option java_package = "com.uber.submitqueue.base.messagequeue";
2828
// in any domain — annotates itself with stable logical topic key(s), making the
2929
// key-to-payload binding part of the language-neutral proto contract rather than
3030
// out-of-band Go wiring. A single payload may list several keys (one shape can
31-
// serve a queue pair, e.g. a dry-run check and a committing merge). Domains
31+
// serve a queue pair, e.g. a dry-run check and a committing operation). Domains
3232
// import this rather than redefining their own.
3333
extend google.protobuf.MessageOptions {
3434
// topic_keys are the stable logical topic keys that carry this message — not

api/submitqueue/gateway/proto/gateway.proto

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ message PingResponse {
4646
string hostname = 4;
4747
}
4848

49-
// LandRequest defines a request to land (merge into target branch of the source control repository) a set of code changes.
49+
// LandRequest defines a request to land a set of code changes on the source control repository's target branch.
5050
//
5151
// SubmitQueue guarantees changes are landed in order with no other changes in between.
5252
// SubmitQueue does not guarantee each change is individually valid, but produces a validity marker on such changes.
@@ -261,7 +261,7 @@ service SubmitQueueGateway {
261261
// state transition is performed in the background by the orchestrator and may not have completed by the time the
262262
// caller receives a response.
263263
//
264-
// Cancellation is NOT GUARANTEED: a request that has already merged, or that races to completion before the cancel
264+
// Cancellation is NOT GUARANTEED: a request that has already landed, or that races to completion before the cancel
265265
// signal propagates through the pipeline, may still land (or end in an error). Callers must NOT assume that a
266266
// successful Cancel response means the request was cancelled — the actual terminal outcome (cancelled, landed, or
267267
// error) must be checked through the request-summary or request-history APIs.

api/submitqueue/gateway/protopb/gateway.pb.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/submitqueue/gateway/protopb/gateway_grpc.pb.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)