Repository navigation
fix(stovepipe): persist the terminal outcome reason on Request - #965
Open
sbalabanov wants to merge 1 commit into
Open
sbalabanov wants to merge 1 commit into
sbalabanov wants to merge 1 commit into
Conversation
Request stored its terminal state but not the reason, so buildsignal relabeled every failed request build_failed while the DLQ reconcilers logged their own stage reason. Two writers of one state/<version> log disagreed, the materializer rejected the second, and the DLQ retried that rejection forever. Store OutcomeReason on Request in the same compare-and-swap as the terminal state (buildsignal, the DLQ reconcilers, process supersede), and build every state log from the stored reason. Only rows made terminal before this change fall back to the writing stage's reason. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Why?
Requestpersisted its terminal state but not the reason, so each writer of astate/<version>log reconstructed the reason itself: buildsignal mappedfailedtobuild_failed, while the DLQ reconcilers used their own stage reason. The materializer correctly rejects the second, disagreeing writer, and DLQ consumers retry without an attempt cap; in the opposite order a late poll mislabels a forced infrastructure failure asbuild_failed. Repro:failedand retainsstate/3withbuild_failed.failedand re-logsstate/3withbuild_polling_exhausted.PersistLogrejects the conflict, and the DLQ retries it forever.What?
entity.RequestgainsOutcomeReason, stored in the MySQLrequest.outcome_reasoncolumn. Every writer sets it in the same compare-and-swap that writes the terminal state: buildsignalmarkOutcome(build-derived reason), the DLQfailRequest(its stage reason), and processsupersedeRequest.requestlog.NewRequestStateLognow records the request's stored reason; the caller-supplied reason is only a fallback for nonterminal requests and for rows made terminal before this change. So buildsignal and the DLQ log the same reason for one state version and no longer conflict. The materializer's conflict check is unchanged. Existing deployments must add the column before deploying (see the storage README).Test Plan
make fmt && make lint && make check-tidy && make check-gazelle && make check-mocks,./tool/bazel test //stovepipe/... //service/stovepipe/... --test_output=errors, and./tool/bazel test //test/integration/stovepipe/extension/storage/mysql:go_default_test(Docker).Audit: SP-OCT-001 in doc/audit/stovepipe-fable-v2.md (Sol v2 #4, Opus v2 #2).
🤖 Generated with Claude Code