Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions doc/rfc/stovepipe/steps/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ For a delivery carrying request id `R`:
8. ack.
```

`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every downstream hop — `build` → `buildsignal` → `record` each carry the build id in the message, so every reader reaches the `Build` by a direct get on identity it was handed. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passed a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.
`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and the build's terminal status is projected onto `Request.State` before the publish, so `record` never reaches a `Build` at all. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.

`build` writes only the `Build`, and only at creation; it never mutates `Request.State`. The Request stays `processing` (set by `process`) through `build` until `buildsignal` moves it terminal by recording the build's outcome. `Build.Status` is the fine-grained build lifecycle; `Request.State` is the coarse pipeline lifecycle. This is also what keeps `process.md` step 3 correct: because `build` leaves the Request at `processing`, a redelivered `process` message still matches its "if processing, re-publish to build" guard.

Expand Down Expand Up @@ -263,7 +263,9 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo
| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in CLAUDE.md | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs |
| Enforces (rather than assumes) the direct-navigation property SubmitQueue's speculate takes on faith | Diverges entity shape and controller flow from SubmitQueue, weakening the "structurally the same controller" claim and dual-implementing-backend symmetry |

Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s terminal-Request short-circuit and `record`'s CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land.
Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land.

Note that moving `record`'s input from the build id to the request id does *not* trigger this alternative, even though it removes the last hop that carried a build id to a Request-scoped consumer. The trigger condition is a stage that must **derive a build's key from a Request**, and `record` does not: the build's terminal status is projected onto `Request.State` before the publish, so `record` reads the Request and never reaches a `Build`.

#### Alternative B: caller-supplied idempotency key on `Trigger`

Expand Down Expand Up @@ -312,7 +314,7 @@ Either could be adopted independently: the idempotency token, if a backend that

`IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#algorithm), step 6).

Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` and `record` reach a build by the id carried in their messages, so no reverse index from `Request` to its builds is ever needed.
Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed.

**`BuildStore`** (new, added to the `Storage` aggregator via `GetBuildStore()`), matching stovepipe's existing `RequestStore` conventions — **generic `Update` with caller-owned version arithmetic, not SubmitQueue's field-specific `UpdateStatus`**:

Expand Down
22 changes: 12 additions & 10 deletions doc/rfc/stovepipe/steps/buildsignal.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Buildsignal stage

`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the build id to `record`. See [workflow.md](doc/rfc/stovepipe/workflow.md) for where it sits in the pipeline and [build.md](doc/rfc/stovepipe/steps/build.md) for how the build it polls was triggered.
`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the **request id** to `record`. See [workflow.md](doc/rfc/stovepipe/workflow.md) for where it sits in the pipeline and [build.md](doc/rfc/stovepipe/steps/build.md) for how the build it polls was triggered.

It handles only the poll loop: it does not decide build strategy, write greenness, or map targets to projects — those are `process`'s, `record`'s, and `analyze`'s jobs. It reports the *fact* (this request's build ended succeeded, failed, or cancelled); `record` derives what that fact *means* for greenness.

Expand All @@ -10,7 +10,7 @@ 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](doc/rfc/stovepipe/workflow.md#workflow)).

Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and publishes the build 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 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.

`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](doc/rfc/stovepipe/steps/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.

Expand Down Expand Up @@ -65,12 +65,14 @@ For a delivery carrying build id `B`:
- failure here aborts the step: R must not go terminal while still holding a slot.
b. CAS R from processing to the outcome the stored status projects onto it:
succeeded -> succeeded, failed -> failed, cancelled -> cancelled. First writer wins.
Then publish B (the build key, Build.ID) to the record topic,
partitioned by request id; ack, return. No re-publish to buildsignal.
- record loads the Build directly by this key (Build->Request gives it R for the per-Queue writes),
so no reverse lookup from Request to its builds is ever needed.
- a redelivery republishes the same terminal signal; record is idempotent.
- publish failure -> return raw (non-retryable); status is persisted, operational republish recovers.
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 and derives greenness from its outcome,
so it never reaches a Build and no reverse lookup from Request to its builds is needed.
- 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.

8. Else PublishAfter(B -> buildsignal, delayMs), partitioned by build id:
- delayMs = pollDelay(status): shorter while running, longer while accepted.
Expand Down Expand Up @@ -135,7 +137,7 @@ Everything else — factory lookup, an `Update` store error other than a CAS con
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 re-schedule the poll (non-terminal) or republish to `record` (terminal, idempotent). No corruption.
- **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 re-schedule the 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.
- **`PublishAfter` failed, then retried** — the nacked delivery re-runs from step 1; there is no way to resume mid-algorithm, so it re-polls the runner too, but the row already carries the non-terminal status and step 6 no-ops. Only the final enqueue does new work.

Expand All @@ -154,4 +156,4 @@ One boundary of that posture is worth stating: the `MaxAttempts` path fires only

## Entity, storage, and queue additions

No additions beyond [build.md](doc/rfc/stovepipe/steps/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/re-produces the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. `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 **build id** (the build key, so `record` loads the `Build` directly) reaches the record topic once the build is terminal, partitioned by request id.
No additions beyond [build.md](doc/rfc/stovepipe/steps/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/re-produces the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. `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.
4 changes: 2 additions & 2 deletions doc/rfc/stovepipe/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe
1. **ingest** — invoked by the external poller with a **Queue name**. It asks `SourceControl` for that Queue's current head URI, mints a Request namespaced by the Queue, persists it with no recorded greenness yet, and dedups on `(Queue, head URI)` so a re-reported head is processed once. It publishes the RequestID onward.
2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, and publishes to `build`. See [process.md](steps/process.md).
3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID.
4. **buildsignal** — records the build's status and target graph when the build completes, then releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the BuildID to `record` (the `Build` row carries its RequestID, so `record` reaches the Request with a direct get).
4. **buildsignal** — records the build's status and target graph when the build completes, then releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`.
5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here. It fires the **Hooks** extension with the green/not-green event, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal.

### Phase 2 — project greenness
Expand All @@ -147,7 +147,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe
| **process** | RequestID | build | Build strategy, concurrency gate, backlog coalescing → [process.md](steps/process.md) |
| **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental |
| **buildsignal** | BuildID | record (P1), record (P2) | Record build status + target graph; release `in_flight_count`; project the outcome onto the Request; signal completion |
| **record** | BuildID | analyze (P1→P2), Hooks | Write greenness; advance last-green URI on whole-repo green; fire Hooks |
| **record** | RequestID | analyze (P1→P2), Hooks | Write greenness; advance last-green URI on whole-repo green; fire Hooks |
| **analyze** | RequestID | build | Map broken/at-risk targets → projects; decide project-scoped builds |

## Step RFCs
Expand Down
24 changes: 13 additions & 11 deletions stovepipe/controller/buildsignal/buildsignal.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// BuildSignal messages (a build id), polls the build-runner until the build
// reaches a terminal status, persists that status, and — once terminal —
// releases the queue's build slot, projects the outcome onto the request, and
// publishes the build id to record. See
// publishes the request id to record. See
// doc/rfc/stovepipe/steps/buildsignal.md.
package buildsignal

Expand Down Expand Up @@ -57,7 +57,7 @@ var (
// Controller consumes BuildSignal messages, polls the build-runner toward a
// terminal status, persists the result, and either reschedules itself or
// releases the queue's build slot, projects the outcome onto the request, and
// publishes the build id to record. Implements consumer.Controller.
// publishes the request id to record. Implements consumer.Controller.
type Controller struct {
logger *zap.SugaredLogger
metricsScope tally.Scope
Expand Down Expand Up @@ -98,8 +98,8 @@ func NewController(
// Process reloads the build referenced by the delivery, polls its runner for
// the latest status, persists a real transition, and either reschedules a
// poll or, once terminal, releases the queue's build slot, projects the
// outcome onto the request, and publishes the build id to record. Returns nil
// to ack (success) or an error to nack (retry) / reject (DLQ).
// outcome onto the request, and publishes the request id to record. Returns
// nil to ack (success) or an error to nack (retry) / reject (DLQ).
func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) error {
msg := delivery.Message()

Expand Down Expand Up @@ -157,8 +157,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
if err := c.finishRequest(ctx, &request, effective); err != nil {
return err
}
if err := c.publishRecord(ctx, build.ID, request.ID); err != nil {
return fmt.Errorf("failed to publish record for build %s: %w", build.ID, err)
if err := c.publishRecord(ctx, request.ID); err != nil {
return fmt.Errorf("failed to publish record for request %s: %w", request.ID, err)
}
c.logger.Infow("build reached terminal status",
"build_id", build.ID,
Expand Down Expand Up @@ -336,14 +336,16 @@ func pollDelay(status entity.BuildStatus) int64 {
}
}

// publishRecord publishes buildID to the record stage, partitioned by
// requestID.
func (c *Controller) publishRecord(ctx context.Context, buildID, requestID string) error {
payload, err := stovepipemq.Marshal(&stovepipemq.Record{Id: buildID})
// publishRecord publishes requestID to the record stage, partitioned by request
// id. The message id is the request id too, so a redelivery republishing the
// same terminal signal dedups into the original message rather than enqueuing a
// second one.
func (c *Controller) publishRecord(ctx context.Context, requestID string) error {
payload, err := stovepipemq.Marshal(&stovepipemq.Record{Id: requestID})
if err != nil {
return fmt.Errorf("failed to serialize record: %w", err)
}
msg := entityqueue.NewMessage(buildID, payload, requestID, nil)
msg := entityqueue.NewMessage(requestID, payload, requestID, nil)
return c.publish(ctx, stovepipemq.TopicKeyRecord, msg, 0)
}

Expand Down
Loading
Loading