Skip to content

feat(stovepipe): Wire the project analysis stage #1065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
prathshenoy wants to merge 1 commit into from
+1,089 −858
Draft

feat(stovepipe): Wire the project analysis stage #1065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions doc/rfc/stovepipe/steps/buildsignal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ It handles only the poll loop: it does not decide build strategy, write greennes

`buildsignal` consumes a build id, published by `build` — once per phase, since Phase 1 (whole-repo) and Phase 2 (per-project) each run their own `build` → `buildsignal` cycle against builds tied to the same `Request` (see [workflow.md](../workflow.md#workflow)).

Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and publishes the request id onward to `record`. What differs between phases is what `record` does with that publish (whole-repo vs. per-project greenness) — not anything `buildsignal` decides.
Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and queues terminal build analysis. Cancelled outcomes also go directly to `record`. What differs between phases is what `record` does with that publish (whole-repo vs. per-project greenness) — not anything `buildsignal` decides.

`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once, at the terminal transition, to record the build's outcome — the one `Request.State` write outside `process` and the DLQ reconciler.

Its early-exit guard is deliberately narrower than `State.IsTerminal()`: it proceeds when the request is `processing` **or** already carries a build outcome. The second case matters because a redelivery after the outcome was stamped but before the `record` publish landed must re-publish rather than drop the signal; everything it re-runs is a no-op (the status is unchanged, the outcome is already recorded, the slot is not released twice) and the `record` publish is idempotent.
Its early-exit guard is deliberately narrower than `State.IsTerminal()`: it proceeds when the request is `processing` **or** already carries a build outcome. The second case matters because a redelivery after the outcome was stamped but before the terminal-analysis publish landed must re-publish rather than drop the signal; everything it re-runs is a no-op (the status is unchanged, the outcome is already recorded, the slot is not released twice) and the downstream publish is idempotent.

The states the guard actually rejects are terminal-without-an-outcome — today `superseded` alone, which `process` sets only from `accepted`, so it cannot reach the poll loop at all. The guard is kept rather than dropped because the slot release in step 7a is conditioned on *not* already having an outcome, so a superseded request arriving here would decrement a slot that superseding never claimed.

Expand All @@ -34,7 +34,7 @@ For a delivery carrying build id `B`:
3. If R.State is neither processing nor already an outcome: ack and return.
- a request reaches the poll loop only after process admitted it, so it is processing —
or already carries this build's outcome on a redelivery, which falls through so the
signal is re-published to record instead of dropped.
terminal analysis is re-published instead of dropped.
- the only other terminal-without-an-outcome state is superseded, and process supersedes
solely from accepted, so it cannot occur here. Guarded anyway: step 7a would otherwise
release a build slot the request never claimed, since superseding consumes none.
Expand Down Expand Up @@ -68,14 +68,13 @@ For a delivery carrying build id `B`:
succeeded -> succeeded, failed -> failed, cancelled -> cancelled. First writer wins.
c. If the winning durable R.State is succeeded, advance the last-green bookmark
in a CAS retry loop, even on a terminal redelivery. Preserve a newer bookmark.
Then publish R.ID to the record topic, partitioned by request id; ack, return.
No re-publish to buildsignal.
- record loads the Request directly by this key, derives greenness from its outcome, and
supplies its terminal build id to project-result resolution; it never reaches a Build.
- the message id is the request id, so a redelivery republishing the same terminal signal
dedups into the original message; record is idempotent regardless.
- publish failure -> return raw (non-retryable); the outcome is persisted, operational
republish recovers.
Then publish terminal BuildAnalysis, partitioned by build id. Cancelled requests
also publish R.ID directly to record, partitioned by request id. Ack and return.
- successful or failed outcomes reach record through completed build analysis
and project analysis, not directly from this poll.
- stable handoff IDs make republishing safe.
- publication failures return raw; DLQ recovery repairs the handoff without
polling the runner again.

8. Else hold the delivery for delayMs (postpone: the same message redelivers after the delay):
- delayMs = pollDelay(status): shorter while running, longer while accepted.
Expand Down Expand Up @@ -150,8 +149,8 @@ When the budget does run out the message dead-letters, and the buildsignal DLQ r
Every branch is safe under at-least-once redelivery:

- **Build not found** — non-retryable; storage's read-after-write guarantee means a miss here is a storage defect, not a lag condition to retry through.
- **Status already persisted** — a redelivery re-runs the whole algorithm from step 1, including a redundant `Status` poll (harmless — the runner reports the same thing); step 6 no-ops on the unchanged status, and the delivery proceeds to hold for the next poll (non-terminal) or republish the request id to `record` (terminal, idempotent). No corruption.
- **Terminal already published** — a redelivery reloads, re-polls, no-ops at step 6, republishes the same terminal signal to `record` (idempotent), and acks. Harmless.
- **Status already persisted** — a redelivery re-runs the whole algorithm from step 1, including a redundant `Status` poll (harmless — the runner reports the same thing); step 6 no-ops on the unchanged status, and the delivery proceeds to hold for the next poll (non-terminal) or retry the terminal-analysis handoff (terminal, idempotent). No corruption.
- **Terminal already published** — a redelivery reloads, re-polls, no-ops at step 6, retries the same terminal-analysis handoff (idempotent), and acks. Harmless.
- **Postpone write failed** — the framework abandons the delivery; the visibility timeout lapses into a normal redelivery, which re-runs from step 1 and no-ops at step 6. The poll loop's continuation is framework-owned.

The window to guard is between persisting status (step 6) and ack (steps 7–8); because status writes are CAS-guarded, monotonic, and write-once at terminal, a redelivery always observes a consistent row.
Expand All @@ -169,4 +168,4 @@ One boundary of that posture is worth stating: the `MaxAttempts` path fires only

## Entity, storage, and queue additions

No additions beyond [build.md](build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. A non-terminal poll calls `delivery.Hold` on that delivery rather than publishing another one. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. The message it publishes to `record`, and the `record` topic key itself, are owned by the `record` stage and land with `record.md`; `buildsignal` only needs that the **request id** reaches the record topic once the build is terminal, partitioned by request id.
No additions beyond [build.md](build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. A non-terminal poll calls `delivery.Hold` on that delivery rather than publishing another one. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. Terminal analysis owns the next normal handoff; only cancellation publishes directly to the record topic. Both messages use their existing build-id or request-id partitions.
19 changes: 9 additions & 10 deletions doc/rfc/stovepipe/steps/record.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

`record` turns a terminal build outcome into a durable validation fact.

- Phase 1 records whole-repository greenness and optional project facts. On green it promotes the commit onto the Queue's promotion ref if the Request still holds the last-green bookmark, then announces the outcome as a hook event. This is implemented: see [stovepipe/controller/record/record.go](../../../../stovepipe/controller/record/record.go).
- Phase 1 records whole-repository greenness using project results already persisted by `projectanalysis`. On green it promotes the commit onto the Queue's promotion ref if the Request still holds the last-green bookmark, then announces the outcome as a hook event. This is implemented: see [stovepipe/controller/record/record.go](../../../../stovepipe/controller/record/record.go).
- Phase 2 records greenness per project instead of per repository. Sketched here, to be expanded before implementation.

Notifying downstream systems rides the cross-domain hook framework rather than a Stovepipe-specific extension; see [Hooks](#hooks).

See [workflow.md](../workflow.md) for the whole pipeline, [build.md](build.md) for how builds are created, and [buildsignal.md](buildsignal.md) for the terminal-only handoff into this stage.

`record` owns the validation facts and the promotion ref. `buildsignal` advances the last-green build baseline before handing off to this stage. `record` does not decide build scope, poll a build, release the Queue's build slot, interpret a target graph, or map targets to projects. Those belong to `process`, `buildsignal`, and `analyze`.
`record` owns the repository fact and promotion ref; `projectanalysis` owns project facts and issue attribution. `buildsignal` advances the last-green build baseline before handing off to this stage. `record` does not decide build scope, poll a build, release the Queue's build slot, interpret a target graph, or map targets to projects. Those belong to `process`, `buildsignal`, and `projectanalysis`.

## Phase 1 algorithm

Expand Down Expand Up @@ -39,8 +39,8 @@ For a delivery carrying a `Record` payload:
- superseded -> ack, no fact. Unreachable in practice.
- accepted / processing / anything else -> return a non-retryable invariant error.

6. Resolve optional project results using R and its TerminalBuildID, and validate
the entire result set before writing facts. A resolver error returns raw.
6. Load the request summary and its persisted ProjectCount. Project analysis
has already recorded results or abandonment before publishing here.

7. Map R.State and the project results to a whole-repository degree and create the
ValidationFact keyed by (R.URI, empty project) in the queue-bound fact store.
Expand All @@ -50,9 +50,8 @@ For a delivery carrying a `Record` payload:
- ErrAlreadyExists -> load and reconcile the existing immutable fact.
- other store error -> return raw.

8. Record each project fact with its resolver-provided degree and persist history
for the repository fact and completed project facts. A failure returns raw;
replay completes missing writes without replacing immutable facts.
8. Persist history for the repository fact. A failure returns raw; replay adopts
the immutable fact and repairs its history.

9. If the persisted repository fact is not green, skip to step 11. Report how long
the break went undetected only if step 7 created the fact (see "Observability").
Expand Down Expand Up @@ -108,7 +107,7 @@ Absence is still distinct from degree `0`. Callers gating deployments must treat

A fact is written only when the Request reaches `succeeded` or `failed`. A `cancelled` build is acked with no fact.

The fail-closed path is subtler than "no fact". A DLQ reconciler forces the Request to `failed` and does not itself publish here, so reconciliation on its own records nothing. That is not the same as nothing being recorded: a `buildsignal` delivery still in flight can reach this stage afterwards, and it will write `DegreeBroken` from the forced state. See [DLQ and fail-closed behavior](#dlq-and-fail-closed-behavior).
Project-analysis recovery hands off after retaining abandonment. A failed request without a completed set of project facts therefore records `DegreeBroken`; partial writes alone do not supply a project count. Build-status recovery also repairs the downstream handoff without polling the runner again. See [DLQ and fail-closed behavior](#dlq-and-fail-closed-behavior).

### Phase 1 degree mapping

Expand All @@ -123,15 +122,15 @@ Whole-repository degree depends on the outcome `buildsignal` stamped on the Requ
| `cancelled` | no fact |


`DegreePartial` is hard coded to `0.5` for now; it does not measure the fraction of projects failing. Each project fact preserves the degree returned by the resolver, unless its immutable identity already exists. After persisting every project fact and its degree index, `record` retains total and broken project counts in `project_facts_recorded` for the request summary. Broken means a persisted degree above zero; retries count adopted facts rather than contradictory resolver results. Empty results record zero counts.
`DegreePartial` is hard coded to `0.5` for now; it does not measure the fraction of projects failing. Each project fact preserves the degree returned by the resolver, unless its immutable identity already exists. After persisting every project fact and its degree index, `projectanalysis` retains total and broken project counts in `project_facts_recorded` for the request summary. Broken means a persisted degree above zero; retries count adopted facts rather than contradictory resolver results. Empty results record zero counts.

## Last-green advancement (Queue bookmark)

`buildsignal` advances the bookmark after releasing the build slot and persisting the successful Request outcome, before publishing to `record`. The bookmark only moves forward. `entity.Queue` carries `LastGreenURI` alongside `LastGreenRequestID`, the request id that owns the current URI, plus `InFlightCount` and `LatestRequestID`. The guard compares ids with `entity.CompareRequestID(R.Queue, …)`, the same ingest-order comparison `ingest` and `process` use for coalescing. It errors on an id that does not match the queue's format, which is non-retryable: re-parsing the same ids cannot start succeeding.

The write goes through `QueueStore.Update(ctx, queue, oldVersion, newVersion)`, so version arithmetic stays in the controller: it computes `newVersion = oldVersion + 1` and the store does a pure conditional write. `ErrVersionMismatch` is absorbed by the loop rather than returned, since a concurrent writer moved the row and reloading converges.

The bookmark is a successful-build baseline and can precede validation facts. Project resolution and Git promotion remain in `record`. Releasing capacity and advancing the bookmark are separate Queue updates, so a new build may be admitted with the previous baseline before the advance lands. A terminal redelivery retries the bookmark update without releasing the slot again.
The bookmark is a successful-build baseline and can precede validation facts. Project resolution runs in `projectanalysis`; Git promotion remains in `record`. Releasing capacity and advancing the bookmark are separate Queue updates, so a new build may be admitted with the previous baseline before the advance lands. A terminal redelivery retries the bookmark update without releasing the slot again.

A failed or cancelled build never moves the bookmark, and so never promotes either.

Expand Down
Loading
Loading