You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 655b844
Browse filesBrowse the repository at this point in the historyBrowse files
fix(stovepipe): fail validations that exceed max_validation_ms
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). A nonterminal poll of a
nonterminal build past the deadline persists the build as failed through
the write-once reconcile and runs the normal terminal path, marking the
request failed with reason validation_timeout. A redelivery finds the
stored terminal build and re-logs the stored reason.
Existing MySQL deployments must add created_at to build before deploying.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- the row carries no scope; it is recoverable from the Request's immutable fields
58
58
(see the entity table).
@@ -301,6 +301,7 @@ Either could be adopted independently: the idempotency token, if a backend that
301
301
|`ID`| The build's own key — the runner-assigned id returned by `Trigger` (a Buildkite build number, a CI-gateway job id); opaque, never parsed or derived | no |
302
302
|`RequestID`| The `Request` this build validates (`Build`→`Request` navigation) | no |
|`CreatedAt`| Unix milliseconds when the row was created; zero on rows created before it was recorded | no |
304
305
|`Version`|`int32` optimistic-locking version |**yes** — with `Status`|
305
306
306
307
The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrategy` — immutable and reachable through `RequestID` — fully determine what a build ran against.
Copy file name to clipboardExpand all lines: doc/rfc/stovepipe/steps/buildsignal.md
+3-1Lines changed: 3 additions & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -51,6 +51,8 @@ For a delivery carrying build id `B`:
51
51
see "Status" below for what it's for and how a future check could use it.
52
52
53
53
6. Reconcile the polled status against the stored row:
54
+
- status and Build.Status both non-terminal, and max_validation_ms elapsed since
55
+
Build.CreatedAt -> reconcile failed in place of status (see Fail-closed interaction).
54
56
- status == Build.Status -> no-op poll: skip the write, continue with the stored status.
55
57
- Build.Status already terminal, status differs -> terminal is WRITE-ONCE: do not overwrite;
56
58
continue with the STORED status as authoritative (see Edge cases).
@@ -165,7 +167,7 @@ The window to guard is between persisting status (step 6) and ack (steps 7–8);
165
167
166
168
A build that never reaches terminal `Status` — runner outage, a build the runner lost — must not wedge its `Request` forever, since callers gate deployments on greenness reaching a recorded terminal state. `buildsignal` does not implement the forcing function: per [workflow.md](../workflow.md#fail-closed-on-unprocessable-work) and the `in_flight_count` slot lifecycle in [process.md](process.md#concurrency-lifecycle), a `Request` stuck at `buildsignal` past `MaxAttempts` dead-letters, and the DLQ reconciler forces a conservative terminal `failed` and releases the Queue's slot. This is the same posture SubmitQueue's build/buildsignal pair relies on: terminal status is what releases the slot and lets validation progress.
167
169
168
-
One boundary of that posture is worth stating: the `MaxAttempts` path fires only when polls *fail*. A runner that keeps answering a healthy non-terminal status forever — a hung build on a backend with no timeout of its own — never errors, so the hold chain (whose redeliveries deliberately do not count toward the retry limit) re-polls indefinitely and nothing dead-letters; SubmitQueue's poll loop shares this property. Bounding it requires a poll deadline — a `max_validation_ms` past which `buildsignal` treats the build as failed and lets the normal terminal path run — which pairs naturally with the lease idea [process.md](process.md#per-queue-concurrency-gate) floats for `in_flight_count`. Deferred with it; until then a too-old non-terminal `Build` is an operational alert, not a self-healing path.
170
+
The `MaxAttempts` path fires only when polls *fail*. A runner that keeps answering a healthy non-terminal status — a hung build on a backend with no timeout of its own — never errors, and hold redeliveries deliberately do not count toward the retry limit, so a deadline bounds that case instead: the per-queue `max_validation_ms` (`QueueConfig.MaxValidationMs`) caps the time from `Build.CreatedAt` to a terminal status. A non-terminal poll of a non-terminal `Build` past the deadline treats the build as `failed`. Step 6 persists `failed` under the write-once rule, so a later poll cannot reopen it and a redelivery resumes from the stored status. Step 7 then runs exactly as for a polled failure, except that the request records the reason `validation_timeout`. Zero disables the deadline, as does a zero `CreatedAt` on a `Build` created before creation times were recorded. The runner's build is not cancelled (see [build.md](build.md#cancellation-defined-not-yet-called)) and may keep running on its backend. SubmitQueue's poll loop has no equivalent deadline.
Copy file name to clipboardExpand all lines: doc/rfc/stovepipe/steps/process.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -67,7 +67,7 @@ A slot is held from admit until the build goes terminal (`process → build →
67
67
68
68
**Liveness — a stuck Request wedges the whole Queue.** The slot is shared per-Queue, so a Request admitted but never driven terminal holds the only slot and stalls the whole Queue. The fail-closed path in [workflow.md](../workflow.md#fail-closed-on-unprocessable-work) prevents this: every admitted Request terminates — `build`/`buildsignal` errors retry to `MaxAttempts`, then dead-letter, and the DLQ reconciler forces a conservative terminal `failed`, freeing the slot. Residual risk is operational: a **poison DLQ message** the reconciler can't process loops forever (it runs always-retryable), wedging that Queue until an operator removes it. The gate makes the blast radius the whole Queue, not one Request — monitor and alert on the DLQ.
69
69
70
-
**Future alternative — time-bounded leases.** To self-heal instead of relying on operator cleanup, `in_flight_count` could become a set of `{owner, expires_at}` leases: the gate counts only unexpired leases, terminal transitions drop the owner's lease, and a leaked lease is reclaimed on expiry — bounding any stall to a `max_validation_ms`. A *list* of leases would also generalize to `max_concurrent > 1`. Deferred; `in_flight_count` plus the fail-closed path is enough for MVP.
70
+
**Future alternative — time-bounded leases.** To self-heal instead of relying on operator cleanup, `in_flight_count` could become a set of `{owner, expires_at}` leases: the gate counts only unexpired leases, terminal transitions drop the owner's lease, and a leaked lease is reclaimed on expiry — bounding a stall whose request never reaches a polled build, which `buildsignal`'s `max_validation_ms` deadline cannot (see [buildsignal.md](buildsignal.md#fail-closed-interaction)). A *list* of leases would also generalize to `max_concurrent > 1`. Deferred; `in_flight_count` plus the fail-closed path is enough for MVP.
71
71
72
72
`max_concurrent` is a scalar, and `= 1` is an MVP simplicity choice, **not** a correctness requirement — see [Raising `max_concurrent`](#raising-max_concurrent-speculative-validation).
0 commit comments