From 922cb266c504206751f1b435cc34cfe095520368 Mon Sep 17 00:00:00 2001 From: Albert Wu Date: Tue, 21 Jul 2026 11:15:37 -0700 Subject: [PATCH] docs(rfc): conclude request failures Summary: Broaden the existing conclude topic to accept structured request failures from business-decision controllers and DLQs. Document the tagged payload contract, producer behavior, replay semantics, and rollout. Test Plan: - make lint - make check-tidy - make check-gazelle --- doc/rfc/index.md | 1 + .../submitqueue/conclude-request-failures.md | 129 ++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 doc/rfc/submitqueue/conclude-request-failures.md diff --git a/doc/rfc/index.md b/doc/rfc/index.md index 3f2a9af2..acbfdcee 100644 --- a/doc/rfc/index.md +++ b/doc/rfc/index.md @@ -15,6 +15,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting - [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 +- [Conclude Request Failures](submitqueue/conclude-request-failures.md) - Broaden the existing conclude topic to finalize structured business rejections and permanent controller failures - [Gateway Status and List APIs](submitqueue/status-list-api.md) - Gateway-owned request context, materialized current status, sqid or change-URI status lookup, and queue admission listing - [Speculation](submitqueue/speculation.md) - Why SubmitQueue speculates, the path/tree model, and the two pluggable seams: speculation-tree enumeration and path selection - [Modular Queue Wiring](submitqueue/modular-queue-wiring.md) - Declare-don't-assemble engine (`pipeline.Construct`) that unifies topic registry, controller registration, DLQ pairing, and lifecycle ordering into one typed call; services self-declare via Deps struct + Stages slice, hosts own per-queue profiles and transport diff --git a/doc/rfc/submitqueue/conclude-request-failures.md b/doc/rfc/submitqueue/conclude-request-failures.md new file mode 100644 index 00000000..ca8cbc41 --- /dev/null +++ b/doc/rfc/submitqueue/conclude-request-failures.md @@ -0,0 +1,129 @@ +# Conclude Request Failures + +Broaden the existing `conclude` stage so controllers and DLQs can send it structured request failures. Do not add a separate `request-failure` topic. + +## Current versus proposed + +Current `conclude` consumes a `BatchID`, loads the terminal batch, updates each member request, and publishes its terminal request log. + +The proposal keeps that path and adds `RequestFailureID` as a second tagged input: + +| | Current | Proposed | +|---|---|---| +| Input | `BatchID` | `BatchID` or `RequestFailureID` | +| Source | Terminal batch | Terminal batch, business rejection, or DLQ | +| Context | Batch state and ID | Structured stage, code, message, and metadata | +| Failure ownership | Controllers and DLQs may terminalize directly | They delegate terminalization and logging to `conclude` | + +The finalization algorithm is not new. The change is a broader input contract and durable failure context. + +``` +terminal batch ────────────────────────────────> conclude { batch_id } + +known business rejection ─> RequestFailure ────> conclude { failure_id } + +permanent controller error ─> DLQ ─> RequestFailure ─> conclude { failure_id } +``` + +Both business-decision controllers and DLQs publish to `conclude`. A controller publishes only after reaching a known terminal decision. Execution and infrastructure errors continue to return as errors while recovery remains possible; the DLQ publishes only after they become permanent. + +## Contracts + +`ConcludeMessage` is an internal protobuf with a tagged `oneof`: + +```proto +message ConcludeMessage { + oneof target { + string batch_id = 1; + string request_failure_id = 2; + } +} +``` + +This avoids mixing bare `BatchID` and `RequestFailureID` payloads on one topic. + +`RequestFailure` is an immutable orchestrator entity containing a stable ID, request ID, stage, kind (`business_rejection` or `controller_failure`), stable code, message, metadata, and creation time. Producers create it idempotently; the queue carries only its ID. + +## Controller change + +Today a controller such as `mergeconflictsignal` terminalizes a business rejection directly: + +```go +if result.Outcome != runwaypb.Outcome_SUCCEEDED { + return c.failRequest(ctx, request, result.Reason) +} +``` + +Under this proposal it persists the decision and delegates finalization: + +```go +if result.Outcome != runwaypb.Outcome_SUCCEEDED { + failure := entity.NewRequestFailure( + request.ID, + "mergeconflictsignal", + entity.FailureKindBusinessRejection, + "mergeability_check_failed", + result.Reason, + metadata, + ) + if err := failureStore.Create(ctx, failure); err != nil { + return fmt.Errorf("create request failure: %w", err) + } + return publishConcludeFailure(ctx, failure.ID, request.Queue) +} +``` + +The controller no longer changes `Request.State` or publishes a terminal request log. + +## DLQ change + +Today a request-scoped DLQ changes the request to `RequestStateError` and publishes a log using `dlq.last_error`. + +Under this proposal it creates a generic durable failure and publishes it to `conclude`: + +```go +failure := entity.NewRequestFailure( + requestID, + originalTopic, + entity.FailureKindControllerFailure, + "controller_failure", + delivery.Metadata()["dlq.last_error"], + nil, +) +if err := failureStore.Create(ctx, failure); err != nil { + return err +} +return publishConcludeFailure(ctx, failure.ID, partitionKey) +``` + +A previously persisted business failure wins over a later generic DLQ failure for the same logical transition. A batch-scoped DLQ still marks the batch `BatchStateFailed`, then publishes one failure conclusion per member to preserve the original error context. + +## Broadened conclude + +`conclude` dispatches on the tagged reference: + +```go +switch target := msg.Target.(type) { +case *ConcludeMessage_BatchId: + return c.concludeBatch(ctx, target.BatchId) +case *ConcludeMessage_RequestFailureId: + return c.concludeFailure(ctx, target.RequestFailureId) +default: + return errors.New("conclude message has no target") +} +``` + +- `concludeBatch` retains the current batch fan-out behavior. +- `concludeFailure` loads the failure and request, conditionally changes the request to `RequestStateError`, and publishes `RequestStatusError` with the structured context. +- Both paths use the same request reconciliation and log-repair helper. +- Gateway remains the sole writer of the request-log database. + +## Replay and rollout + +- Failure creation and conclusion publication use stable identities. +- If state update succeeds but log publication fails, redelivery republishes the log. +- Version conflicts return errors so redelivery reloads current state. +- A different terminal request outcome is left unchanged. +- `conclude_dlq` replays a referenced failure instead of creating another one. + +To preserve in-flight messages, first deploy a decoder that accepts both legacy bare `BatchID` payloads and `ConcludeMessage`. Then migrate batch producers, add failure producers, and remove legacy decoding after the topic retention window.