From cd130f18af5f017d29d2fb4566743ae09bb6fc5a Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Mon, 7 Sep 2026 22:19:57 -0700 Subject: [PATCH] docs: align SubmitQueue workflow terminology with land MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? The implementation now distinguishes SubmitQueue's land workflow from Runway's merge execution contract, but stale documentation and API comments would continue to blur that boundary for maintainers and users. ### What? Update guides, RFCs, package documentation, interface comments, protobuf comments, and generated protobuf documentation to use land terminology within SubmitQueue while retaining merge terminology for Runway and generic merge operations. ## Test Plan - ✅ `make proto` - ✅ `make gazelle` - ✅ `make lint` --- AGENTS.md | 8 +-- README.md | 2 +- api/base/hook/protopb/hook.pb.go | 9 ++- .../messagequeue/proto/messagequeue.proto | 2 +- api/submitqueue/gateway/proto/gateway.proto | 4 +- api/submitqueue/gateway/protopb/gateway.pb.go | 2 +- .../gateway/protopb/gateway_grpc.pb.go | 4 +- doc/howto/QUICKSTART.md | 4 +- doc/rfc/change-uri.md | 2 +- doc/rfc/consumer-gate.md | 2 +- doc/rfc/hook-framework.md | 12 ++-- doc/rfc/index.md | 6 +- doc/rfc/messagequeue-contract.md | 6 +- doc/rfc/sql-queue-rfc.md | 8 +-- doc/rfc/stovepipe/steps/build.md | 2 +- doc/rfc/stovepipe/steps/process.md | 2 +- doc/rfc/stovepipe/workflow.md | 4 +- doc/rfc/submitqueue/extension-contract.md | 21 +++--- doc/rfc/submitqueue/modular-queue-wiring.md | 6 +- .../speculation-generator-best-first.md | 2 +- doc/rfc/submitqueue/speculation.md | 34 +++++----- doc/rfc/submitqueue/workflow.md | 68 +++++++++---------- platform/errs/README.md | 2 +- platform/extension/messagequeue/README.md | 2 +- .../extension/messagequeue/mysql/README.md | 4 +- .../messagequeue/mysql/ctl/README.md | 40 +++++------ .../mysql/schema/queue_messages.sql | 2 +- platform/hook/README.md | 2 +- platform/hook/dlq.go | 2 +- platform/metrics/README.md | 2 +- platform/metrics/metrics.go | 2 +- service/README.md | 4 +- service/submitqueue/README.md | 2 +- stovepipe/README.md | 2 +- submitqueue/core/changeset/README.md | 2 +- submitqueue/core/changeset/changeset.go | 10 +-- submitqueue/entity/request.go | 6 +- submitqueue/entity/request_log.go | 6 +- submitqueue/entity/speculation.go | 2 +- .../speculation/allocator/sticky/sticky.go | 2 +- .../speculation/generator/bestfirst/README.md | 2 +- .../extension/speculation/scorer/scorer.go | 2 +- .../speculation/speculator/README.md | 2 +- .../speculation/speculator/speculator.go | 2 +- .../speculation/speculator/standard/README.md | 2 +- submitqueue/gateway/controller/cancel.go | 4 +- submitqueue/orchestrator/README.md | 10 +-- submitqueue/orchestrator/controller/README.md | 2 +- .../controller/conclude/conclude.go | 2 +- .../dependencyanalysis/dependencyanalysis.go | 4 +- .../orchestrator/controller/dlq/README.md | 2 +- .../orchestrator/controller/dlq/batch.go | 4 +- .../orchestrator/controller/dlq/dlq.go | 2 +- .../orchestrator/controller/dlq/speculate.go | 2 +- .../controller/speculate/check.go | 10 +-- .../controller/speculate/dispatch.go | 2 +- .../orchestrator/controller/speculate/doc.go | 14 ++-- .../orchestrator/controller/speculate/run.go | 6 +- .../controller/speculate/snapshot.go | 2 +- 59 files changed, 188 insertions(+), 192 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f95bf9bbc..ac047121c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ submitqueue/ # repo root (Go module github.com/uber/submi └── doc/ # Documentation ``` -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. +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. 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/`. @@ -113,7 +113,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er 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`. -**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. +**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. ### Entities @@ -198,7 +198,7 @@ To add a new `.proto` to a service, drop it in the service's `api/{domain}/{serv 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. -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. +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. 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. @@ -376,6 +376,6 @@ Errors are classified by origin (user vs infra) and retryability. The framework **Key rules:** 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). 2. **Infra by default** — any error not wrapped with `NewUserError` is infra. There is no `NewInfraError`. -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. +3. **Extensions return plain errors** — extension interfaces (`ChangeProvider`, `Storage`, `Publisher`) return standard `error` values with their own domain sentinels (e.g. `storage.ErrNotFound`). They do NOT classify errors as user or infra. 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). 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. diff --git a/README.md b/README.md index 6c7fb86a7..130407284 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) [![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) -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. +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. Designed for large monorepos and fast-moving teams where concurrent changes can introduce subtle conflicts and destabilize builds. diff --git a/api/base/hook/protopb/hook.pb.go b/api/base/hook/protopb/hook.pb.go index cc0046c49..d84f9b0fc 100644 --- a/api/base/hook/protopb/hook.pb.go +++ b/api/base/hook/protopb/hook.pb.go @@ -40,19 +40,18 @@ const ( // HookEvent is one fire-and-forget lifecycle event. Every domain publishes this // same shape to its own hook topic, so a sink that consumes several domains -// reads one schema rather than one per producer. See api/base/hook/README.md. +// reads one schema rather than one per producer. type HookEvent struct { state protoimpl.MessageState `protogen:"open.v1"` // id is the opaque identity of this occurrence, derived from the transition // it describes so that replaying the transition mints the same id. It is // the queue's dedupe key and a hook's idempotency key, and is never parsed. Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` - // source is the domain that produced the event: "submitqueue", - // "stovepipe", ... An open string rather than an enum so a new producer - // does not break existing consumers. + // source is the domain that produced the event. An open string rather than + // an enum so a new producer does not break existing consumers. Source string `protobuf:"bytes,2,opt,name=source,proto3" json:"source,omitempty"` // type is what happened, as one dotted open string: "request.landed", - // "batch.failed", ... It is the only dimension a consumer filters on, and + // "batch.failed", etc. It is the only dimension a consumer filters on, and // open for the same reason as source. Type string `protobuf:"bytes,3,opt,name=type,proto3" json:"type,omitempty"` // timestamp_ms is when the occurrence happened, in milliseconds since the diff --git a/api/base/messagequeue/proto/messagequeue.proto b/api/base/messagequeue/proto/messagequeue.proto index 3dbabb705..bd18807d1 100644 --- a/api/base/messagequeue/proto/messagequeue.proto +++ b/api/base/messagequeue/proto/messagequeue.proto @@ -28,7 +28,7 @@ option java_package = "com.uber.submitqueue.base.messagequeue"; // in any domain — annotates itself with stable logical topic key(s), making the // key-to-payload binding part of the language-neutral proto contract rather than // out-of-band Go wiring. A single payload may list several keys (one shape can -// serve a queue pair, e.g. a dry-run check and a committing merge). Domains +// serve a queue pair, e.g. a dry-run check and a committing operation). Domains // import this rather than redefining their own. extend google.protobuf.MessageOptions { // topic_keys are the stable logical topic keys that carry this message — not diff --git a/api/submitqueue/gateway/proto/gateway.proto b/api/submitqueue/gateway/proto/gateway.proto index 272c7d013..e9a8dec4a 100644 --- a/api/submitqueue/gateway/proto/gateway.proto +++ b/api/submitqueue/gateway/proto/gateway.proto @@ -46,7 +46,7 @@ message PingResponse { string hostname = 4; } -// LandRequest defines a request to land (merge into target branch of the source control repository) a set of code changes. +// LandRequest defines a request to land a set of code changes on the source control repository's target branch. // // SubmitQueue guarantees changes are landed in order with no other changes in between. // SubmitQueue does not guarantee each change is individually valid, but produces a validity marker on such changes. @@ -261,7 +261,7 @@ service SubmitQueueGateway { // state transition is performed in the background by the orchestrator and may not have completed by the time the // caller receives a response. // - // Cancellation is NOT GUARANTEED: a request that has already merged, or that races to completion before the cancel + // Cancellation is NOT GUARANTEED: a request that has already landed, or that races to completion before the cancel // signal propagates through the pipeline, may still land (or end in an error). Callers must NOT assume that a // successful Cancel response means the request was cancelled — the actual terminal outcome (cancelled, landed, or // error) must be checked through the request-summary or request-history APIs. diff --git a/api/submitqueue/gateway/protopb/gateway.pb.go b/api/submitqueue/gateway/protopb/gateway.pb.go index 301a53930..135455a89 100644 --- a/api/submitqueue/gateway/protopb/gateway.pb.go +++ b/api/submitqueue/gateway/protopb/gateway.pb.go @@ -157,7 +157,7 @@ func (x *PingResponse) GetHostname() string { return "" } -// LandRequest defines a request to land (merge into target branch of the source control repository) a set of code changes. +// LandRequest defines a request to land a set of code changes on the source control repository's target branch. // // SubmitQueue guarantees changes are landed in order with no other changes in between. // SubmitQueue does not guarantee each change is individually valid, but produces a validity marker on such changes. diff --git a/api/submitqueue/gateway/protopb/gateway_grpc.pb.go b/api/submitqueue/gateway/protopb/gateway_grpc.pb.go index ac9a4a2fa..79f70f8e2 100644 --- a/api/submitqueue/gateway/protopb/gateway_grpc.pb.go +++ b/api/submitqueue/gateway/protopb/gateway_grpc.pb.go @@ -62,7 +62,7 @@ type SubmitQueueGatewayClient interface { // state transition is performed in the background by the orchestrator and may not have completed by the time the // caller receives a response. // - // Cancellation is NOT GUARANTEED: a request that has already merged, or that races to completion before the cancel + // Cancellation is NOT GUARANTEED: a request that has already landed, or that races to completion before the cancel // signal propagates through the pipeline, may still land (or end in an error). Callers must NOT assume that a // successful Cancel response means the request was cancelled — the actual terminal outcome (cancelled, landed, or // error) must be checked through the request-summary or request-history APIs. @@ -185,7 +185,7 @@ type SubmitQueueGatewayServer interface { // state transition is performed in the background by the orchestrator and may not have completed by the time the // caller receives a response. // - // Cancellation is NOT GUARANTEED: a request that has already merged, or that races to completion before the cancel + // Cancellation is NOT GUARANTEED: a request that has already landed, or that races to completion before the cancel // signal propagates through the pipeline, may still land (or end in an error). Callers must NOT assume that a // successful Cancel response means the request was cancelled — the actual terminal outcome (cancelled, landed, or // error) must be checked through the request-summary or request-history APIs. diff --git a/doc/howto/QUICKSTART.md b/doc/howto/QUICKSTART.md index da49f4896..fbe66d203 100644 --- a/doc/howto/QUICKSTART.md +++ b/doc/howto/QUICKSTART.md @@ -200,7 +200,7 @@ Still no credential. `PROVIDER=git` provisions a bare repository at `/tmp/sq-san ✅ Stack is running against provider 'git'. Gateway gRPC port: 55295 -Merge target: /tmp/sq-sandbox/sandbox.git +Land target: /tmp/sq-sandbox/sandbox.git ``` Then the same command as before, unchanged: @@ -364,7 +364,7 @@ make land QUEUE=demo-queue \ URI='git://demo.example.com/demo/refs%2Fheads%2Fbad/2222222222222222222222222222222222222222?sq-fake=build-fail' ``` -That request walks the same path as far as `speculating`, records `building`, and then goes terminal at `error` instead of landing. Other tokens follow the same `sq-fake=` convention and are documented on the fake they drive — `provider-error` on the change provider, `unmergeable` and `mergecheck-error` on the merge checker, `trigger-error` and `build-error` on the build runner. +That request walks the same path as far as `speculating`, records `building`, and then goes terminal at `error` instead of landing. Other tokens follow the same `sq-fake=` convention and are documented on the fake they drive — `provider-error` on the change provider, `trigger-error` and `build-error` on the build runner, and `merge-conflict`, `merge-invalid`, and `merge-error` on Runway's merger. A hand-written URI like the one above belongs to the `fake` rung alone. On `git` it names a commit the merger cannot fetch, and on `github` the change provider tries to resolve it as a pull request — both fail, but for reasons that have nothing to do with the marker. diff --git a/doc/rfc/change-uri.md b/doc/rfc/change-uri.md index 05fd0c959..74cb97686 100644 --- a/doc/rfc/change-uri.md +++ b/doc/rfc/change-uri.md @@ -6,7 +6,7 @@ A change URI is the system-wide identity of a code change — a Pull Request, a Every change URI is an RFC 3986 URI of the form `scheme://{host[:port]}/{path}`, with a uniform division of labor: -- **scheme** — the provider *model*: how to parse the path and which extension family (change provider, merge checker, pusher) can act on it. One scheme per model — deployment flavors of the same model (github.com vs. GitHub Enterprise) do **not** get their own schemes, because the flavor is derivable from the host and two spellings for one instance would break identity. +- **scheme** — the provider *model*: how to parse the path and which change-provider implementation can resolve it. One scheme per model — deployment flavors of the same model (github.com vs. GitHub Enterprise) do **not** get their own schemes, because the flavor is derivable from the host and two spellings for one instance would break identity. - **authority** — the provider *instance*: the `host[:port]` the change lives on. Mandatory. - **path** — the change within that instance, pinned to an exact code state (head SHA or diff ID), so staleness is detectable by comparing the pin against the provider's current state. diff --git a/doc/rfc/consumer-gate.md b/doc/rfc/consumer-gate.md index 342aac93e..9b26ceb39 100644 --- a/doc/rfc/consumer-gate.md +++ b/doc/rfc/consumer-gate.md @@ -76,7 +76,7 @@ If gate state cannot be read (directory missing, I/O error), the check logs, inc The cancellation scenario, expressed as stop → observe → start: 1. The test closes the gate for `runway-mergeconflictcheck` (all partitions, or scoped to the test queue's partition key), before landing. -2. It lands a request. The orchestrator runs it to the merge-conflict-check hand-off; runway's subscriber delivers the check message, and the gate parks it. +2. It lands a request. The orchestrator runs it to the merge-conflict-check hand-off; Runway's subscriber delivers the check message, and the gate parks it. 3. The test awaits the parked record — proof the controller is stopped *and* holding exactly this message. Runway itself is still running; its RPC surface and merge controller are untouched. 4. While stopped, the test observes and acts: it cancels the request, awaits the terminal `cancelled` status through the existing event plane, and asserts no batch ever enrolled the request. 5. The test opens the gate. Within a re-check tick the postponed delivery redelivers, clears the open gate, and proceeds into the controller as a fresh attempt (postponing resets retry accounting); runway answers the now-stale check, and the test asserts the signal is dropped for the halted request. diff --git a/doc/rfc/hook-framework.md b/doc/rfc/hook-framework.md index bd035a8ca..e4a5ae601 100644 --- a/doc/rfc/hook-framework.md +++ b/doc/rfc/hook-framework.md @@ -4,9 +4,9 @@ Fire-and-forget side effects for pipeline lifecycle events: one shared event con ## Problem -The pipelines emit lifecycle transitions — a request lands or fails, a batch merges, a build finishes — but nothing can react outside pipeline state: no warehouse export, no PR comments or closes on merge events, no notifications or audit trails. The log topic is not this seam: SubmitQueue request statuses only, consumed solely to build gateway read models. +The pipelines emit lifecycle transitions — a request lands or fails, a batch lands, a build finishes — but nothing can react outside pipeline state: no warehouse export, no PR comments or closes on land events, no notifications or audit trails. The log topic is not this seam: SubmitQueue request statuses only, consumed solely to build gateway read models. -Two requirements: side effects must never stall or fail the pipeline, and "fire and forget" must not mean lossy — a merge-failure comment that silently never posts is a support ticket. +Two requirements: side effects must never stall or fail the pipeline, and "fire and forget" must not mean lossy — a land-failure comment that silently never posts is a support ticket. ## Proposal @@ -54,7 +54,7 @@ Delivery promise: - `api/base/hook/`: no owning domain, so the message-queue location rule extends — platform-owned contracts live under `api/base/`. - Envelope = only fields every consumer keys on uniformly; subject, queue, and error are occurrence facts → payload. `source`/`type` are strings, not enums, for additive evolution. -- Payload (`Struct`): shaped per type, add-only, documented by its domain; must carry the subject's id and transient facts (merge step outcomes, build failure detail) — the event is their only durable record. Never entity snapshots; hooks resolve entities from stores. +- Payload (`Struct`): shaped per type, add-only, documented by its domain; must carry the subject's id and transient facts (land step outcomes, build failure detail) — the event is their only durable record. Never entity snapshots; hooks resolve entities from stores. ### Hooks and dispatch @@ -93,7 +93,7 @@ message HookEvent { } ``` -A failed batch, carrying merge-result facts persisted nowhere else (protojson: int64 as string, empty fields omitted): +A failed batch, carrying land-result facts persisted nowhere else (protojson: int64 as string, empty fields omitted): ```json { @@ -105,7 +105,7 @@ A failed batch, carrying merge-result facts persisted nowhere else (protojson: i "payload": { "batch_id": "batch-778", "queue": "go-monorepo", - "error": "merge conflict", + "error": "land conflict", "failed_step": "sq-12346", "conflict_paths": ["foo/bar.go"] } @@ -116,7 +116,7 @@ A failed batch, carrying merge-result facts persisted nowhere else (protojson: i - **A contract per domain.** N schemas, N hook shapes, N warehouse tables; one envelope absorbs differences additively. - **Inline hook calls.** Couples pipeline latency to integrations; a crash between write and call silently drops the notification. -- **A second consumer group on the log topic.** Request statuses only; no path to batch, build, merge, or other domains. +- **A second consumer group on the log topic.** Request statuses only; no path to batch, build, land, or other domains. - **Enums for source/type.** protojson rejects unknown enum values; every addition would break consumers. - **Subject, queue, or error on the envelope.** Occurrence facts; they live in the payload. No major event platform carries a top-level error. - **Entity snapshots as payload.** Stale on redelivery; competes with the store; drags domain schemas into the shared contract. diff --git a/doc/rfc/index.md b/doc/rfc/index.md index d3319e661..2e8319e25 100644 --- a/doc/rfc/index.md +++ b/doc/rfc/index.md @@ -13,7 +13,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting ## SubmitQueue -- [Orchestrator Workflow](submitqueue/workflow.md) - Queue-driven controller pipeline from gateway entry through batching, scoring, build, merge, and conclude +- [Orchestrator Workflow](submitqueue/workflow.md) - Queue-driven controller pipeline from gateway entry through batching, scoring, build, land, and conclude - [Gateway History APIs](submitqueue/history-api.md) - Request lifecycle history exposed through separate request ID and change ID endpoints - [Build Runner](submitqueue/build-runner.md) - Vendor-agnostic BuildRunner interface, provider-neutral BuildStatus lifecycle, and how the orchestrator wires it into the build stage - [Extension Contract](submitqueue/extension-contract.md) - When extensions take orchestrator identity (request/batch) and resolve granular content themselves vs. take controller-resolved data; revises the BuildRunner base/head contract @@ -24,7 +24,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting ## Stovepipe -- [Stovepipe Workflow](stovepipe/workflow.md) - Post-merge validation pipeline overview: ingest, process, build, record greenness, analyze projects, notify downstream +- [Stovepipe Workflow](stovepipe/workflow.md) - Post-land validation pipeline overview: ingest, process, build, record greenness, analyze projects, notify downstream - [Process stage](stovepipe/steps/process.md) - Build-strategy decision, per-queue concurrency gate, backlog coalescing, entity model, platform prerequisites - [Build stage](stovepipe/steps/build.md) - Trigger-only stage and Stovepipe's URI-based BuildRunner contract - [Buildsignal stage](stovepipe/steps/buildsignal.md) - Build polling, terminal status persistence, and the handoff to record @@ -34,4 +34,4 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting ## Runway -- [Runway Workflow](runway/workflow.md) - Landing service: merge-conflict checking and merging on behalf of SubmitQueue +- [Runway Workflow](runway/workflow.md) - Merge service: merge-conflict checking and merging on behalf of SubmitQueue diff --git a/doc/rfc/messagequeue-contract.md b/doc/rfc/messagequeue-contract.md index c52201e53..7c0383650 100644 --- a/doc/rfc/messagequeue-contract.md +++ b/doc/rfc/messagequeue-contract.md @@ -57,7 +57,7 @@ So the option earns its place as the single, language-neutral source of truth fo ### Go binding: the generated `protopb` -The generated message types in `protopb` are the Go binding, sitting beside `proto/` exactly as for the RPC contracts. The contract package adds only thin helpers — `protojson` (de)serialization and the `topic_keys` reflection lookup. Shared field types (`change.Change`, `mergestrategy.MergeStrategy`) are themselves shared protos under `api/base/{change,mergestrategy}/proto`, imported by every contract that needs them. +The generated message types in `protopb` are the Go binding, sitting beside `proto/` exactly as for the RPC contracts. The contract package adds only thin helpers — `protojson` (de)serialization and the `topic_keys` reflection lookup. Shared field types (`change.Change`, `mergestrategy.Strategy`) are themselves shared protos under `api/base/{change,mergestrategy}/proto`, imported by every contract that needs them. ## Example @@ -82,9 +82,9 @@ message ExampleRequest { message ExampleResult { // One shape, two queues: the same result is published under the check-result - // key for a dry run and the merge-result key for a committing run. + // key for a dry run and the land-result key for a committing run. option (uber.base.messagequeue.topic_keys) = "example-check-result"; - option (uber.base.messagequeue.topic_keys) = "example-merge-result"; + option (uber.base.messagequeue.topic_keys) = "example-land-result"; string id = 1; // Echoes the request's correlation id. bool success = 2; diff --git a/doc/rfc/sql-queue-rfc.md b/doc/rfc/sql-queue-rfc.md index 34bd5c840..2c743e649 100644 --- a/doc/rfc/sql-queue-rfc.md +++ b/doc/rfc/sql-queue-rfc.md @@ -18,7 +18,7 @@ MySQL-based distributed message queue with immutable message log, per-consumer-g ### Motivation SubmitQueue needs a reliable message queue for coordinating asynchronous workflows: -- **Orchestrator** publishes merge jobs and speculative build requests to workers +- **Orchestrator** publishes land jobs and speculative build requests to workers - **Workers** need distributed coordination without duplicate processing - **Crash recovery** must preserve exactly where processing stopped @@ -223,7 +223,7 @@ See `platform/extension/messagequeue/mysql/schema/queue_subscriber_heartbeats.sq ### Dead Letter Queue -DLQ messages are stored in the same `queue_messages` table under a different topic name (original topic + DLQ suffix, e.g., `merge_queue_dlq`). This allows DLQ messages to be consumed using the normal subscriber with the DLQ topic name. DLQ-specific fields (`failed_at`, `failure_count`, `last_error`, `original_topic`) are populated when a message is moved to DLQ; they are zero/empty for normal messages. +DLQ messages are stored in the same `queue_messages` table under a different topic name (original topic + DLQ suffix, e.g., `land_queue_dlq`). This allows DLQ messages to be consumed using the normal subscriber with the DLQ topic name. DLQ-specific fields (`failed_at`, `failure_count`, `last_error`, `original_topic`) are populated when a message is moved to DLQ; they are zero/empty for normal messages. ## Message Flow @@ -387,7 +387,7 @@ For our use case, we need ordering per repository. With Watermill: With our custom implementation: - Single `queue_messages` table for all topics and partitions -- Rows like `('merge_events', 'repo-123', offset, ...)` provide ordering within partition +- Rows like `('land_events', 'repo-123', offset, ...)` provide ordering within partition - No schema migrations for new repos or topics - Ordering guaranteed within `(topic, partition_key)` @@ -453,7 +453,7 @@ The current design separates the immutable message log from per-consumer-group d **At-Least-Once vs Exactly-Once** - Simpler, better performance - Applications must handle duplicates -- Mitigation: Idempotency keys (e.g., merge request ID) +- Mitigation: Idempotency keys (e.g., land request ID) ## Appendix diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 0501c9900..f630870b2 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -121,7 +121,7 @@ Both domains have a `build` controller that triggers via a build-runner extensio ### SubmitQueue -SubmitQueue validates **stacks of changes** before merging. Its `build` controller loads `base []entity.Batch` (ordered dependency batches) and `head entity.Batch` and triggers: +SubmitQueue validates **stacks of changes** before landing. Its `build` controller loads `base []entity.Batch` (ordered dependency batches) and `head entity.Batch` and triggers: ```go buildID, err := buildRunner.Trigger(ctx, base, head, metadata) diff --git a/doc/rfc/stovepipe/steps/process.md b/doc/rfc/stovepipe/steps/process.md index e541139d6..8f6962733 100644 --- a/doc/rfc/stovepipe/steps/process.md +++ b/doc/rfc/stovepipe/steps/process.md @@ -73,7 +73,7 @@ A slot is held from admit until the build goes terminal (`process → build → ## Raising `max_concurrent` (speculative validation) -Setting `max_concurrent = N > 1` overlaps validations to start work sooner, and it is **safe** — because Stovepipe validates **already-landed, linear trunk heads**, not pre-merge candidates. Successive commits (`G0 → A → B → C …`) each contain everything below them, so validating `G0..B` already tests A+B together. (A pre-merge queue serializes to catch "two changes green alone, broken combined"; that risk isn't present here.) A green result for head `H` on baseline `B` is an immutable property of `H`, true no matter where last-green moves afterward. +Setting `max_concurrent = N > 1` overlaps validations to start work sooner, and it is **safe** — because Stovepipe validates **already-landed, linear trunk heads**, not pre-land candidates. Successive commits (`G0 → A → B → C …`) each contain everything below them, so validating `G0..B` already tests A+B together. (A pre-land queue serializes to catch "two changes green alone, broken combined"; that risk isn't present here.) A green result for head `H` on baseline `B` is an immutable property of `H`, true no matter where last-green moves afterward. The scheme: each admit pins its baseline to last-green *at admit time*; a green head is adopted even if last-green has since advanced. A late green result is either the newest (adopt) or already behind the pointer (**moot** — dropped, never a regression). diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index bad91d8d8..00da86a97 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -1,6 +1,6 @@ # Stovepipe Workflow -Stovepipe answers one question for the rest of the company: **at which commit is this thing green?** It continuously polls a repository branch for its latest commit, validates that commit, works out which projects (if any) are broken at it, records the result, and notifies downstream systems so they can gate deployments on a known-good commit. It is a post-merge service: code lands first, Stovepipe finds out whether it was good. +Stovepipe answers one question for the rest of the company: **at which commit is this thing green?** It continuously polls a repository branch for its latest commit, validates that commit, works out which projects (if any) are broken at it, records the result, and notifies downstream systems so they can gate deployments on a known-good commit. It is a post-land service: code lands first, Stovepipe finds out whether it was good. The pipeline is a queue-driven chain of small, single-purpose controllers, in the same style as SubmitQueue (SQ). Each controller consumes one topic, advances one entity, and publishes to the next topic. Most hops carry only an **ID** and the controller reloads the entity from storage; the entry hop carries the caller's input because there is no row to load yet. The high-level shape is: @@ -23,7 +23,7 @@ Everything Stovepipe records greenness *about* is a URI: a specific commit on a ### Queue — the unit of identity for "what we validate" -Stovepipe reuses SQ's **Queue** concept for the same two reasons SQ does — to **namespace the generated IDs** and to give callers a **stable handle for the repo+ref being validated** — plus a third that is specific to a post-merge validator: a Queue **owns the last-known-good URI** and the greenness history for its branch. +Stovepipe reuses SQ's **Queue** concept for the same two reasons SQ does — to **namespace the generated IDs** and to give callers a **stable handle for the repo+ref being validated** — plus a third that is specific to a post-land validator: a Queue **owns the last-known-good URI** and the greenness history for its branch. A Queue is named by a **stable logical string** (e.g. `monorepo/main`), and that name is what the ingest API takes — *not* a raw URI. SourceControl/config resolves the Queue name to a concrete VCS URI base. This keeps callers (and the external poller) free of VCS detail: they say "the `monorepo/main` Queue has moved", and Stovepipe resolves what that means. diff --git a/doc/rfc/submitqueue/extension-contract.md b/doc/rfc/submitqueue/extension-contract.md index 80c241ff2..d45498a74 100644 --- a/doc/rfc/submitqueue/extension-contract.md +++ b/doc/rfc/submitqueue/extension-contract.md @@ -4,7 +4,7 @@ Design notes for what SubmitQueue's pluggable extensions accept: orchestrator ** ## Problem -Extension input granularity is inconsistent across the pipeline stages (see [workflow.md](workflow.md)). `conflict.Analyzer` takes identity (`entity.Batch`); `scorer`, `changeprovider`, `buildrunner`, `pusher` take controller-resolved `entity.Change`. The split caps what an extension can do: +Extension input granularity is inconsistent across the pipeline stages (see [workflow.md](workflow.md)). `conflict.Analyzer` takes identity (`entity.Batch`); `scorer`, `changeprovider`, and `buildrunner` take controller-resolved `entity.Change`. The split caps what an extension can do: - `ConflictType` already names `target_overlap`, but a real target-overlap analyzer **cannot be written** — the dependency-analysis stage hands it identity-level batches (no changed targets) and the contract has nowhere to put them. - `scorer` gets a URIs-only `Change`, so a heuristic scorer **cannot see** lines-changed / file-count. @@ -15,7 +15,7 @@ Both unblock with the shape `conflict` already uses: accept identity, resolve in - **Decision/action extensions** take orchestrator identity at their stage granularity and resolve granular content through narrowly-injected dependencies. Request stage → `entity.Request`; batch stage → `entity.Batch` / `[]entity.Batch`. Both are thin reference entities (a `Request` carries URIs, not diffs; a `Batch` carries IDs, not changes). - **Resolution targets** — `storage`, `changestore`, `queueconfig` — stay key/value-shaped. They are what the others resolve *through* (see [storage/README.md](../../../submitqueue/extension/storage/README.md) and [AGENTS.md](../../../AGENTS.md)). Refinement: the storage *aggregate* has since gained the same per-queue factory resolution every other seam has — the stores it hands back remain strictly key/value, bound to their queue, while the cross-queue read-model stores stay individually-injected singletons. -- **Output mirrors the input unit.** Each output element self-identifies with the input it corresponds to — `changeprovider`'s `ChangeInfo` carries its `URI`, `conflict`'s `Conflict` carries its `BatchID` — so a flat list suffices and the caller correlates results back to inputs without re-deriving boundaries. A *wrapper* entity (`entity.BatchChanges`) is introduced only to aggregate *up* to a coarser unit than the elements — the scorer needs batch-wide line/file totals, so the rollup earns its keep; no `RequestChanges` exists because nothing needs request-wide rollups. And when the input is a *collection* of independently-actioned units, the output groups by them: `pusher`, fed `[]entity.Batch`, returns outcomes grouped per batch, the same way `conflict` already tags each `Conflict` with its in-flight `BatchID`. +- **Output mirrors the input unit.** Each output element self-identifies with the input it corresponds to — `changeprovider`'s `ChangeInfo` carries its `URI`, `conflict`'s `Conflict` carries its `BatchID` — so a flat list suffices and the caller correlates results back to inputs without re-deriving boundaries. A *wrapper* entity (`entity.BatchChanges`) is introduced only to aggregate *up* to a coarser unit than the elements — the scorer needs batch-wide line/file totals, so the rollup earns its keep; no `RequestChanges` exists because nothing needs request-wide rollups. ### What each stage resolves today @@ -24,10 +24,9 @@ Both unblock with the shape `conflict` already uses: accept identity, resolve in | `validate` | `entity.Request` | nothing — `request.Change` is already in hand (the change-store reads here serve duplicate detection) | `request.Change` → `changeprovider` | | `dependency` | `entity.Batch` + active `[]entity.Batch` | **nothing** — the batch it analyzes is already persisted, with `Contains` set to `[requestID]` | `entity.Batch`, `[]entity.Batch` → `conflict` | | `score` | `entity.Batch`, then each `entity.Request` | batch → requests | `request.Change` per request, then multiplies the scores → `scorer` | -| `build` | `entity.Batch`, then `collectChanges` | batch → requests → changes, **flattening batch boundaries** | base `[]Change`, head `[]Change` → `buildrunner` | -| `merge` | `entity.Batch`, then `collectChanges` | batch → requests → changes | `[]Change` → `pusher` | +| `build` | head `entity.Batch` + path base `[]entity.Batch` | **nothing** — the build runner resolves each batch through its injected `changeset.Resolver` | base `[]entity.Batch`, head `entity.Batch` → `buildrunner` | -Two facts this grounds: `conflict` already resolves nothing (the baseline), and the batch→changes walk is **already duplicated** in `build`/`merge` `collectChanges` — the shared resolver below only consolidates it. +This grounds `conflict` as the baseline: it already resolves nothing because the controller passes the identity it needs. ## Verdict @@ -37,22 +36,20 @@ Two facts this grounds: `conflict` already resolves nothing (the baseline), and | `scorer.Scorer` | score | flat `Change`, per request | `entity.Batch` — resolve + reduce internally | one batch score (`float64`) — unchanged | request store + change provider | | `changeprovider.ChangeProvider` | validate | `Change` | `entity.Request` | per-URI change info (`[]ChangeInfo`, `URI`-tagged) — unchanged | none — it *is* the resolver | | `buildrunner.BuildRunner` | build | base/head `[]Change` | base `[]entity.Batch` + head `entity.Batch` | build id, then status/cancel (`BuildID`, `BuildStatus`) — unchanged | request store + change provider | -| `pusher.Pusher` *(removed)* | merge | — | **moved out-of-process to runway** (`merge` / `merge-signal`); see the note below the table | — | — | | `storage`, `changestore`, `queueconfig` | — | keys + entities | unchanged — resolution targets | entities | — | -**Outputs are unchanged.** This RFC moves the *input* toward identity; the four live return contracts — conflicts, score, change info, build id/status — are exactly what they are today. (The `pusher` row is not an in-process extension: merge runs out-of-process in runway, so its output is not part of this catalog — see the note below.) No other output shape changes. +**Outputs are unchanged.** This RFC moves the *input* toward identity; the four live return contracts — conflicts, score, change info, build id/status — are exactly what they are today. No output shape changes. -The validate-time mergeability **check** and the **merge** itself both run **asynchronously and out-of-process** in runway rather than as in-process extensions, over the one shared `MergeRequest`/`MergeResult` contract — a check is a dry run of a merge. `validate` hands off directly to runway (→ `merge-conflict-check`, result back via `mergeconflictsignal`); `merge` hands the batch to runway (→ `merge`, result back via `mergesignal`) rather than calling an in-process `pusher`. See [workflow.md](workflow.md). The in-process `mergechecker` and `pusher` packages are unused on the pipeline path. +The validate-time landability **check** and the **land** itself both run **asynchronously and out-of-process** in Runway rather than as in-process extensions. SubmitQueue adapts its land request to Runway's shared `MergeRequest`/`MergeResult` contract, where a conflict check is a dry run of a merge. `validate` hands off directly to Runway (→ `merge-conflict-check`, result back via `landconflictsignal`); `land` hands the batch to Runway (→ `runway-merge`, result back via `landsignal`). See [workflow.md](workflow.md). SubmitQueue retains no parallel in-process checking or pushing contract. Non-obvious points: - **scorer** — owning the batch moves batch-level reduction (today the controller's multiplicative product) into the scorer, where the `composite` reduce step already lives. -- **buildrunner** — this **revises** [build-runner.md](build-runner.md), which deliberately kept batches out of the boundary. The base/head split survives, expressed as batches; the provider still operates on changes (the shared resolver produces them inside the extension). Cost: a `buildrunner` / `pusher` implementation now depends on a request store + change provider. -- **pusher** — a *list* of batches (not one) designs for a merge-train: land several ready batches, or a batch with not-yet-landed deps, in one atomic push. Today merge pushes a single batch because deps are already on trunk. Since the input is now a list, the output groups outcomes per batch (`BatchID`-tagged, with per-change commit detail kept underneath) instead of one flat per-change list — the only output shape this RFC changes. Push atomicity is unchanged (all-or-nothing across the whole call), so a per-batch *status* is intentionally omitted: a partial-landing train would be a separate, larger change to the atomicity contract. +- **buildrunner** — this **revises** [build-runner.md](build-runner.md), which deliberately kept batches out of the boundary. The base/head split survives, expressed as batches; the provider still operates on changes (the shared resolver produces them inside the extension). Cost: a `buildrunner` implementation now depends on a request store + change provider. ## Mechanism -Dependencies are injected per-extension at the existing `Factory.For` (wiring: `service/submitqueue/orchestrator/server/main.go`) — only the handles a contract justifies, never the whole storage aggregator. The repeated batch→changes walk becomes one shared resolver (today's duplicated `collectChanges`, consolidated, and preserving the batch boundaries build's copy flattens). Controllers shrink to passing the identity entity they already load. +Dependencies are injected per-extension at the existing `Factory.For` (wiring: `service/submitqueue/orchestrator/server/main.go`) — only the handles a contract justifies, never the whole storage aggregator. Batch→changes resolution is centralized in `changeset.Resolver`, while `buildrunner.ResolveBatches` shares the ordered flattening needed by build-runner backends. Controllers pass the identity entities they already load. `entity.BatchChanges` is kept, not removed — it becomes the shared resolver's *detailed output* (URIs + provider details for a batch, what the scorer consumes) rather than a value the score controller assembles and passes in. Its line/file helpers move with it; only its producer changes. @@ -60,5 +57,5 @@ Dependencies are injected per-extension at the existing `Factory.For` (wiring: ` - **Status quo (controller resolves).** Keeps extensions pure and trivially testable, but thickens controllers and caps every extension at what the controller chose to pre-compute — the two blocked features are that ceiling. - **Literal string IDs.** An extra read per call when the controller already holds the entity; pass thin reference entities instead. -- **Per-implementation batch→changes resolution.** How the `build`/`merge` duplication arose; one shared resolver instead. +- **Per-implementation batch→changes traversal.** Duplicates storage and ordering rules across backends; use the shared resolver and build-runner helper instead. - *Acknowledged:* decision extensions gain dependencies and are no longer pure functions — mitigated by their existing mock packages and `Factory` injection. diff --git a/doc/rfc/submitqueue/modular-queue-wiring.md b/doc/rfc/submitqueue/modular-queue-wiring.md index 1acdddb89..a3e716e10 100644 --- a/doc/rfc/submitqueue/modular-queue-wiring.md +++ b/doc/rfc/submitqueue/modular-queue-wiring.md @@ -281,12 +281,12 @@ row appears on topic "start", partition key "monorepo/exp" gateway orchestrator stovepipe runway ──────────────────────────────────────────────────────────────────────────────────────────── Deps seams counter · storage · changeprovider · storage · counter · storage · - queueconfig.Store · buildrunner · scorer sourcecontrol. merger Factory + queueconfig.Store · buildrunner · scorer sourcecontrol. lander Factory requestlog store analyzer · validator Factory · (+7 speculation) queueconfig.Store - Stages log start · cancel · process mergeconflictcheck · - (rows) validate · batch · merge + Stages log start · cancel · process landconflictcheck · + (rows) validate · batch · land … (+ DLQ column) Controllers Gateway Orchestrator Stovepipe Runway diff --git a/doc/rfc/submitqueue/speculation-generator-best-first.md b/doc/rfc/submitqueue/speculation-generator-best-first.md index 458e5e007..5558109fc 100644 --- a/doc/rfc/submitqueue/speculation-generator-best-first.md +++ b/doc/rfc/submitqueue/speculation-generator-best-first.md @@ -447,7 +447,7 @@ The ordering stays the same. `CandidatePath.RankingScore` contains this logarith - `Succeeded` fixes an assumption to succeeds. - `Failed` or `Cancelled` fixes an assumption to fails. - `Cancelling` remains undecided because cancellation may lose a race with completion. -- `Merging` also remains undecided, because a merge can fail. It is tempting to treat it as committed to landing and skip the scorer call, but that puts a state-specific policy inside the search: whether a path betting against a merging batch is worth funding is a question of price, and price belongs to the scorer. The allocator draws the same line — "no batch state enters this decision" — and the generator holds it too. Nothing is lost by staying open: a single passed path still waits for the merge result, while passed paths covering every outcome let the controller bypass the dependency (see [speculation.md](speculation.md)). Funding the unlikely side spends budget, which is the allocator's to ration. +- `Landing` also remains undecided, because a land can fail. It is tempting to treat it as committed to landing and skip the scorer call, but that puts a state-specific policy inside the search: whether a path betting against a landing batch is worth funding is a question of price, and price belongs to the scorer. The allocator draws the same line — "no batch state enters this decision" — and the generator holds it too. Nothing is lost by staying open: a single passed path still waits for the land result, while passed paths covering every outcome let the controller bypass the dependency (see [speculation.md](speculation.md)). Funding the unlikely side spends budget, which is the allocator's to ration. - A fixed assumption stays in the returned path but contributes probability 1 and has no flip. - A shared dependency is scored once per run. diff --git a/doc/rfc/submitqueue/speculation.md b/doc/rfc/submitqueue/speculation.md index 1887cff7e..526bfa9c3 100644 --- a/doc/rfc/submitqueue/speculation.md +++ b/doc/rfc/submitqueue/speculation.md @@ -1,20 +1,20 @@ # Speculation -A merge queue that verifies one change at a time is limited by its slowest build. Speculation removes that limit: it builds a batch early, against an assumption about how the conflicting batches ahead of it will resolve, so a valid build is usually ready by the time they do. +A land queue that verifies one change at a time is limited by its slowest build. Speculation removes that limit: it builds a batch early, against an assumption about how the conflicting batches ahead of it will resolve, so a valid build is usually ready by the time they do. -Work enters SubmitQueue as **batches** — changes verified and merged together. Two batches **conflict** when they touch the same code, which makes the earlier one a **dependency** of the later. A **path** is one set of assumptions about how a batch's dependencies resolve, and the batch it builds is the path's **head**. +Work enters SubmitQueue as **batches** — changes verified and landed together. Two batches **conflict** when they touch the same code, which makes the earlier one a **dependency** of the later. A **path** is one set of assumptions about how a batch's dependencies resolve, and the batch it builds is the path's **head**. -On every queue update the **speculate controller** reruns from scratch: it reads the current state, applies the incoming signals, asks a pluggable **Speculator** which paths are worth building within the CI budget, and persists only those. Everything else is recomputed next time, never stored. A batch normally merges after its dependencies resolve and a matching build has passed; complete passed coverage of every unresolved outcome lets it bypass those dependencies. +On every queue update the **speculate controller** reruns from scratch: it reads the current state, applies the incoming signals, asks a pluggable **Speculator** which paths are worth building within the CI budget, and persists only those. Everything else is recomputed next time, never stored. A batch normally lands after its dependencies resolve and a matching build has passed; complete passed coverage of every unresolved outcome lets it bypass those dependencies. ## The speculation run -The speculate controller runs whenever the queue changes — after a new batch, a completed build, a merge result, or a cancel. Each publishes a **dirty signal** carrying the changed batch ID, partitioned by the queue so a queue's runs happen one at a time. The dirty signal is an internal queue contract — payload in `submitqueue/core/messagequeue`, topic key in `submitqueue/core/topickey`. +The speculate controller runs whenever the queue changes — after a new batch, a completed build, a land result, or a cancel. Each publishes a **dirty signal** carrying the changed batch ID, partitioned by the queue so a queue's runs happen one at a time. The dirty signal is an internal queue contract — payload in `submitqueue/core/messagequeue`, topic key in `submitqueue/core/topickey`. ``` dirty(queue) — "trigger a run" — published after: - a new batch - a build completes (success/failure/cancellation) - - a merge result arrives (success/failure) + - a land result arrives (success/failure) - a cancel │ │ carries the changed batch ID, partitioned by queue, so a @@ -31,15 +31,15 @@ The speculate controller runs whenever the queue changes — after a new batch, │ → paths to build, paths to preempt │ 4 check validate that output: drop actions it shouldn't propose │ (non-Speculating head, refuted, incoherent, terminal path) - │ 5 write record each head's decisions; send build / cancel / merge messages + │ 5 write record each head's decisions; send build / cancel / land messages │ (a head whose write loses is re-planned on the next run) │ ├─▶ build / cancel (path ID, attempt) → build (orchestrator/controller/build): │ reserve → BuildRunner.Trigger(base, head) → record build ID, mark path building │ CI runs → buildsignal marks path passed/failed/cancelled ─▶ dirty(queue) │ - └─▶ merge (batch) → Runway performs the merge - → mergesignal marks the batch succeeded/failed ─▶ dirty(queue) + └─▶ land (batch) → Runway performs the land + → landsignal marks the batch succeeded/failed ─▶ dirty(queue) ``` ### State reconciliation @@ -54,8 +54,8 @@ Every write is a compare-and-swap: a writer that loses re-reads on a later run. Verdicts are controller-owned facts: the Speculator can neither compute nor veto them. -- **Merge.** Each path carries an assumption about every dependency — *succeeds* (built on top of) or *fails* (built without). Normally, once a path's build has passed and every dependency has finished the way the path assumed — one assumed *succeeds* has merged, one assumed *fails* has failed or been cancelled — the speculate controller moves the head to Merging and hands it to Runway. A dependency that is merely *merging* has not finished, because a merge can fail, so a single matching path still waits for the answer. Complete passed coverage is the exception described in Bypass large diff: it lets a head merge before those answers arrive. If the hand-off is lost, the next run re-sends it. The same run sets the head's remaining in-flight paths *cancelling*: once the head can merge they cannot help, and they hold CI slots until they stop. The mergesignal controller records Runway's terminal result: success marks the head Succeeded, while failure marks it Failed. The result publishes a single dirty signal — no per-dependent fan-out — and the next run refutes paths whose assumption disagrees with the result: *fails* assumptions after success, *succeeds* assumptions after failure. The hand-off is idempotent, so Runway reports success without another merge when the change is already present. A chain ordinarily merges one at a time, but a fully covered head can bypass its unsettled predecessors. -- **Failure (no viable path).** A batch fails when every possible future has a failed build — no path can pass, so it can never merge. +- **Land.** Each path carries an assumption about every dependency — *succeeds* (built on top of) or *fails* (built without). Normally, once a path's build has passed and every dependency has finished the way the path assumed — one assumed *succeeds* has landed, one assumed *fails* has failed or been cancelled — the speculate controller moves the head to Landing and hands it to Runway. A dependency that is merely *landing* has not finished, because a land can fail, so a single matching path still waits for the answer. Complete passed coverage is the exception described in Bypass large diff: it lets a head land before those answers arrive. If the hand-off is lost, the next run re-sends it. The same run sets the head's remaining in-flight paths *cancelling*: once the head can land they cannot help, and they hold CI slots until they stop. The landsignal controller records Runway's terminal result: success marks the head Succeeded, while failure marks it Failed. The result publishes a single dirty signal — no per-dependent fan-out — and the next run refutes paths whose assumption disagrees with the result: *fails* assumptions after success, *succeeds* assumptions after failure. The hand-off is idempotent, so Runway reports success without another land when the change is already present. A chain ordinarily lands one at a time, but a fully covered head can bypass its unsettled predecessors. +- **Failure (no viable path).** A batch fails when every possible future has a failed build — no path can pass, so it can never land. - **Cancel.** A cancelled batch is driven terminal: its in-flight paths are set *cancelling*, then the batch is marked Cancelled once they stop (see Cancellation). ### Conflict relaxation @@ -64,23 +64,23 @@ Conflict analysis is conservative — it flags any *possible* conflict — so he **Not implemented.** An earlier design expressed it per path, with a third assumption value — *ignored* — meaning "this path makes no claim about this dependency". Nothing ever produced one, and the value has been removed rather than left as vocabulary the system could not create. -When relaxation is built, it belongs in the **controller**, as a trim of the dependency list before the snapshot is handed over: the Speculator then sees a head whose dependencies are exactly the ones that count, and a path stays a total function over them — one assumption per dependency, each *succeeds* or *fails*, with no third state to reason about. That keeps the decision where the other correctness decisions live, since dropping a dependency is a judgement about what may land untested, not about which candidate is most promising. It also keeps every consumer honest by construction: a merge gate, a refutation check, or a generator cannot forget to special-case a value that does not exist. +When relaxation is built, it belongs in the **controller**, as a trim of the dependency list before the snapshot is handed over: the Speculator then sees a head whose dependencies are exactly the ones that count, and a path stays a total function over them — one assumption per dependency, each *succeeds* or *fails*, with no third state to reason about. That keeps the decision where the other correctness decisions live, since dropping a dependency is a judgement about what may land untested, not about which candidate is most promising. It also keeps every consumer honest by construction: a land gate, a refutation check, or a generator cannot forget to special-case a value that does not exist. The open question that design has to answer is what a stored path means once the trim changes between runs — a path built against a trimmed list no longer lines up with a head whose list has grown back, and `isWellFormed` rejects it. The per-path marker made that case self-describing; a trim does not, so the trim has to be either stable for a head's lifetime or recorded alongside the path. -Example of the payoff either way: `H` conflicts with `B1` and weak `B2`. Relax `B2`, and `H` merges once `B1` merges and its build passes — even if `B2` later merges. Without it, `H` waits on both. +Example of the payoff either way: `H` conflicts with `B1` and weak `B2`. Relax `B2`, and `H` lands once `B1` lands and its build passes — even if `B2` later lands. Without it, `H` waits on both. ### Bypass large diff -If a batch's passed builds cover *every* way its dependencies could resolve, the outcome is the same either way — so it can merge now, ahead of them. Classic case: a small change stuck behind a slow one is built both with and without it; both pass, and it merges immediately. +If a batch's passed builds cover *every* way its dependencies could resolve, the outcome is the same either way — so it can land now, ahead of them. Classic case: a small change stuck behind a slow one is built both with and without it; both pass, and it lands immediately. The controller checks coverage over only the dependencies that have not settled yet. Settled dependencies pin each surviving path to the outcome that actually happened; for every combination of the remaining dependencies, the path set must contain a passed, unbroken path with that combination of assumptions. If any combination is missing, unbuilt, failed, or contradicted by a settled dependency, the head waits normally. The check only observes paths the Speculator already funded — it does not fund the exponential path space itself or alter the queue's build budget. -Coverage makes the bypass sound because whichever way the dependencies later resolve, a passed build already validated the resulting set of changes. The build order and merge order differ: a path assuming dependency `D` succeeds validates `D` then head `H`, while bypass lands `H` before `D`. SubmitQueue treats those orders as content-equivalent. Runway still performs the real merge, so if the reordered changes conflict textually, the older dependency can fail after the newer head has bypassed it; this is an accepted cost of landing the fully covered head early rather than a licence to put unmergeable content on the target. +Coverage makes the bypass sound because whichever way the dependencies later resolve, a passed build already validated the resulting set of changes. The build order and land order differ: a path assuming dependency `D` succeeds validates `D` then head `H`, while bypass lands `H` before `D`. SubmitQueue treats those orders as content-equivalent. Runway still performs the real land, so if the reordered changes conflict textually, the older dependency can fail after the newer head has bypassed it; this is an accepted cost of landing the fully covered head early rather than a licence to put unlandable content on the target. ### Cancellation -Cancellation is best-effort: a batch marked *cancelling* may still merge if a merge wins the race, so terminal states prevail. A cancel sets the intent; a later run drives it terminal. +Cancellation is best-effort: a batch marked *cancelling* may still land if a land wins the race, so terminal states prevail. A cancel sets the intent; a later run drives it terminal. Two kinds of cancel, split by owner: @@ -93,7 +93,7 @@ Cancelling a path sends a cancel (path ID, attempt) to the build controller, whi ## Speculator Extension -The one extension. It decides *which paths to build and which running ones to cancel* — nothing else; the controller handles the rest (reconciling facts, cancelling ruled-out paths, verdicts, checking output). A swapped-in Speculator changes which paths run, never whether a batch merges or fails. +The one extension. It decides *which paths to build and which running ones to cancel* — nothing else; the controller handles the rest (reconciling facts, cancelling ruled-out paths, verdicts, checking output). A swapped-in Speculator changes which paths run, never whether a batch lands or fails. **The contract** is `Speculate(batches, pathSets) → []Speculation`: @@ -116,6 +116,6 @@ Signatures live in code and are not copied here, so they cannot drift. This sect **Entities** — [`submitqueue/entity/speculation.go`](../../../submitqueue/entity/speculation.go). A `SpeculationPath` is a head batch plus one `PathDependency` per dependency in queue order, each carrying a `DependencyAssumption`: *succeeds* or *fails*. A `SpeculationPathEntry` is the stored record of one chosen path, keyed by a hash of its content, plus its status and attempt number; it holds no build reference — the execution record has that, keyed by (path ID, attempt) — and no ranking score, which means nothing outside the run that produced it. A `SpeculationPathSet` is one head's chosen paths, live and recently finished, under a single version for compare-and-swap. Every logical path is self-describing, but a store may encode the common head and ordered dependency IDs once per set and keep each path's assumptions positionally — one bit per dependency. -**Speculator** — [`submitqueue/extension/speculation/speculator`](../../../submitqueue/extension/speculation/speculator/README.md). `Speculate` takes one queue snapshot (the batches and their path sets) and returns the build and cancel actions it proposes; a path it wants left alone has no entry. Actions must target Speculating heads. Verdicts stay controller-owned, so there is no merge or fail action. +**Speculator** — [`submitqueue/extension/speculation/speculator`](../../../submitqueue/extension/speculation/speculator/README.md). `Speculate` takes one queue snapshot (the batches and their path sets) and returns the build and cancel actions it proposes; a path it wants left alone has no entry. Actions must target Speculating heads. Verdicts stay controller-owned, so there is no land or fail action. **Generator and Allocator** — [`generator`](../../../submitqueue/extension/speculation/generator/README.md) and [`allocator`](../../../submitqueue/extension/speculation/allocator/README.md), the two composition points inside the default Speculator. The Generator opens a pull-based stream of candidate paths over the batches; the Allocator spends the build budget over that stream, reconciling it against the path sets. Both abort on a cancelled context. diff --git a/doc/rfc/submitqueue/workflow.md b/doc/rfc/submitqueue/workflow.md index b50e3958c..b1d590948 100644 --- a/doc/rfc/submitqueue/workflow.md +++ b/doc/rfc/submitqueue/workflow.md @@ -1,8 +1,8 @@ # Orchestrator Workflow -The orchestrator processes land requests through a queue-driven pipeline of small, single-purpose controllers. The gateway accepts a request over RPC and hands it off asynchronously; from there each controller consumes one topic, advances the request or batch, and publishes to the next topic. Most hops carry only an ID — the controller fetches the entity from storage — while a few entry points (`start`, `buildsignal`, `log`) carry the full payload because there is no row to fetch yet. Some stages cross a service boundary: they publish a full payload to the other service's queue and consume a full payload back, because neither service can read the other's storage. (The `validate` and `merge` stages both hand work to runway — a merge-conflict check and the merge itself — and consume its result on `mergeconflictsignal` / `mergesignal`.) See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). +The orchestrator processes land requests through a queue-driven pipeline of small, single-purpose controllers. The gateway accepts a request over RPC and hands it off asynchronously; from there each controller consumes one topic, advances the request or batch, and publishes to the next topic. Most hops carry only an ID — the controller fetches the entity from storage — while a few entry points (`start`, `buildsignal`, `log`) carry the full payload because there is no row to fetch yet. Some stages cross a service boundary: they publish a full payload to the other service's queue and consume a full payload back, because neither service can read the other's storage. The `validate` and `land` stages adapt SubmitQueue's land work to Runway's `MergeRequest` contract and consume `MergeResult` on `landconflictsignal` / `landsignal`. See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). -The pipeline has two cycles: `speculate → build → buildsignal → speculate` (CI feedback loop) and `merge → runway → mergesignal → speculate` (land the batch out of process, then advance the next). `conclude` is the only stage that transitions a request to a terminal state; `log` is an append-only sink that any controller can publish to via `submitqueue/core/request.PublishLog`. +The pipeline has two cycles: `speculate → build → buildsignal → speculate` (CI feedback loop) and `land → runway → landsignal → speculate` (land the batch out of process, then advance the next). `conclude` is the only stage that transitions a request to a terminal state; `log` is an append-only sink that any controller can publish to via `submitqueue/core/request.PublishLog`. ## Diagram @@ -24,18 +24,18 @@ The pipeline has two cycles: `speculate → build → buildsignal → speculate` | | Dedup, fetch metadata, publish | | | check request to runway | | +----------------+-----------------+ - | MergeRequest - | v - | #################################### - | # runway (separate service) # - | # Dry-run merge, emit result # - | ####################+############### - | MergeResult - | v - | +----------------------------------+ - | | mergeconflictsignal | - | | Correlate result, gate request | - | +----------------+-----------------+ + | MergeRequest + | v + | #################################### + | # runway (separate service) # + | # Dry-run merge, emit result # + | ####################+############### + | MergeResult + | v + | +----------------------------------+ + | | landconflictsignal | + | | Correlate result, gate request | + | +----------------+-----------------+ | | RequestID | v | +----------------------------------+ @@ -50,20 +50,20 @@ The pipeline has two cycles: `speculate → build → buildsignal → speculate` | | +------+-----------------+---------+ | | | BatchID | | BatchID | | | v v | - | | +------------------+ +------------------+ | - | | | build | | merge | | - | | | Trigger CI build | | Publish to runway| | - | | +--------+---------+ +--------+---------+ | - | | Build | MergeRequest | - | | v v | - | | +------------------+ #################### | - | +--| buildsignal | # runway (sep.) # | - | BatchID | Feed CI result | # Merge, emit res. # | - | | back to spec. | ########+########### | - | +------------------+ MergeResult | - | ^ v | - | Build (ext.CI) | +------------------+ | - | | | mergesignal |--+ + | | +------------------+ +------------------+ | + | | | build | | land | | + | | | Trigger CI build | | Publish to runway| | + | | +--------+---------+ +--------+---------+ | + | | Build | MergeRequest | + | | v v | + | | +------------------+ #################### | + | +--| buildsignal | # runway (sep.) # | + | BatchID | Feed CI result | # Merge, emit res. # | + | | back to spec. | ########+########### | + | +------------------+ MergeResult | + | ^ v | + | Build (ext.CI) | +------------------+ | + | | | landsignal |--+ | | | Gate batch + fan | | | | +--------+---------+ | | | | BatchID | @@ -82,14 +82,14 @@ The pipeline has two cycles: `speculate → build → buildsignal → speculate` |---|---|---|---| | **gateway/Land** | RPC | start | Accept request, mint ID, log Accepted, hand off async | | **start** | LandRequest | validate, log | Persist Request and emit Started log | -| **validate** | RequestID | merge-conflict-check (runway) | Dedup, fetch change metadata, claim changes, then publish the full check request to runway (keyed by the request id, the correlation id) | -| **mergeconflictsignal** | MergeResult | batch | Correlate runway's result; advance if mergeable, fail if conflicted | +| **validate** | RequestID | merge-conflict-check (Runway) | Dedup, fetch change metadata, claim changes, then adapt and publish the full `MergeRequest` to Runway (keyed by the request id, the correlation id) | +| **landconflictsignal** | MergeResult | batch | Correlate Runway's result; advance if landable, fail if conflicted | | **batch** | RequestID | speculate | Group request into a Batch with dependencies | -| **speculate** | BatchID | build, merge | (stub) Decide whether to verify via CI or land | +| **speculate** | BatchID | build, land | (stub) Decide whether to verify via CI or land | | **build** | BatchID | buildsignal | Trigger CI build for the batch | | **buildsignal** | Build | speculate | Feed CI result back into speculation | -| **merge** | BatchID | merge (runway) | Build the full merge request from the batch's member requests and publish to runway, keyed by the batch id (the correlation id) | -| **mergesignal** | MergeResult | conclude, speculate | Correlate runway's result; mark the batch Succeeded/Failed and fan out | +| **land** | BatchID | runway-merge (Runway) | Build the full land request from the batch's member requests, adapt it to `MergeRequest`, and publish to Runway keyed by the batch id (the correlation id) | +| **landsignal** | MergeResult | conclude, speculate | Correlate Runway's result; mark the batch Succeeded/Failed and fan out | | **conclude** | BatchID | — | Map terminal batch state to request state | | **log** | RequestLog | — | Gateway-owned sink: persists request log events to storage | @@ -97,7 +97,7 @@ The pipeline has two cycles: `speculate → build → buildsignal → speculate` Every *consumed* primary pipeline topic above is paired with a `{topic}_dlq` subscription consumed by a dedicated DLQ controller. The `log` topic is the exception: the orchestrator only publishes to it (the gateway is the sole consumer that persists the request log), so it has no orchestrator-side subscription and therefore no DLQ. The consumer framework moves a message to its DLQ once the primary controller returns a non-retryable error or exhausts retries on a retryable one; without the DLQ side the affected request would stay in a non-terminal state forever and the gateway would still report it as "in progress". -The DLQ controllers do not re-attempt the failed work. They decode the payload to recover the affected request (`RequestID`) or batch (`BatchID`) and drive the entity to a terminal failed state — `RequestStateError` for requests, `BatchStateFailed` for batches, with fan-out to the member requests. A DLQ whose topic carries a full payload rather than a bare ID recovers the id from that payload instead — the `mergeconflictsignal` and `mergesignal` DLQs read it from the runway `MergeResult` the producer echoed back. State writes use the same optimistic-locking CAS as the primary pipeline, so a late primary-pipeline update wins cleanly and a version mismatch is asked back for redelivery. +The DLQ controllers do not re-attempt the failed work. They decode the payload to recover the affected request (`RequestID`) or batch (`BatchID`) and drive the entity to a terminal failed state — `RequestStateError` for requests, `BatchStateFailed` for batches, with fan-out to the member requests. A DLQ whose topic carries a full payload rather than a bare ID recovers the id from that payload instead — the `landconflictsignal` and `landsignal` DLQs read it from the Runway `MergeResult` the producer echoed back. State writes use the same optimistic-locking CAS as the primary pipeline, so a late primary-pipeline update wins cleanly and a version mismatch is asked back for redelivery. DLQ consumers are wired with `errs.AlwaysRetryableProcessor` and a very high `Retry.MaxAttempts`, with their own DLQ disabled. That combination makes reconciliation effectively non-droppable: any failure is forced retryable rather than escalating to a second-level dead-letter that nobody consumes. The trade-off is that a genuinely unprocessable DLQ message — typically a malformed payload — must be removed by an operator. diff --git a/platform/errs/README.md b/platform/errs/README.md index b86c3b104..95bc4d40a 100644 --- a/platform/errs/README.md +++ b/platform/errs/README.md @@ -178,7 +178,7 @@ In particular, **do not reach for `NewRetryableError` just because replaying the ## Extensions Return Go Errors -Extension interfaces (`MergeChecker`, `Storage`, `Publisher`) return `error` values and may define domain-specific sentinels. Most sentinels remain unclassified because their meaning depends on the call site; for example, `storage.ErrNotFound` might be a user error in one controller and an infrastructure error in another. A sentinel whose classification is intrinsic in every context may carry that classification at its declaration. `storage.ErrVersionMismatch`, for example, is always a retryable infrastructure error because it reports a lost optimistic-concurrency race. +Extension interfaces (`ChangeProvider`, `Storage`, `Publisher`) return `error` values and may define domain-specific sentinels. Most sentinels remain unclassified because their meaning depends on the call site; for example, `storage.ErrNotFound` might be a user error in one controller and an infrastructure error in another. A sentinel whose classification is intrinsic in every context may carry that classification at its declaration. `storage.ErrVersionMismatch`, for example, is always a retryable infrastructure error because it reports a lost optimistic-concurrency race. Controllers should return intrinsically classified sentinels without adding another framework wrapper. The declaration remains reusable across implementations while every caller observes the same classification. diff --git a/platform/extension/messagequeue/README.md b/platform/extension/messagequeue/README.md index ab3b30837..3a143f2e7 100644 --- a/platform/extension/messagequeue/README.md +++ b/platform/extension/messagequeue/README.md @@ -100,7 +100,7 @@ for delivery := range deliveries { A message ID is the deduplication key, scoped to its topic and partition key. A backend matches a publish against messages it still holds — including ones already consumed, since reclamation is lazy and may lag delivery by an unbounded interval — and a collision is reported to the publisher as a success that stored nothing. There is no error to retry and no row to deliver. -The ID therefore names the *occasion* to publish, not the entity published about. An entity's own ID buys exactly one message for that entity for as long as the backend remembers the first, so a stage that announces a batch at creation and another that wakes it after a merge would collide, and the wake-up would vanish. +The ID therefore names the *occasion* to publish, not the entity published about. An entity's own ID buys exactly one message for that entity for as long as the backend remembers the first, so a stage that announces a batch at creation and another that wakes it after a land would collide, and the wake-up would vanish. Producers do not choose IDs by hand. They publish through `platform/publish`, whose `IntentID(entityID, cause...)` composes the entity with the cause of this particular message: a retry of the same cause dedups, which is what makes redelivery safe, while a new cause about the same entity can never be swallowed. `UniqueID` is the fallback for a cause with nothing stable to name it by, and it trades that idempotency for guaranteed delivery. diff --git a/platform/extension/messagequeue/mysql/README.md b/platform/extension/messagequeue/mysql/README.md index c9fa8c69a..914665e64 100644 --- a/platform/extension/messagequeue/mysql/README.md +++ b/platform/extension/messagequeue/mysql/README.md @@ -26,11 +26,11 @@ defer q.Close() // Publish msg := entityqueue.NewMessage("msg-id", []byte(`{"data": "value"}`), "repo-123", nil) -q.Publisher().Publish(ctx, "merge_events", msg) +q.Publisher().Publish(ctx, "land_events", msg) // Subscribe with per-subscription config subConfig := extqueue.DefaultSubscriptionConfig("worker-1", "orchestrator") -deliveryCh, _ := q.Subscriber().Subscribe(ctx, "merge_events", subConfig) +deliveryCh, _ := q.Subscriber().Subscribe(ctx, "land_events", subConfig) for delivery := range deliveryCh { if err := process(delivery.Message()); err != nil { delivery.Nack(ctx) // Retry diff --git a/platform/extension/messagequeue/mysql/ctl/README.md b/platform/extension/messagequeue/mysql/ctl/README.md index e6cb64c2c..8cbbf1780 100644 --- a/platform/extension/messagequeue/mysql/ctl/README.md +++ b/platform/extension/messagequeue/mysql/ctl/README.md @@ -25,14 +25,14 @@ Via Make (uses Bazel): ```bash make run-queue-admin ARGS="list-topics" -make run-queue-admin ARGS="topic-stats --topic merge_queue" +make run-queue-admin ARGS="topic-stats --topic land_queue" ``` Via Bazel directly: ```bash bazel run //platform/extension/messagequeue/mysql/ctl -- list-topics -bazel run //platform/extension/messagequeue/mysql/ctl -- topic-stats --topic merge_queue +bazel run //platform/extension/messagequeue/mysql/ctl -- topic-stats --topic land_queue ``` ## Commands @@ -44,20 +44,20 @@ bazel run //platform/extension/messagequeue/mysql/ctl -- topic-stats --topic mer queue-admin list-topics # Detailed stats for a topic (total messages, DLQ count, partitions, consumer groups) -queue-admin topic-stats --topic merge_queue +queue-admin topic-stats --topic land_queue ``` ### Inspect Messages ```bash # List messages (default limit 50) -queue-admin list-messages --topic merge_queue +queue-admin list-messages --topic land_queue # Filter by partition, custom limit -queue-admin list-messages --topic merge_queue --partition uber/cadence --limit 10 +queue-admin list-messages --topic land_queue --partition uber/cadence --limit 10 # Full message details including payload and metadata -queue-admin inspect-message --topic merge_queue --message-id msg-123 +queue-admin inspect-message --topic land_queue --message-id msg-123 ``` ### Manage Messages @@ -66,13 +66,13 @@ Destructive commands prompt for confirmation by default. Use `--no-interactive` ```bash # Delete a single message -queue-admin delete-message --topic merge_queue --message-id msg-123 +queue-admin delete-message --topic land_queue --message-id msg-123 # Purge all messages from a topic -queue-admin purge-topic --topic merge_queue +queue-admin purge-topic --topic land_queue # Skip confirmation prompt (for scripting) -queue-admin purge-topic --topic merge_queue --no-interactive +queue-admin purge-topic --topic land_queue --no-interactive ``` ### Dead Letter Queue (DLQ) @@ -81,26 +81,26 @@ DLQ messages live in the same `queue_messages` table under `topic + "_dlq"` (def ```bash # List DLQ messages -queue-admin list-dlq --topic merge_queue +queue-admin list-dlq --topic land_queue # Inspect a DLQ message (use the DLQ topic name) -queue-admin inspect-message --topic merge_queue_dlq --message-id msg-456 +queue-admin inspect-message --topic land_queue_dlq --message-id msg-456 # Move a DLQ message back to the original topic -queue-admin requeue-dlq --topic merge_queue --message-id msg-456 +queue-admin requeue-dlq --topic land_queue --message-id msg-456 # Purge all DLQ messages -queue-admin purge-dlq --topic merge_queue +queue-admin purge-dlq --topic land_queue # Custom DLQ suffix (if not using default "_dlq") -queue-admin list-dlq --topic merge_queue --dlq-suffix _dead +queue-admin list-dlq --topic land_queue --dlq-suffix _dead ``` ### Consumer Lag ```bash # Per-partition lag for all consumer groups on a topic -queue-admin consumer-lag --topic merge_queue +queue-admin consumer-lag --topic land_queue ``` Output shows `ACKED` (last processed offset), `LATEST` (newest message offset), and `LAG` (unprocessed count) per partition per consumer group. @@ -115,10 +115,10 @@ queue-admin list-offsets queue-admin list-offsets --consumer-group orchestrator # Reset offset to 0 (reprocess all messages) -queue-admin reset-offset --consumer-group orchestrator --topic merge_queue --partition uber/cadence +queue-admin reset-offset --consumer-group orchestrator --topic land_queue --partition uber/cadence # Reset to a specific offset -queue-admin reset-offset --consumer-group orchestrator --topic merge_queue --partition uber/cadence --offset 42 +queue-admin reset-offset --consumer-group orchestrator --topic land_queue --partition uber/cadence --offset 42 ``` ### Partition Leases @@ -132,7 +132,7 @@ queue-admin stale-leases # default 60s threshold queue-admin stale-leases --threshold 30000 # 30s threshold # Force-release a stuck lease -queue-admin release-lease --consumer-group orchestrator --topic merge_queue --partition uber/cadence +queue-admin release-lease --consumer-group orchestrator --topic land_queue --partition uber/cadence ``` ### JSON Output @@ -141,6 +141,6 @@ Add `--json` to any read command for machine-readable output: ```bash queue-admin list-topics --json -queue-admin consumer-lag --topic merge_queue --json -queue-admin list-messages --topic merge_queue --json | jq '.[] | .ID' +queue-admin consumer-lag --topic land_queue --json +queue-admin list-messages --topic land_queue --json | jq '.[] | .ID' ``` diff --git a/platform/extension/messagequeue/mysql/schema/queue_messages.sql b/platform/extension/messagequeue/mysql/schema/queue_messages.sql index a3a655365..1bb5bf406 100644 --- a/platform/extension/messagequeue/mysql/schema/queue_messages.sql +++ b/platform/extension/messagequeue/mysql/schema/queue_messages.sql @@ -1,7 +1,7 @@ -- MESSAGES TABLE (Immutable Log) -- Single table for all topics. Partition key determines distribution across workers. -- Messages are append-only; per-consumer-group delivery tracking is in queue_delivery_state. --- Example: topic="merge_queue", partition_key="uber/cadence" +-- Example: topic="land_queue", partition_key="uber/cadence" CREATE TABLE IF NOT EXISTS queue_messages ( -- Auto-incrementing global offset for ordering diff --git a/platform/hook/README.md b/platform/hook/README.md index ec6a8a87d..12e1f5561 100644 --- a/platform/hook/README.md +++ b/platform/hook/README.md @@ -4,7 +4,7 @@ The consumer side of the hooks framework: the stage that turns hook events on a ## Why a stage at all -Side effects must never stall or fail the pipeline, and "fire and forget" must not mean lossy — a merge-failure comment that silently never posts is a support ticket. A durable queue between the two resolves the tension: the producer's obligation ends once the event is enqueued, which is fast and local, and everything after that gets real retry semantics and a dead-letter queue. +Side effects must never stall or fail the pipeline, and "fire and forget" must not mean lossy — a land-failure comment that silently never posts is a support ticket. A durable queue between the two resolves the tension: the producer's obligation ends once the event is enqueued, which is fast and local, and everything after that gets real retry semantics and a dead-letter queue. Calling hooks inline would give up both halves. It couples pipeline latency to whatever an integration talks to, and a crash between the state write and the call drops the notification with nothing to replay. diff --git a/platform/hook/dlq.go b/platform/hook/dlq.go index b34b1556a..f73e84fde 100644 --- a/platform/hook/dlq.go +++ b/platform/hook/dlq.go @@ -41,7 +41,7 @@ const reconcileOp = "reconcile" // // That is a deliberate step up from the log topic's DLQ, which warns and moves // on. Dropping an observability row costs a gap in a read model; dropping a -// merge-failure comment costs a support ticket, and nothing else in the system +// land-failure comment costs a support ticket, and nothing else in the system // will notice it is missing. type DLQController struct { logger *zap.SugaredLogger diff --git a/platform/metrics/README.md b/platform/metrics/README.md index b88cf6146..7a2426222 100644 --- a/platform/metrics/README.md +++ b/platform/metrics/README.md @@ -85,7 +85,7 @@ There is no default bucket set. The package exports four common sets: |-----|-------|---------| | `FastLatencyBuckets` | ~100µs – 5s | Fast in-process work such as scoring, cache lookups, and CPU-bound operations | | `StorageLatencyBuckets` | ~1ms – 1m | Storage and message-queue round trips such as database reads, writes, publishing, and consuming | -| `LongLatencyBuckets` | ~5ms – 4h | Long-running pipeline work and external calls such as builds, merges, pushes, and provider calls | +| `LongLatencyBuckets` | ~5ms – 4h | Long-running pipeline work and external calls such as builds, lands, pushes, and provider calls | | `ChangeAgeBuckets` | ~1m – 30d | Elapsed time measured from a source-control change's commit timestamp rather than from work this system started | Pass one of these sets or a custom `tally.DurationBuckets` to `Begin` or `NamedHistogram`. diff --git a/platform/metrics/metrics.go b/platform/metrics/metrics.go index 5bba9793f..aab5feb62 100644 --- a/platform/metrics/metrics.go +++ b/platform/metrics/metrics.go @@ -108,7 +108,7 @@ var ( } // LongLatencyBuckets suits long-running pipeline work and external calls - // (~5ms to hours): builds, merges, git pushes, and external provider calls. + // (~5ms to hours): builds, lands, git pushes, and external provider calls. LongLatencyBuckets = tally.DurationBuckets{ 5 * time.Millisecond, 10 * time.Millisecond, diff --git a/service/README.md b/service/README.md index 72c76bb46..0177846ba 100644 --- a/service/README.md +++ b/service/README.md @@ -6,14 +6,14 @@ Each domain has its own subdirectory with a dedicated README: - [`submitqueue/`](submitqueue/README.md) — the multi-service SubmitQueue domain (Gateway + Orchestrator). - [`stovepipe/`](stovepipe/README.md) — the single-service Stovepipe domain (ingest → process → build → buildsignal → record). -- [`runway/`](runway/README.md) — the single-service Runway landing service (consumes the merge queues). +- [`runway/`](runway/README.md) — the single-service Runway merge execution service. ## Services | Service | Port | Domain | RPCs | Backing stores | |---------|------|--------|------|----------------| | **SubmitQueue Gateway** | 8081 | `submitqueue` | `Ping`, `Land`, `Cancel`, `GetRequestSummaryByID`, `GetRequestSummaryByChangeURI`, `List`, `GetRequestHistoryByID`, `GetRequestHistoryByChangeURI` | MySQL app + queue | -| **SubmitQueue Orchestrator** | 8082 | `submitqueue` | `Ping` (+ consumes start, cancel, validate, merge-conflict-check-signal, batch, dependency-analysis, speculate, build, buildsignal, submitqueue-merge, merge-signal, conclude, submitqueue-hook, and paired DLQ topics) | MySQL app + queue | +| **SubmitQueue Orchestrator** | 8082 | `submitqueue` | `Ping` (+ consumes start, cancel, validate, merge-conflict-check-signal, batch, dependency-analysis, speculate, build, buildsignal, submitqueue-land, merge-signal, conclude, submitqueue-hook, and paired DLQ topics) | MySQL app + queue | | **Stovepipe** | 8083 | `stovepipe` | `Ping`, `Ingest` (+ consumes process, build, buildsignal, record, stovepipe-hook, and paired DLQ topics) | MySQL storage + queue | | **Runway** | 8086 | `runway` | `Ping` (+ consumes merge-conflict-check & runway-merge topics) | MySQL queue | diff --git a/service/submitqueue/README.md b/service/submitqueue/README.md index cc80c582b..9c94dee19 100644 --- a/service/submitqueue/README.md +++ b/service/submitqueue/README.md @@ -1,6 +1,6 @@ # SubmitQueue Services -Runnable wiring for the **SubmitQueue** domain's two services — the Gateway (entry point for land requests) and the Orchestrator (coordinates the pipeline) — wired with MySQL-backed extensions. The full Docker Compose workflow also starts Runway, which performs merge-conflict checks and merges. +Runnable wiring for the **SubmitQueue** domain's two services — the Gateway (entry point for land requests) and the Orchestrator (coordinates the pipeline) — wired with MySQL-backed extensions. The full Docker Compose workflow also starts Runway, which performs merge-conflict checks and merges on SubmitQueue's behalf. ## Starting diff --git a/stovepipe/README.md b/stovepipe/README.md index 269e8388d..ec1a6c9cd 100644 --- a/stovepipe/README.md +++ b/stovepipe/README.md @@ -1,6 +1,6 @@ # Stovepipe -Stovepipe is a post-merge validation service. Its layout: +Stovepipe is a post-land validation service. Its layout: - `controller/` — business logic (transport-agnostic). Exposes the `Ping` and `Ingest` RPCs, and consumes the internal pipeline stages (`process`, `build`, `buildsignal`, `record`) plus a DLQ reconciler. diff --git a/submitqueue/core/changeset/README.md b/submitqueue/core/changeset/README.md index 4e8c9463c..fb32905f0 100644 --- a/submitqueue/core/changeset/README.md +++ b/submitqueue/core/changeset/README.md @@ -1,6 +1,6 @@ # changeset -`changeset` resolves batch identity into the changes a batch contains. Current consumers include build-runner and scorer implementations and the path-overlap conflict analyzer. The merge controller loads member requests directly because its Runway payload preserves one ordered merge step per request. +`changeset` resolves batch identity into the changes a batch contains. Current consumers include build-runner and scorer implementations and the path-overlap conflict analyzer. The land controller loads member requests directly because its Runway payload preserves one ordered merge step per request. ## Why it exists diff --git a/submitqueue/core/changeset/changeset.go b/submitqueue/core/changeset/changeset.go index 3edf21a8f..b3709d559 100644 --- a/submitqueue/core/changeset/changeset.go +++ b/submitqueue/core/changeset/changeset.go @@ -14,7 +14,7 @@ // Package changeset resolves batch identity into the changes a batch contains. // It is the single place the orchestrator walks batch -> requests -> changes, -// consolidating what the build and merge controllers each did privately. +// consolidating controller-side and backend-specific traversal. // Decision/action extensions (scorer, buildrunner, and future // detail-aware conflict analyzers) take thin identity entities and resolve their // granular content through an injected Resolver instead of being handed @@ -31,15 +31,15 @@ import ( ) // Resolver turns batch identity into the changes the batch contains. Both methods -// operate on a single batch — callers with several batches (a build's base, a -// merge train) loop and keep the per-batch boundary by holding a slice per batch. +// operate on a single batch — callers with several batches loop and keep the +// per-batch boundary by holding a slice per batch. // The two methods differ only in fidelity: ChangesForBatch is the cheap URI-only // view; DetailedForBatch reads the change store for provider details. type Resolver interface { // ChangesForBatch resolves a batch's contained requests into their raw // changes (URIs only; no change-store read), in batch.Contains order. A batch - // with no requests yields an empty slice. Used by the build (base/head) and - // merge stages. + // with no requests yields an empty slice. Used by build runners for their + // base and head batches. ChangesForBatch(ctx context.Context, batch entity.Batch) ([]change.Change, error) // DetailedForBatch resolves a batch into its normalized, batch-level view: diff --git a/submitqueue/entity/request.go b/submitqueue/entity/request.go index 83bd30a5d..5d970d25d 100644 --- a/submitqueue/entity/request.go +++ b/submitqueue/entity/request.go @@ -29,7 +29,7 @@ const ( RequestStateUnknown RequestState = "" // RequestStateStarted is the initial state of a land request. It is confirmed by the system but the processing is not started yet. RequestStateStarted RequestState = "started" - // RequestStateValidated indicates that the request has been validated (duplicate check, merge check etc.) successfully. + // RequestStateValidated indicates that the request has been validated (duplicate check, landability check, etc.) successfully. RequestStateValidated RequestState = "validated" // RequestStateBatched indicates that the request is enrolled in a batch whose dependencies have been // resolved. The CAS-write of this state is the serialization point against cancellation: it lands with @@ -45,7 +45,7 @@ const ( RequestStateError RequestState = "error" // RequestStateCancelling is the non-terminal intent state set when the user has requested cancellation but the // request has not yet been transitioned to RequestStateCancelled. A request in this state may still reach - // RequestStateLanded or RequestStateError if a concurrent merge or failure wins the race; those terminal + // RequestStateLanded or RequestStateError if a concurrent land or failure wins the race; those terminal // states prevail. Forward-progress controllers must treat this state the same as terminal (i.e. do not start // any new work on the request). RequestStateCancelling RequestState = "cancelling" @@ -68,7 +68,7 @@ func IsRequestStateHalted(s RequestState) bool { return IsRequestStateTerminal(s) || s == RequestStateCancelling } -// Request defines a request to land (merge into target branch of the source control repository) a set of code changes. +// Request defines a request to land a set of code changes on the source control repository's target branch. // The object is immutable after creation. type Request struct { // **************** diff --git a/submitqueue/entity/request_log.go b/submitqueue/entity/request_log.go index 72bad9ef6..a1be20fc3 100644 --- a/submitqueue/entity/request_log.go +++ b/submitqueue/entity/request_log.go @@ -45,10 +45,10 @@ const ( // RequestStatusStarted is the initial status of a request. It corresponds to the RequestStateStarted state and typically set by the orchestrator service when the request is received and persisted to the operating database. RequestStatusStarted RequestStatus = "started" - // RequestStatusValidating indicates that the request is currently being validated (e.g., duplicate check, merge check, etc.). + // RequestStatusValidating indicates that the request is currently being validated (e.g., duplicate check, landability check, etc.). RequestStatusValidating RequestStatus = "validating" - // RequestStatusValidated indicates that the request has been validated (duplicate check, merge check etc.) successfully. It corresponds to the RequestStateValidated state. + // RequestStatusValidated indicates that the request has been validated (duplicate check, landability check, etc.) successfully. It corresponds to the RequestStateValidated state. RequestStatusValidated RequestStatus = "validated" // RequestStatusBatching indicates that a batch has been created for the request and is resolving what it must serialize behind. @@ -75,7 +75,7 @@ const ( RequestStatusError RequestStatus = "error" // RequestStatusCancelling indicates that the user has requested cancellation but the request has not yet transitioned - // to the RequestStateCancelled state. Cancellation is best-effort: a request that has already been merged or that + // to the RequestStateCancelled state. Cancellation is best-effort: a request that has already landed or that // races to completion before the cancel propagates through the pipeline may still land. Observers should treat this // as intent only and rely on RequestStatusCancelled (or RequestStatusLanded) for the terminal outcome. Emitted by // the gateway when the Cancel RPC is received. diff --git a/submitqueue/entity/speculation.go b/submitqueue/entity/speculation.go index 1d199c902..734dfe1f6 100644 --- a/submitqueue/entity/speculation.go +++ b/submitqueue/entity/speculation.go @@ -185,7 +185,7 @@ type SpeculationPathSet struct { } // PathAction is an action proposed on a speculation path. The set is limited to -// build and cancel; there is no merge or fail action, because a batch's verdict +// build and cancel; there is no land or fail action, because a batch's verdict // is a controller-owned fact, not a proposed action. type PathAction string diff --git a/submitqueue/extension/speculation/allocator/sticky/sticky.go b/submitqueue/extension/speculation/allocator/sticky/sticky.go index fac42c657..d5135b6f3 100644 --- a/submitqueue/extension/speculation/allocator/sticky/sticky.go +++ b/submitqueue/extension/speculation/allocator/sticky/sticky.go @@ -81,7 +81,7 @@ func (a alloc) Allocate(ctx context.Context, pathSets []entity.SpeculationPathSe // keeps its slot until it actually stops. // // Only the path's status matters. No batch state enters this - // decision — "merging" and the rest are states of a batch, never of + // decision — "landing" and the rest are states of a batch, never of // a path — so the rule is simply that CI is still busy with it. funded[entry.ID] = true } diff --git a/submitqueue/extension/speculation/generator/bestfirst/README.md b/submitqueue/extension/speculation/generator/bestfirst/README.md index bd34769dd..b661b2e8e 100644 --- a/submitqueue/extension/speculation/generator/bestfirst/README.md +++ b/submitqueue/extension/speculation/generator/bestfirst/README.md @@ -10,7 +10,7 @@ The [best-first speculation path generation RFC](../../../../../doc/rfc/submitqu - `Next` removes the highest-ranked candidate, advances only that head's stream, constructs that candidate's complete path, and returns it. Pulling long enough returns every path exactly once in non-increasing score order. - Ranking scores are sums of log probabilities, avoiding underflow while preserving probability order. They are meaningful only within the run that produced them. - Exact ties prefer fewer flips; head ID then decides between heads (the cross-head heap holds one candidate per head), and taken flip indexes decide within a head. -- A dependency counts as resolved only once it is terminal. Merging and cancelling are both still in progress and either can end the other way, so both stay open questions here. Whether a path betting against a merging dependency is worth funding is a matter of price, and price is the scorer's to say. +- A dependency counts as resolved only once it is terminal. Landing and cancelling are both still in progress and either can end the other way, so both stay open questions here. Whether a path betting against a landing dependency is worth funding is a matter of price, and price is the scorer's to say. - A dependency that cannot be priced — the scorer call failed, the score was not a probability, or the snapshot never carried the batch — is treated as very likely to succeed rather than ending the run. One unusable number costs its own estimate, never the queue's whole set of candidates. A batch missing from the snapshot is never handed to the scorer at all: it would resolve to a zero batch belonging to no queue. - The snapshot must contain every batch a head's direct dependencies reference, carry unique non-empty batch IDs, and give no head an empty, duplicate, or self dependency. That is the caller's precondition, not something checked here: a malformed snapshot yields undefined candidates rather than an error. diff --git a/submitqueue/extension/speculation/scorer/scorer.go b/submitqueue/extension/speculation/scorer/scorer.go index a0d75401c..c654cd821 100644 --- a/submitqueue/extension/speculation/scorer/scorer.go +++ b/submitqueue/extension/speculation/scorer/scorer.go @@ -29,7 +29,7 @@ type Scorer interface { // ultimately succeeds — reaches its terminal Succeeded state with its // changes landed, rather than Failed or Cancelled. A passing build is // necessary but not sufficient: a batch whose build already passed can - // still fail to merge, so this is the probability of the final outcome, + // still fail to land, so this is the probability of the final outcome, // not of the build alone. It is handed the batch identity and resolves the // batch's changes itself through an injected changeset.Resolver. // diff --git a/submitqueue/extension/speculation/speculator/README.md b/submitqueue/extension/speculation/speculator/README.md index 62343796a..3b842a7c7 100644 --- a/submitqueue/extension/speculation/speculator/README.md +++ b/submitqueue/extension/speculation/speculator/README.md @@ -1,6 +1,6 @@ # speculator -The `speculator` package defines the one speculation extension the speculate controller calls. A `Speculator` decides **which speculation paths to build and which running ones to cancel**, within the queue's build budget — and nothing else. It can never express a verdict: whether a batch merges or fails is fixed by the facts and computed by the controller, so swapping in a different `Speculator` changes which paths run, never a batch's outcome. +The `speculator` package defines the one speculation extension the speculate controller calls. A `Speculator` decides **which speculation paths to build and which running ones to cancel**, within the queue's build budget — and nothing else. It can never express a verdict: whether a batch lands or fails is fixed by the facts and computed by the controller, so swapping in a different `Speculator` changes which paths run, never a batch's outcome. `Speculate` is handed the queue's in-flight batches plus any finalized batches still referenced as dependencies (each with its dependency list and state) and every path set for them — live and recently finished, so a `Speculator` will not re-propose a path that already passed or failed. It returns a list of build and cancel actions; a path it wants left as-is has no entry in the result. The controller validates the output (dropping builds it shouldn't propose and rejecting cancels of passed paths), so an implementation may read extra injected data without affecting correctness. diff --git a/submitqueue/extension/speculation/speculator/speculator.go b/submitqueue/extension/speculation/speculator/speculator.go index acf7237f8..296d1c19e 100644 --- a/submitqueue/extension/speculation/speculator/speculator.go +++ b/submitqueue/extension/speculation/speculator/speculator.go @@ -25,7 +25,7 @@ import ( // Speculator decides which speculation paths to build and which running ones to // cancel, within the queue's build budget. It is the only speculation extension the // speculate controller calls. It can never express a verdict: whether a batch -// merges or fails is fixed by the facts and computed by the controller, so a +// lands or fails is fixed by the facts and computed by the controller, so a // swapped-in Speculator changes which paths run, never a batch's outcome. type Speculator interface { // Speculate proposes this run's path actions. diff --git a/submitqueue/extension/speculation/speculator/standard/README.md b/submitqueue/extension/speculation/speculator/standard/README.md index c5d869e65..c96f1f8f6 100644 --- a/submitqueue/extension/speculation/speculator/standard/README.md +++ b/submitqueue/extension/speculation/speculator/standard/README.md @@ -4,7 +4,7 @@ The `standard` `Speculator` funds the queue's most promising speculation paths f Each run it considers candidate paths in descending order of their probability of being the future that actually happens, and proposes builds down that ranking. Paths already pending or building keep the slot they hold rather than restarting; paths whose builds already finished are skipped for as long as their records remain in the supplied path sets, so a finished path can be proposed again — for a retry, say — once retention drops it; new builds fill whatever budget remains. -When the budget runs out, everything below the cut waits for a later run. That is safe because the propose-side cannot invent a batch verdict: the speculate controller still decides merge from the persisted paths, including complete coverage of unsettled dependencies. +When the budget runs out, everything below the cut waits for a later run. That is safe because the propose-side cannot invent a batch verdict: the speculate controller still decides land from the persisted paths, including complete coverage of unsettled dependencies. Both halves are swappable. The ranking is the `Generator`'s: the default `bestfirst` scores each path by the probability that all its assumptions hold. The budget policy is the `Allocator`'s: the default `sticky` fills only free slots and never preempts, where a preempting allocator would cancel a low-value in-flight path to fund a better one. diff --git a/submitqueue/gateway/controller/cancel.go b/submitqueue/gateway/controller/cancel.go index 644ae3448..2cd42459e 100644 --- a/submitqueue/gateway/controller/cancel.go +++ b/submitqueue/gateway/controller/cancel.go @@ -32,7 +32,7 @@ import ( // CancelController handles cancel business logic for the gateway. It validates the request, // records a RequestStatusCancelling log entry (intent-only — cancellation is best-effort -// and may still race a successful merge), publishes a CancelRequest to the cancel topic, +// and may still race a successful land), publishes a CancelRequest to the cancel topic, // and returns a response. The orchestrator-side cancel controller performs the actual // state transitions and emits the terminal RequestStatusCancelled log entry. type CancelController interface { @@ -66,7 +66,7 @@ func NewCancelController(logger *zap.SugaredLogger, scope tally.Scope, stores st // is performed asynchronously by the orchestrator cancel controller. Cancel is idempotent: // the orchestrator treats already-terminal requests as a no-op. // -// Cancellation is best-effort: a request that has already merged or that races to +// Cancellation is best-effort: a request that has already landed or that races to // completion before the cancel propagates may still land. The RequestStatusCancelling // entry written here records the user's intent; the terminal outcome is reflected by a // later RequestStatusCancelled (orchestrator side) or RequestStatusLanded entry. diff --git a/submitqueue/orchestrator/README.md b/submitqueue/orchestrator/README.md index 6dea30d7c..50a837c41 100644 --- a/submitqueue/orchestrator/README.md +++ b/submitqueue/orchestrator/README.md @@ -8,17 +8,17 @@ The pipeline is queue-driven: each stage consumes one topic, advances one entity - **start** — receives `LandRequest` from the gateway, persists the `Request` entity, and emits `Started`. - **cancel** — records cancellation intent and hands affected batches to speculation for best-effort cancellation. -- **validate** — checks for duplicates, resolves change metadata, and publishes a `MergeRequest` to Runway's `merge-conflict-check` topic. -- **merge-conflict-check-signal** — correlates the dry-run result, fails the request on conflict, or forwards it to batching. +- **validate** — checks for duplicates, resolves change metadata, and adapts the land request to Runway's `MergeRequest` on the `merge-conflict-check` topic. +- **landconflictsignal** — consumes `MergeResult` from Runway's `merge-conflict-check-signal`, fails the request on conflict, or forwards it to batching. - **batch** — creates an inert batch attempt and hands it to dependency analysis. - **dependency-analysis** — enrols requests, computes dependencies, and promotes the selected batch attempt. - **speculate** — reconciles queue-wide path state, decides outcomes, and allocates speculative builds. - **build** — triggers a CI build for a speculative path. - **buildsignal** — polls or receives CI state, records the result, wakes `speculate`, and holds non-terminal deliveries until the next poll. -- **merge** — publishes a committing `MergeRequest` to Runway's `runway-merge` topic. -- **merge-signal** — correlates the merge result and fans out to `conclude` and back to `speculate`. +- **land** — adapts a batch to a committing `MergeRequest` on Runway's `runway-merge` topic. +- **landsignal** — consumes `MergeResult` from Runway's `merge-signal`, correlates the land result, and fans out to `conclude` and back to `speculate`. - **conclude** — maps the terminal batch state to the request states. - **submitqueue-hook** — dispatches lifecycle hook events to configured integrations. - **DLQ reconcilers** — one per primary consumed topic, driving stuck requests/batches to a conservative terminal `failed` state. -The orchestrator publishes request-log entries to `log`, but does not consume or persist them; the gateway owns that stage. It also publishes full cross-service requests to Runway's merge-conflict-check and merge topics. +The orchestrator publishes request-log entries to `log`, but does not consume or persist them; the gateway owns that stage. It also adapts SubmitQueue's land terminology to full cross-service requests on Runway's merge-conflict-check and merge topics. diff --git a/submitqueue/orchestrator/controller/README.md b/submitqueue/orchestrator/controller/README.md index 4fc03735a..9f69b9a4b 100644 --- a/submitqueue/orchestrator/controller/README.md +++ b/submitqueue/orchestrator/controller/README.md @@ -50,7 +50,7 @@ Such effects require a provider-supported idempotency key, a stable operation id The speculate topic is a dirty signal for a queue, not a command to transition only the named batch. A run reloads the queue's in-flight batches, dependency outcomes, path sets, and build results; admits any batches still in `Created`; commits outcomes to a fixed point; asks the configured speculator for new proposals; persists changed path sets; and dispatches pending builds. -A batch can advance to merge when one passed path matches all settled dependency outcomes. It can also advance while dependencies remain unsettled when passed paths cover every possible outcome of those dependencies, proving that the head passed regardless of how they finish. +A batch can advance to land when one passed path matches all settled dependency outcomes. It can also advance while dependencies remain unsettled when passed paths cover every possible outcome of those dependencies, proving that the head passed regardless of how they finish. Path-set changes are persisted before build dispatch. Terminal outcomes are committed before later decisions derive from them. Selected request-log announcements are intentionally published before their corresponding state write so replay cannot lose the observation. diff --git a/submitqueue/orchestrator/controller/conclude/conclude.go b/submitqueue/orchestrator/controller/conclude/conclude.go index 8f7c28d38..86432d831 100644 --- a/submitqueue/orchestrator/controller/conclude/conclude.go +++ b/submitqueue/orchestrator/controller/conclude/conclude.go @@ -106,7 +106,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er ) // Map batch terminal state to request state. - // We expect the batch to be in a terminal state as written by the merge + // We expect the batch to be in a terminal state as written by the land // controller (Succeeded) or the speculate controller (Failed via // failOnDependency, Cancelled via cancelBatch). requestState, err := batchStateToRequestState(batch.State) diff --git a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go index cb36da1d8..66f8ec81d 100644 --- a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go +++ b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go @@ -219,7 +219,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er metrics.NamedCounter(c.metricsScope, opName, "reannounced", 1) default: - // Speculating or merging: the announcement landed and the batch has + // Speculating or landing: the announcement landed and the batch has // already moved past this stage. metrics.NamedCounter(c.metricsScope, opName, "already_admitted", 1) return nil @@ -246,7 +246,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // with its own hand-off. This is where that is resolved: the topic is // partitioned by queue and consumed in order, so the first hand-off through // here enrols the request and the second finds it and stops. Without the check -// the same change would end up in two live batches, both admitted, both merged. +// the same change would end up in two live batches, both admitted, both landed. func (c *Controller) requestEnrolledInAnotherBatch(ctx context.Context, store storage.Storage, batch entity.Batch) (bool, error) { for _, requestID := range batch.Contains { existing, stale, err := corebatch.FindByRequestID(ctx, store, requestID) diff --git a/submitqueue/orchestrator/controller/dlq/README.md b/submitqueue/orchestrator/controller/dlq/README.md index befa1bb9f..a4dfe6ccf 100644 --- a/submitqueue/orchestrator/controller/dlq/README.md +++ b/submitqueue/orchestrator/controller/dlq/README.md @@ -31,7 +31,7 @@ Two controller shapes cover the eleven primary pipeline topics: | Controller | Topics | Decoded ID | Terminal state | |---|---|---|---| | `NewDLQRequestController` | `start`, `validate`, `batch`, `cancel`, `log` | `RequestID` | `RequestStateError` | -| `NewDLQBatchController` | `speculate`, `build`, `merge`, `conclude` | `BatchID` | `BatchStateFailed` + fan-out to member requests as `RequestStateError` | +| `NewDLQBatchController` | `speculate`, `build`, `land`, `conclude` | `BatchID` | `BatchStateFailed` + fan-out to member requests as `RequestStateError` | `buildsignal` carries a `Build` payload and has its own small dedicated controller. The split exists because the DLQ message payload shape mirrors the primary topic's payload shape (the queue framework preserves bytes verbatim under the `_dlq` topic name), so the decoder is what changes per topic — not the reconciliation step. The package-level `RequestIDDecoder` interface plus `DecodeLandRequestID` / `DecodeCancelRequestID` / `DecodeRequestID` cover the three payload shapes used by request-scoped topics. diff --git a/submitqueue/orchestrator/controller/dlq/batch.go b/submitqueue/orchestrator/controller/dlq/batch.go index 5111926bf..9bc25fb65 100644 --- a/submitqueue/orchestrator/controller/dlq/batch.go +++ b/submitqueue/orchestrator/controller/dlq/batch.go @@ -27,7 +27,7 @@ import ( ) // batchController is the DLQ reconciler for batch-scoped pipeline stages -// (build, merge, conclude). All three topics carry a BatchID payload, so this +// (build, land, conclude). All three topics carry a BatchID payload, so this // controller is registered three times — one per topic, each with the matching // DLQ topic key and consumer group. // @@ -39,7 +39,7 @@ import ( // // Blaming the batch on the message is right for these stages because their // work is that batch: whatever failed, it failed doing this batch's build, -// merge, or conclusion. The speculate stage is not like that — it re-plans a +// land, or conclusion. The speculate stage is not like that — it re-plans a // whole queue from a message that names one batch — so it has its own // reconciler; see speculate.go. type batchController struct { diff --git a/submitqueue/orchestrator/controller/dlq/dlq.go b/submitqueue/orchestrator/controller/dlq/dlq.go index ee9c6b387..3d99e4d1c 100644 --- a/submitqueue/orchestrator/controller/dlq/dlq.go +++ b/submitqueue/orchestrator/controller/dlq/dlq.go @@ -28,7 +28,7 @@ // new `{topic}_dlq` name). The DLQ controllers decode that payload to recover // the affected request or batch, then transition it to a terminal failed // state — Error for requests, Failed for batches — with an idempotent -// optimistic-locking write so concurrent activity (a late merge, a cancel +// optimistic-locking write so concurrent activity (a late land, a cancel // race) wins cleanly. Batch failures also fan out to the member requests so // the gateway no longer reports them as in-progress. package dlq diff --git a/submitqueue/orchestrator/controller/dlq/speculate.go b/submitqueue/orchestrator/controller/dlq/speculate.go index 7e6fe5876..9d6403a32 100644 --- a/submitqueue/orchestrator/controller/dlq/speculate.go +++ b/submitqueue/orchestrator/controller/dlq/speculate.go @@ -42,7 +42,7 @@ import ( // So this reconciler reads the failure's subjects and acts on those. It also // republishes to speculate afterwards, because a dead letter here consumes an // edge the queue needed. Speculation is driven only by messages; a batch -// admitted to Speculating produces no build to signal and no merge to conclude, +// admitted to Speculating produces no build to signal and no land to conclude, // so once the message that would have funded it is gone, nothing is left to // look at it again. Without the republish the failure of one batch silently // strands every other batch in the queue. diff --git a/submitqueue/orchestrator/controller/speculate/check.go b/submitqueue/orchestrator/controller/speculate/check.go index 14528c913..3e0607ba4 100644 --- a/submitqueue/orchestrator/controller/speculate/check.go +++ b/submitqueue/orchestrator/controller/speculate/check.go @@ -47,10 +47,10 @@ const ( // each drop. // // The Speculator is an extension, so its output is untrusted input: it decides -// which paths run, never whether a batch merges or fails. Every rule here +// which paths run, never whether a batch lands or fails. Every rule here // protects an invariant the extension could otherwise break — acting on a batch // that is finalizing, resurrecting a path a resolved dependency has ruled out, -// or discarding a passed build the queue is about to merge on. A proposal that +// or discarding a passed build the queue is about to use for landing. A proposal that // trips one of these is a bug in the Speculator, not a normal outcome, which is // why the caller counts them. func filterProposals(proposals []entity.Speculation, snap snapshot) ([]entity.Speculation, []rejection) { @@ -130,9 +130,9 @@ func rejectionReason(proposal entity.Speculation, snap snapshot) (rejection, boo // combination its assumptions name. With the length check and position-wise // equality, a missing, extra, or duplicate dependency is also impossible. // -// A malformed path is not merely suboptimal, it is unmergeable — the merge -// preconditions are read off the path's assumptions (see mergeablePath), so a -// path missing a dependency would let its head merge without waiting for it. +// A malformed path is not merely suboptimal, it is unlandable — the land +// preconditions are read off the path's assumptions (see landablePath), so a +// path missing a dependency would let its head land without waiting for it. func isWellFormed(path entity.SpeculationPath, head entity.Batch) bool { if path.Head != head.ID { return false diff --git a/submitqueue/orchestrator/controller/speculate/dispatch.go b/submitqueue/orchestrator/controller/speculate/dispatch.go index c489ace90..5e3883f87 100644 --- a/submitqueue/orchestrator/controller/speculate/dispatch.go +++ b/submitqueue/orchestrator/controller/speculate/dispatch.go @@ -34,7 +34,7 @@ import ( // // It walks every in-flight batch, not only the speculating ones. Proposals // apply to speculating heads alone and are simply absent for the rest, but -// observations are not: a merging or cancelling head's paths keep holding CI +// observations are not: a landing or cancelling head's paths keep holding CI // slots until their builds stop, and this is the only writer that can record // that they have. Batches already finalized arrive here clean — // commitOutcome persisted their set with their outcome — so only their diff --git a/submitqueue/orchestrator/controller/speculate/doc.go b/submitqueue/orchestrator/controller/speculate/doc.go index a8927cdbb..79ec04ee7 100644 --- a/submitqueue/orchestrator/controller/speculate/doc.go +++ b/submitqueue/orchestrator/controller/speculate/doc.go @@ -23,8 +23,8 @@ // Batches in a queue depend on the batches ahead of them, so without // speculation everything is serial: C waits for B, B waits for A. Speculation // builds a batch against a guess about how its dependencies turn out. When -// the guess holds, the batch merges the moment the guessed-on dependencies -// land. If passed paths cover every possible outcome, the batch can merge +// the guess holds, the batch lands the moment the guessed-on dependencies +// land. If passed paths cover every possible outcome, the batch can land // before those dependencies settle. // // # Paths @@ -48,11 +48,11 @@ // Fund both and every future is covered: // // - While A is still unresolved, both P1 and P2 passing lets B bypass A and -// merge immediately: either possible future has already been validated. -// - A succeeds and P1 passed: B merges the moment A lands. P2's guess +// land immediately: either possible future has already been validated. +// - A succeeds and P1 passed: B lands the moment A lands. P2's guess // ("A fails") is broken — it can no longer come true — so its build is // cancelled to free the slot. -// - A fails and P2 passed: B merges without A, again with no new build. +// - A fails and P2 passed: B lands without A, again with no new build. // P1's guess is broken. // - A resolved either way, and every unbroken path failed: no future // remains in which B passes, so B fails. @@ -79,7 +79,7 @@ // until its build stops. A path is broken once a dependency's actual result // proves one of its assumptions wrong: its guess can no longer come true, so // its build is cancelled to free the slot. A path is superseded when its head -// becomes mergeable, so any still-running siblings are cancelled too. +// becomes landable, so any still-running siblings are cancelled too. // // Cancelling is intent, not fact: the build keeps its slot until CI actually // stops it, and only an observation of that stop (or proof nothing was ever @@ -91,7 +91,7 @@ // // # The life of a batch, as seen from here // -// Created ──admit──► Speculating ──┬── merge or bypass ──► Merging +// Created ──admit──► Speculating ──┬── land or bypass ──► Landing // └── fail ─────────────► Failed // user cancel (cancel stage): // ... ──► Cancelling ── every path stopped ──► Cancelled diff --git a/submitqueue/orchestrator/controller/speculate/run.go b/submitqueue/orchestrator/controller/speculate/run.go index afc642cce..dba882c63 100644 --- a/submitqueue/orchestrator/controller/speculate/run.go +++ b/submitqueue/orchestrator/controller/speculate/run.go @@ -49,7 +49,7 @@ func (c *Controller) run(ctx context.Context, store storage.Storage, trigger ent if len(snap.speculating) == 0 { // No head is open to new work, so there is nothing to ask the // Speculator. The dispatch step still runs: what the build stages saw about a - // merging or cancelling head's paths has to be persisted so those + // landing or cancelling head's paths has to be persisted so those // paths stop counting against the budget. return c.dispatch(ctx, trigger.Queue, snap, nil) } @@ -292,7 +292,7 @@ func terminalPathStatus(status entity.BuildStatus) entity.SpeculationPathStatus // head's dependencies are the facts its paths are built from, so a dependency // withheld is one the Speculator has to plan around blind. Every in-flight // path set goes over for the same reason: a path holds its CI slot until its -// build actually stops, so a merging head's superseded siblings and a +// build actually stops, so a landing head's superseded siblings and a // cancelling head's live builds spend the budget just like a speculating // head's do. Hiding either would let the allocator count occupied slots as // free and oversubscribe CI. @@ -301,7 +301,7 @@ func terminalPathStatus(status entity.BuildStatus) entity.SpeculationPathStatus // head, and check rejects any proposal aimed at a head that is not // speculating. A head this run has just decided still reads as Speculating // here, but it can only rebuild the path that already passed — see -// mergeablePath — which the allocator skips as finished. +// landablePath — which the allocator skips as finished. func (c *Controller) ask(ctx context.Context, queue string, snap snapshot) ([]entity.Speculation, error) { spec, err := c.speculators.For(speculator.Config{QueueName: queue}) if err != nil { diff --git a/submitqueue/orchestrator/controller/speculate/snapshot.go b/submitqueue/orchestrator/controller/speculate/snapshot.go index 2e43b3892..a1677d12d 100644 --- a/submitqueue/orchestrator/controller/speculate/snapshot.go +++ b/submitqueue/orchestrator/controller/speculate/snapshot.go @@ -33,7 +33,7 @@ type snapshot struct { // of one of them. batches map[string]entity.Batch // inFlight is the queue's in-flight batches in queue order, whatever their - // state. This is what the dispatch step walks: a merging or cancelling head + // state. This is what the dispatch step walks: a landing or cancelling head // is closed to new work, but its paths still hold CI slots and their // observations still need persisting. inFlight []entity.Batch