Skip to content

refactor(orchestrator): centralize request termination#424

Draft
albertywu wants to merge 1 commit into
mainfrom
wua/refactor-request-termination
Draft

refactor(orchestrator): centralize request termination#424
albertywu wants to merge 1 commit into
mainfrom
wua/refactor-request-termination

Conversation

@albertywu

@albertywu albertywu commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Centralizes existing request terminal-state reconciliation without changing the queue topology or pipeline behavior.

Scope: This is a code refactor. It does not change anything core to the architecture, so an RFC is not needed: the same controllers make terminal decisions, the same queues and payloads carry work, and the same request-log path materializes status. This PR only replaces duplicated terminal request CAS and log-publication code with a shared helper.

Before After
Cancel, conclude, and merge-conflict controllers each implemented their own terminal CAS, status mapping, idempotency checks, and request-log publication. Cancel and merge-conflict handling call request.ReconcileTerminalState once for the affected request; conclude calls request.ReconcileTerminalState once for each batch member, passing the selected terminal outcome.
RequestLog already supported map[string]string metadata, but each terminal path constructed its log independently, so metadata propagation was call-site-specific. Each caller passes TerminalOutcome{LastError, Metadata} to request.ReconcileTerminalState, which includes that context in the matching terminal request log.
Fixes to optimistic locking or terminal-state race handling had to be repeated across controllers. request.ReconcileTerminalState performs the terminal CAS, same-state log repair, competing-terminal preservation, version handling, and log publication in one implementation.
Request termination behavior was coupled to each controller's private helper code. Controllers retain ownership of the business decision, then call request.ReconcileTerminalState for the shared state-and-log mechanics in submitqueue/core/request.

Helper call path: In every After case above, request terminal-state reconciliation and the matching log are performed by request.ReconcileTerminalState. Unbatched cancel and merge-conflict handling call it directly; conclude calls it once for each member request. The helper does not update batch state.

This preserves the existing request state-machine and request-log behavior. conclude still consumes BatchID messages, individual requests are still terminated by the controller that owns the decision, and DLQ behavior is unchanged in this PR.

ReconcileTerminalState usage

Call site Terminal outcome Purpose
Unbatched cancel Cancelled Finalizes the request and publishes cancellation reason metadata.
Conclude fanout Batch Succeeded, Failed, or Cancelled maps to request Landed, Error, or Cancelled Finalizes each batch member and publishes its batch_id.
Expected merge-conflict failure Error Finalizes a non-mergeable request and publishes the conflict reason as LastError.

Conceptual flow:

ReconcileTerminalState(ctx, requestStore, registry, request, outcome)
    -> (reconciled, error):

    status, err = terminalStatus(outcome.State)
    if err != nil:
        return false, err

    if request.State == outcome.State:
        // Skip CAS and repair the terminal log below.
    else if request.State is already terminal:
        // Preserve the competing terminal outcome.
        return false, nil
    else:
        newVersion = request.Version + 1
        err = requestStore.UpdateState(
            ctx,
            request.ID,
            request.Version,
            newVersion,
            outcome.State,
        )
        if err != nil:
            // Wrapped errors retain their identity.
            // errors.Is(err, storage.ErrVersionMismatch) remains true.
            return false, wrap(err)

        request.Version = newVersion

    log = NewRequestLog(
        request.ID,
        status,
        request.Version,
        outcome.LastError,
        outcome.Metadata,
    )
    err = PublishLog(ctx, registry, log, request.ID)
    if err != nil:
        return false, wrap(err)

    return true, nil

Future callers should use this helper only after deciding that a request must enter a terminal state and needs the matching public request log. The caller owns the business decision and supplies State, LastError, and Metadata; the helper owns the CAS, version advancement, state-to-status mapping, idempotency, and log publication. Do not use it for non-terminal request transitions, batch transitions, or log-only events.

Test Plan

make lint && make check-tidy && make check-gazelle && make test

Revert Plan

Revert this PR to restore per-controller request termination logic.

Stack

  1. @ refactor(orchestrator): centralize request termination #424
  2. fix(orchestrator): preserve conclude DLQ outcomes #426

@albertywu
albertywu force-pushed the wua/refactor-request-termination branch from 53c4a82 to e684e2d Compare July 22, 2026 18:27
Summary:
Intent:
- Remove duplicated terminal request state and log publication logic.
- Keep existing controller behavior while standardizing CAS and idempotency handling.

Changes:
- Add a shared request terminal-state reconciliation helper with metadata support.
- Migrate cancel, conclude, and merge-conflict failure paths to the helper.
- Preserve declaration-level retryability for storage version conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant