Skip to content

fix(speculate): blame the batch that failed, and stop the queue stranding - #563

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/codem-428-speculate-attribution
Aug 11, 2026
Merged

fix(speculate): blame the batch that failed, and stop the queue stranding#563
behinddwalls merged 1 commit into
mainfrom
preetam/codem-428-speculate-attribution

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

Two things go wrong when a speculate message dead-letters, and together they leave a queue quietly stuck.

The first is misattribution. The speculate stage takes a message naming one batch but does a job covering the whole queue: a run lists every in-flight batch, hands them all to the Speculator, and commits outcomes for any of them. Only the handful of errors before the run even starts are about the batch on the message. Everything after is about some other batch, or about the queue itself. Yet the stage shared dlq.NewDLQBatchController with build, merge, and conclude — which marks the named batch Failed and errors all its requests. So a Speculator error, or a storage failure writing another head's path set, terminated a batch that was never at fault while the real culprit carried on.

The second is that the reconcile restored nothing. failBatch published no message at all. Speculation is driven only by messages, and a batch admitted to Speculating produces no build to signal and no merge to conclude — so once the message that would have funded it is gone, nothing is left to look at it again. The dead letter consumed the queue's last edge and left none behind, stranding every other admitted batch with no error recorded against it and its requests still reading batched. From the outside the queue looked like it was working.

What?

Speculate now says what its failures are about, and its dead letters act on that.

Every error return is attributed with errs.Attribute: the message's batch for the errors raised before the run, the specific batch for errors raised while looping over the queue, and {Type: queue} for listing the queue and for both Speculator calls, where no batch is at fault. Attribution is added through entity.BatchSubject / entity.QueueSubject, and a counter tagged by subject type makes queue-scoped failures graphable. Retryability is untouched — the classifiers still read the cause underneath.

The stage gets its own reconciler, dlq/speculate.go, instead of the shared batch one:

  • Batch subjects are taken at their word and those batches are failed, which may not be the batch on the message.
  • A queue subject, or no subjects at all, falls back to the message's batch — a guess, but DLQ reconciliation exists so requests cannot sit non-terminal forever, and that guarantee has to hold even when nothing can say which batch was at fault. Which of the three happened is recorded as dlq.attribution, so a fallback is never mistaken for a confident answer.
  • Afterwards it republishes one speculate message naming a still-live batch, restoring the edge it consumed. Two guards make it terminate: it runs only after a reconcile that actually transitioned a batch, so a redelivery cannot loop, and it names a live batch rather than the one just failed, so each pass fails one more. A genuinely broken queue therefore drains to empty with a reason recorded against every batch, bounded by the batch count, while a queue whose failure was transient or queue-wide recovers on the next run having lost one batch instead of stranding all of them.

Separately, the shared failRequest passed nil where RequestLog.Metadata goes, discarding the failure count, originating topic, and timestamp the reconciler already had in hand and logged. It now carries them, along with the failure's subjects and detail. No schema or proto change is needed: RequestLog.Metadata is already persisted and already exposed on the gateway's status and history, so this lands in front of users directly. All five reconcilers benefit.

Test Plan

bazel test //...
bazel test //test/... --sandbox_writable_path=$HOME/.docker --jobs=1 (11 container suites)
make lint check-tidy check-gazelle check-mocks

New coverage in dlq/speculate_test.go:

  • Attribution as a table: a batch subject fails that batch rather than the message's; a queue subject and a subject-less failure each fall back, and each records which it was.
  • The re-trigger publishes for a queue that still holds live batches, choosing deterministically (ListByStates promises no order).
  • The loop guard: an already-failed batch publishes nothing even with live batches remaining, which is what stops a permanently failing queue from re-triggering forever.

Issue

Closes CODEM-428

Issues

@behinddwalls
behinddwalls force-pushed the preetam/codem-428-speculate-attribution branch from 55d4aa4 to 5e6d9ab Compare August 11, 2026 18:36
@behinddwalls
behinddwalls marked this pull request as ready for review August 11, 2026 18:36
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 11, 2026 18:37
@behinddwalls
behinddwalls force-pushed the preetam/codem-428-speculate-attribution branch from 5e6d9ab to 55d4aa4 Compare August 11, 2026 21:06
Base automatically changed from preetam/codem-428-failure-envelope to main August 11, 2026 22:39
…ding

## Summary

### Why?

Two things go wrong when a speculate message dead-letters, and together they leave a queue quietly stuck.

The first is misattribution. The speculate stage takes a message naming one batch but does a job covering the whole queue: a run lists every in-flight batch, hands them all to the Speculator, and commits outcomes for any of them. Only the handful of errors before the run even starts are about the batch on the message. Everything after is about some other batch, or about the queue itself. Yet the stage shared `dlq.NewDLQBatchController` with build, merge, and conclude — which marks *the named* batch `Failed` and errors all its requests. So a Speculator error, or a storage failure writing another head's path set, terminated a batch that was never at fault while the real culprit carried on.

The second is that the reconcile restored nothing. `failBatch` published no message at all. Speculation is driven only by messages, and a batch admitted to `Speculating` produces no build to signal and no merge to conclude — so once the message that would have funded it is gone, nothing is left to look at it again. The dead letter consumed the queue's last edge and left none behind, stranding every other admitted batch with no error recorded against it and its requests still reading `batched`. From the outside the queue looked like it was working.

### What?

Speculate now says what its failures are about, and its dead letters act on that.

Every error return is attributed with `errs.Attribute`: the message's batch for the errors raised before the run, the specific batch for errors raised while looping over the queue, and `{Type: queue}` for listing the queue and for both Speculator calls, where no batch is at fault. Attribution is added through `entity.BatchSubject` / `entity.QueueSubject`, and a counter tagged by subject type makes queue-scoped failures graphable. Retryability is untouched — the classifiers still read the cause underneath.

The stage gets its own reconciler, `dlq/speculate.go`, instead of the shared batch one:

- Batch subjects are taken at their word and those batches are failed, which may not be the batch on the message.
- A queue subject, or no subjects at all, falls back to the message's batch — a guess, but DLQ reconciliation exists so requests cannot sit non-terminal forever, and that guarantee has to hold even when nothing can say which batch was at fault. Which of the three happened is recorded as `dlq.attribution`, so a fallback is never mistaken for a confident answer.
- Afterwards it republishes one speculate message naming a still-live batch, restoring the edge it consumed. Two guards make it terminate: it runs only after a reconcile that actually transitioned a batch, so a redelivery cannot loop, and it names a *live* batch rather than the one just failed, so each pass fails one more. A genuinely broken queue therefore drains to empty with a reason recorded against every batch, bounded by the batch count, while a queue whose failure was transient or queue-wide recovers on the next run having lost one batch instead of stranding all of them.

Separately, the shared `failRequest` passed `nil` where `RequestLog.Metadata` goes, discarding the failure count, originating topic, and timestamp the reconciler already had in hand and logged. It now carries them, along with the failure's subjects and detail. No schema or proto change is needed: `RequestLog.Metadata` is already persisted and already exposed on the gateway's status and history, so this lands in front of users directly. All five reconcilers benefit.

## Test Plan

✅ `bazel test //...`
✅ `bazel test //test/... --sandbox_writable_path=$HOME/.docker --jobs=1` (11 container suites)
✅ `make lint check-tidy check-gazelle check-mocks`

New coverage in `dlq/speculate_test.go`:

- Attribution as a table: a batch subject fails *that* batch rather than the message's; a queue subject and a subject-less failure each fall back, and each records which it was.
- The re-trigger publishes for a queue that still holds live batches, choosing deterministically (`ListByStates` promises no order).
- The loop guard: an already-failed batch publishes nothing even with live batches remaining, which is what stops a permanently failing queue from re-triggering forever.

## Issue

Closes CODEM-428
@behinddwalls
behinddwalls force-pushed the preetam/codem-428-speculate-attribution branch from 55d4aa4 to 5240c41 Compare August 11, 2026 22:39
@behinddwalls
behinddwalls added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit 581118c Aug 11, 2026
15 checks passed
@behinddwalls
behinddwalls deployed to stack-rebase August 11, 2026 22:52 — with GitHub Actions Active
@behinddwalls
behinddwalls deleted the preetam/codem-428-speculate-attribution branch August 11, 2026 22:52
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.

2 participants