Repository navigation
docs: make terminal-only DLQ policy explicit - #1075
Open
sbalabanov wants to merge 2 commits into
Open
sbalabanov wants to merge 2 commits into
sbalabanov wants to merge 2 commits into
Conversation
Summary: Keep final DLQ retries focused on terminal reconciliation, not restarting failed workflows. Intent: Conflicting retry and partial-transition guidance could lead an implementer to reuse an idempotent Start controller on a DLQ, creating Started and publishing validation again. Make that boundary explicit without changing runtime behavior. Changes: - Add the terminal-only rule and explicit maintainer-approval requirement to Design Defaults. - Correct the existing DLQ guide and link workflow wiring to its source, preserving the named narrow exceptions. - Narrow Stovepipe request-log repair wording to terminal/history obligations. Test Plan: Checked wording against current stage bindings and terminal handlers; all 30 local Markdown links resolve and git diff --check passes. make gazelle completed without BUILD changes. No runtime tests were run for this documentation-only change. Revert Plan: Revert this documentation commit.
|
|
sbalabanov
commented
Nov 12, 2026
| **The DLQ is the final destination.** A message in `{topic}_dlq` has already failed the primary controller's retry budget (`Retry.MaxAttempts` for a retryable error) or surfaced as non-retryable on the first attempt. There is no further dead-letter beyond this point — every DLQ subscription is configured with `DLQ.Enabled = false`, so a DLQ controller's outcome is either "reconciled" or "the message keeps coming back". A genuinely unprocessable DLQ message (e.g. a malformed payload that no controller can ever decode) must be removed by an operator from the queue manually. | ||
| **Terminal reconciliation, not workflow recovery.** Idempotent primary controllers own normal processing under their primary retry policy. A DLQ handler must not resume a partially completed normal workflow or repeat its business action. Repairing state, membership, and status/log outputs needed to express a justified terminal outcome is reconciliation. | ||
|
|
||
| **The implementation is deliberately the simplest and most reliable thing that can work.** Each controller does the same three steps: decode the payload to recover the affected `RequestID` or `BatchID`, fetch the entity, transition it to its terminal failed state with the same optimistic-locking CAS the primary pipeline uses. No business logic, no branching on the original error, no per-stage cleanup. Adding behaviour here trades off the one property the DLQ has to provide — convergence. |
Contributor
Author
There was a problem hiding this comment.
keep wording on keeping the implementation simplest
Contributor
Author
There was a problem hiding this comment.
Restored the simplest-and-most-reliable implementation wording in a4c10e6, while keeping the distinction between terminal bookkeeping repair and restarting normal work.
[addressed by agent]
|
|
||
| Reviews send changes back for these more often than for anything else. Get them right in the first draft. | ||
|
|
||
| - **DLQ is terminal reconciliation, not a second workflow runner.** Idempotent primary controllers own normal processing under their primary retry policy. Unlimited final retries protect reconciliation writes and permitted terminal outputs, not another business retry budget. Do not bind a normal orchestration controller to a DLQ, create a missing request in Started, or publish validate/build/merge work to resume the dead-lettered request, even with its full original payload. Preserve durable terminal winners and valid downstream ownership. Terminal membership/log repair and gateway read-model replay are not orchestration restart. Follow the named limits in [DLQ reconciliation](submitqueue/orchestrator/controller/dlq/README.md); a new exception requires explicit owning-maintainer approval before implementation. Implementer-authored exception prose or another domain's handler is not authorization. |
Contributor
Author
There was a problem hiding this comment.
remove "Unlimited final retries" sentence
Contributor
Author
There was a problem hiding this comment.
also remove this: "Implementer-authored exception prose or another domain's handler is not authorization."
as long as user explicitly wants it, the agent can do, but it should not be what agent can choose on their own.
Contributor
Author
There was a problem hiding this comment.
Removed both requested sentences from Design Defaults in a4c10e6. The guide now explicitly allows a recovery exception when the user requests it or the owning maintainer approves it, while saying agents must not choose recovery exceptions on their own. The README uses the same authorization wording.
[addressed by agent]
sbalabanov
marked this pull request as ready for review
November 12, 2026 21:47
Summary: Keep terminal-only DLQ reconciliation as the default while requiring an explicit user request or owning-maintainer approval for recovery exceptions. Changes: - Restore the simplest-and-most-reliable implementation principle. - Remove the requested top-level retry and implementer-authored authorization sentences. - Clarify that agents must not choose recovery exceptions on their own. Test Plan: Read both complete review threads and checked the requested wording changes. git diff --check passes and all 17 local Markdown links in the two changed files resolve. make gazelle completed without BUILD changes. No runtime tests were run for this documentation-only change. Revert Plan: Revert this documentation commit.
prathshenoy
approved these changes
Nov 12, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Keep final DLQ retries focused on terminal reconciliation, not restarting failed workflows.
Summary
The existing retry guidance did not state the business-effect limit, while the DLQ guide incorrectly prohibited even terminal bookkeeping repair. An implementer could read those contradictions as permission to reuse an idempotent Start controller on the DLQ, creating Started and publishing validation again.
Documentation only; no queue bindings or runtime behavior change.
Test Plan
Checked the wording against the current stage bindings and terminal handlers, including owner/winner preservation, terminal fan-out, speculation's other-work nudge, hook disposition, and Stovepipe record notifications. All 30 local Markdown links in the four changed documents resolve, and
git diff --checkpasses.make gazellecompleted without BUILD changes. No runtime tests were run for this wording-only change.Revert Plan
Revert this documentation commit.