Skip to content

Commit 4dc7628

Browse files
committed
Update record step
1 parent 5870f7f commit 4dc7628

4 files changed

Lines changed: 140 additions & 151 deletions

File tree

doc/rfc/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting
2525
- [Process stage](stovepipe/steps/process.md) - Build-strategy decision, per-queue concurrency gate, backlog coalescing, entity model, platform prerequisites
2626
- [Build stage](stovepipe/steps/build.md) - Trigger-only stage and Stovepipe's URI-based BuildRunner contract
2727
- [Buildsignal stage](stovepipe/steps/buildsignal.md) - Build polling, terminal status persistence, and the handoff to record
28-
- [Record stage](stovepipe/steps/record.md) - Immutable greenness facts, Queue coordination, Hooks notification, and the Phase 1 handoff to analyze
28+
- [Record stage](stovepipe/steps/record.md) - Immutable validation facts, Queue coordination, Hooks notification, and the Phase 1 handoff to analyze
2929

3030
## Runway
3131

doc/rfc/stovepipe/steps/build.md

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ For a delivery carrying request id `R`:
3939
- baseURI = R.BaseURI if R.BuildStrategy == incremental_since_green, else "" (full build).
4040
- (headURI = R.URI, baseURI) identify the scope; both are opaque SourceControl tokens.
4141
42-
5. Trigger: buildID, err := buildRunner.Trigger(ctx, R.URI, baseURI, metadata)
42+
5. Trigger: buildID, err := buildRunner.Trigger(ctx, baseURI, R.URI, metadata)
4343
- Trigger takes no caller-supplied id; the runner mints the build's identity,
4444
and buildID becomes Build.ID — SubmitQueue's exact convention (see
4545
"Alternatives considered" under the contract sketch).
@@ -52,8 +52,10 @@ For a delivery carrying request id `R`:
5252
either domain — the shape is deferred until then, not decided here.
5353
- failure -> return raw; classifier decides (transient runner blip retryable, bad URI not).
5454
55-
6. Persist Build{ID: buildID.ID, RequestID: R.ID, URI: R.URI, BaseURI: baseURI,
56-
Status: accepted, Version: 1} via BuildStore.Create.
55+
6. Persist Build{ID: buildID.ID, RequestID: R.ID, Status: accepted, Version: 1}
56+
via BuildStore.Create.
57+
- the row carries no scope; it is recoverable from the Request's immutable fields
58+
(see the entity table).
5759
- a crash between step 5 and this write orphans the triggered build (see Idempotency).
5860
- ErrAlreadyExists -> benign (reachable only with a backend that returns deterministic ids
5961
for retried triggers); continue to step 7.
@@ -78,7 +80,7 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu
7880
- **Request 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.
7981
- **Strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet.
8082
- **Request already terminal** (step 2) — ack, no build. A redelivery after `record` finished, or after `process` superseded the head, never starts a stale build.
81-
- **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `record`'s terminal transition is CAS-guarded, so the second verdict is a no-op. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue.
83+
- **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `buildsignal`'s outcome write is first-writer-wins, so the second verdict cannot flip the Request's state or overwrite the create-only validation fact. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue.
8284
- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, and the fail-closed reconciler drives the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)).
8385

8486
## Edge cases
@@ -128,7 +130,7 @@ The batches are **identity** — thin references carrying ids, not change conten
128130
Stovepipe validates **one commit** against a baseline (or in full). Its `build` controller reads two opaque URIs off the `Request` and triggers:
129131

130132
```go
131-
buildID, err := buildRunner.Trigger(ctx, headURI, baseURI, metadata)
133+
buildID, err := buildRunner.Trigger(ctx, baseURI, headURI, metadata)
132134
```
133135

134136
There is no batch, no dependency list, and nothing to resolve — the URIs *are* the identity, owned by `SourceControl`. `process` already decided incremental-vs-full; `build` just reads `R.BuildStrategy`/`R.BaseURI` and acts.
@@ -153,16 +155,16 @@ type BuildRunner interface {
153155
// Trigger starts a new build every call and mints the build's identity —
154156
// there is no caller-supplied dedup input, matching SubmitQueue's contract
155157
// exactly (see "Alternatives considered for the build identity" below
156-
// for other shapes this doc considered). headURI is the commit
157-
// under validation; baseURI is the incremental baseline (empty for a full
158-
// build). metadata is caller annotations the runner may echo but must not
158+
// for other shapes this doc considered). baseURI is the incremental
159+
// baseline (empty for a full build); headURI is the commit under
160+
// validation. metadata is caller annotations the runner may echo but must not
159161
// depend on — empty today, but expected to carry real data eventually (e.g.
160162
// conflict-graph info, or other upstream decisions relevant to the build)
161163
// once a concrete need lands in either domain; the shape is deferred until
162164
// then, not decided here. Runner-side work is async; callers learn progress
163165
// via Status.
164166
// Returns the runner-assigned build id, which the caller adopts as Build.ID.
165-
Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error)
167+
Trigger(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error)
166168

167169
// Status returns the current status. Takes the id Trigger returned
168170
// (Build.ID). May round-trip to the backend. BuildMetadata is
@@ -189,7 +191,7 @@ type Factory interface{ For(cfg Config) (BuildRunner, error) }
189191
The shape isn't decided here because project semantics belong to `analyze`, not `build`: how a project maps to a buildable scope (a Bazel target pattern, a directory, a service name) is implementer-specific per [workflow.md](doc/rfc/stovepipe/workflow.md#project---greenness-at-a-finer-grain). The expectation is that this stays an opaque token — following the same "identity in, resolve internally" shape already used for `headURI`/`baseURI` (owned and interpreted by `SourceControl`) — that `build` reads off the `Request`/message and hands to the runner uninterpreted, rather than a structured type `build` would have to understand:
190192

191193
```go
192-
Trigger(ctx context.Context, headURI, baseURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error)
194+
Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error)
193195
```
194196

195197
`ProjectScope` lives in `stovepipe/entity` alongside `BuildID`/`BuildStatus`/`BuildMetadata` — projects have no SubmitQueue equivalent at all, not even a shape to mirror. Its zero value covers Phase 1 (no project — whole-repo/incremental scope only, exactly today's sketch); `analyze` is what would populate a non-zero value for Phase 2. This mirrors the additive optional field already reserved on `BuildRequest` for the same purpose (see [Queue contract additions](#queue-contract-additions)) — the wire message and the extension contract need the same new dimension, and both are deferred to the same design.
@@ -198,7 +200,7 @@ Both `Trigger` and `Status`/`Cancel` differ *in contract* between domains, even
198200

199201
There is exactly one build id: the runner mints it at `Trigger`, `build` adopts it as `Build.ID`, and every later call and message carries it verbatim — `Status`/`Cancel` take the same value `Trigger` returned, the queue payload is the same value, the store key is the same value. This is SubmitQueue's convention end to end. The id is opaque: no stovepipe reader parses it, derives it, or equates it with another entity's id — the trap SubmitQueue's speculate/cancel path falls into. And per the extension rules a runner keeps only transient local state, so the durable `Request``Build` linkage lives in **our** store as `Build.RequestID`, never in the runner.
200202

201-
Supporting entity types: `BuildStatus`, `BuildMetadata`, and `BuildID` live in `stovepipe/entity`, shaped the same as SubmitQueue's `submitqueue/entity` equivalents but defined and duplicated locally rather than shared — `BuildStatus` is the narrow lowercase enum `"" (unknown) / accepted / running / succeeded / failed / cancelled` with an `IsTerminal()` predicate covering the last three, `BuildMetadata` is the free-form `map[string]string`, and `BuildID` is a `{ID string}` wire struct wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, `Status`/`Cancel`'s parameter, the queue payload. `stovepipe/entity/build.go` keeps what's stovepipe-specific: the `Build` entity itself (`RequestID`/`URI`/`BaseURI` alongside `ID`/`Status`/`Version`). How a target graph reaches `analyze` is out of scope for this doc — left to the `analyze` design.
203+
Supporting entity types: `BuildStatus`, `BuildMetadata`, and `BuildID` live in `stovepipe/entity`, shaped the same as SubmitQueue's `submitqueue/entity` equivalents but defined and duplicated locally rather than shared — `BuildStatus` is the narrow lowercase enum `"" (unknown) / accepted / running / succeeded / failed / cancelled` with an `IsTerminal()` predicate covering the last three, `BuildMetadata` is the free-form `map[string]string`, and `BuildID` is a `{ID string}` wire struct wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, `Status`/`Cancel`'s parameter, the queue payload. `stovepipe/entity/build.go` keeps what's stovepipe-specific: the `Build` entity itself (`RequestID` alongside `ID`/`Status`/`Version`). How a target graph reaches `analyze` is out of scope for this doc — left to the `analyze` design.
202204

203205
### Alternatives considered for sharing the contract
204206

@@ -210,7 +212,7 @@ Several shapes for sharing the `BuildRunner` contract across domains were raised
210212
// package platform/extension/buildrunner
211213
type BuildRunner interface {
212214
Trigger(ctx context.Context, base []entity.Batch, head entity.Batch, metadata entity.BuildMetadata) (entity.BuildID, error)
213-
TriggerChanges(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error)
215+
TriggerChanges(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error)
214216
Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error)
215217
Cancel(ctx context.Context, buildID entity.BuildID) error
216218
}
@@ -289,18 +291,18 @@ Either could be adopted independently: the idempotency token, if a backend that
289291

290292
## Entity and storage additions needed
291293

292-
**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), while `RequestID`/`URI`/`BaseURI` stay stovepipe-specific:
294+
**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), while `RequestID` stays stovepipe-specific:
293295

294296

295297
| Field | Role | Mutable? |
296298
|---|---|---|
297299
| `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 |
298300
| `RequestID` | The `Request` this build validates (`Build``Request` navigation) | no |
299-
| `URI` | Head URI being built (`== Request.URI`) | no |
300-
| `BaseURI` | Incremental baseline; empty for full builds | no |
301301
| `Status` | `accepted / running / succeeded / failed / cancelled` | **yes**`buildsignal` |
302302
| `Version` | `int32` optimistic-locking version | **yes** — with `Status` |
303303

304+
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, and keeping one copy avoids two rows that can disagree. Phase 2 is where that stops being free: several project builds share one `RequestID`, so whatever discriminator distinguishes their rows (a project id on `Build`, or resolution through `analyze`'s own state) is part of the deferred project-scoped trigger design, alongside the per-signal message id [record.md](record.md#message-queue-additions) needs for the same reason.
305+
304306
**States** (`Build.Status`):
305307

306308
| Status | Meaning | Terminal? |

0 commit comments

Comments
 (0)