Skip to content

fix(stovepipe): fail validations that exceed max_validation_ms - #967

Open
sbalabanov wants to merge 1 commit into
sergeyb/sp-oct-001-terminal-reasonfrom
sergeyb/sp-oct-002-validation-deadline
Open

sbalabanov wants to merge 1 commit into
sergeyb/sp-oct-001-terminal-reasonfrom
sergeyb/sp-oct-002-validation-deadline

Conversation

@sbalabanov

@sbalabanov sbalabanov commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Why?

buildsignal holds a nonterminal build on every poll, and holds do not consume the retry budget, so a runner that keeps answering accepted or running is re-polled forever. Nothing dead-letters and nothing releases the build slot. Repro:

  1. Run a queue with MaxConcurrent 1.
  2. Its build's runner reports running on every poll, so the buildsignal message holds and redelivers indefinitely.
  3. A newer head is accepted and waits at the process gate behind it.
  4. Nothing ever fails the build, so the queue's only slot stays held forever.

What?

Build gains CreatedAt, which build sets at creation and MySQL persists in a new build.created_at column. QueueConfig gains MaxValidationMs (zero disables the deadline). The reference default store sets it to two hours. When a poll leaves the build nonterminal past the deadline, buildsignal runs the same terminal sequence as a polled failure and marks the request failed with reason validation_timeout (stored on the request by #965). The timed-out build established nothing: it keeps its last polled status, the request's TerminalBuildID stays empty, and there is no build_finished entry or terminal analysis publish. Existing MySQL deployments must add created_at BIGINT NOT NULL DEFAULT 0 to build first.

Convergence: a retry after any failure in the timeout sequence polls again, finds the build still nonterminal and still past the deadline, and takes the same path with the same reason; finishRequest is a no-op once the outcome landed. If the build reached a real terminal status in the meantime and the request has no outcome yet, that real verdict is recorded instead. Nothing lists nonterminal builds and the request is terminal, so nothing reopens.

Test Plan

make fmt && make lint && make check-tidy && make check-gazelle && make check-mocks
./tool/bazel test //stovepipe/... //service/stovepipe/... --test_output=errors && ./tool/bazel test //test/integration/stovepipe/extension/storage/mysql:go_default_test --test_output=errors

Stack

  1. fix(stovepipe): persist the terminal outcome reason on Request #965
  2. @ this PR

Audit: SP-OCT-002 in doc/audit/stovepipe-fable-v2.md (Sol v2 #12, Opus v2 #4).

🤖 Generated with Claude Code

@sbalabanov
sbalabanov force-pushed the sergeyb/sp-oct-002-validation-deadline branch from 655b844 to cfb56bd Compare October 27, 2026 23:43
buildsignal held a nonterminal build unconditionally, and holds do not
consume the retry budget, so a runner that kept answering accepted or
running was re-polled forever. The request never dead-lettered and, at
MaxConcurrent 1, kept the queue's only build slot.

Record Build.CreatedAt and add QueueConfig.MaxValidationMs (two hours in
the reference config; zero disables it). When a poll leaves the build
nonterminal past the deadline, buildsignal runs the normal terminal path
and marks the request failed with reason validation_timeout. The
timed-out build established nothing: it keeps its polled status, is not
recorded as the terminal build, and gets no build_finished entry or
terminal analysis. A retry re-derives the same timeout from the poll,
and finishRequest is a no-op once the outcome is stored.

Existing MySQL deployments must add created_at to build before deploying.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@sbalabanov
sbalabanov force-pushed the sergeyb/sp-oct-002-validation-deadline branch from cfb56bd to 37e2795 Compare October 27, 2026 23:46
@sbalabanov
sbalabanov marked this pull request as ready for review October 27, 2026 23:47
@sbalabanov

Copy link
Copy Markdown
Contributor Author

Two things for the reviewer, neither blocks CI:

Default value needs a human call. max_validation_ms defaults to two hours in the reference queue config store. Nothing in the repository establishes what a full-repository validation takes, so this number is a guess that bounds a hung build without cutting a long real build short. Zero disables the deadline. If you want a different default, say which and I will update the constant; otherwise it ships as is.

Merge order. This PR is stacked on #965. It also touches finishRequest and its doc comment in buildsignal.go, and the lease paragraph in process.md, which #966 rewrites independently. Whichever of #966 and this stack lands second needs a small rebase; I can do that once the first one merges.

@sbalabanov
sbalabanov requested review from a team and behinddwalls as code owners October 27, 2026 23:47

This branch has not been deployed

No deployments
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