Commit 61db11f
authored
feat(requestlog): report speculation and build progress to requests (#568)
## Summary
### Why?
A request read `batched` for the whole of its active life. It kept that
status while its batch was admitted to speculation, while CI built it,
and while it was being pushed, so a customer could not tell a working
request from a wedged one — and diagnosing a stall meant reading the
batch table directly. `entity.RequestStatus` defined eighteen values and
only nine were ever published.
Two things stood in the way of simply adding publish calls.
`PublishLog` derived the queue message id as `{requestID}/{status}`, and
`queue_messages` is uniquely indexed on `(topic, partition_key, id)`
with `ON DUPLICATE KEY UPDATE`. A second `building` for a request was
therefore dropped at publish time, silently. Since a batch is rebuilt
every time speculation re-plans, a one-shot `building` would have left a
request stuck in a rebuild loop reading identically to one that built
cleanly first try — the exact distinction this is meant to restore.
More fundamentally, every log entry competed to become the request's
current status, and build progress cannot be one. A head funds several
speculation paths and each is built separately, so one build succeeding
while its siblings still run does not mean the request is finished — and
because nothing publishes again until the batch resolves, a summary
moved there would stay there. That is the "looks done but is wedged"
reading the issue exists to kill.
### What?
The request log gains two tiers.
**Statuses** are coarse, one per pipeline position, and move the
summary: `validating` on entry to validation, `speculating` on
admission, `speculated` once a path has passed and the head is only
waiting on its dependencies, and `landing` when the merge request is
dispatched. `speculated` sits at the passed-path observation rather than
at the merge decision, because the decision is one queue hop from
`landing` while the dependency wait can run for minutes.
**Events** are per (path, build), appear in history, and never move the
summary: `building` and `built`, carrying the batch, path, attempt, and
the runner's CI URL. `entity.IsRequestStatusEvent` names them and
`logWins` skips them — no schema change, since `request_log` already
stores every entry and history already returns all of them.
The message id gains an occurrence discriminator,
`{requestID}/{status}[/{occurrence}]`: the build id for build events,
the path id for `speculated`, the batch id for `speculating` and
`landing`. A redelivery of one occurrence still dedupes; a genuine
repeat gets through. Existing call sites pass `""` and keep today's
one-shot behaviour, which is what a terminal status wants.
Every new publish goes out before the state write it reports. Each of
these branches runs once — `admit` only from `BatchStateCreated`,
`buildsignal` only on an observed transition — so an entry published
after the write would be lost for good when it failed, since the replay
reads the updated record and takes neither branch. Publishing first
means a failure nacks with nothing changed, and a crash in between
re-publishes under the same occurrence.
`landing` is published by the orchestrator rather than runway,
correcting the issue's "Where": runway is a separate service that
consumes `MergeRequest` protos and holds no submitqueue storage, no log
topic, and no request ids. `building` moves to `buildsignal` rather than
the build controller, because `Trigger` returns only an id and the CI
URL comes from `Status` — which `buildsignal` already called and
discarded. The build controller is unchanged and keeps its "this stage
only starts builds" invariant.
`waitingpath` is dropped: the window it named is now the interval
between `speculated` and `landing`.
`batching` and `processing` stay in the enum unpublished. The batch
controller batches immediately with no waiting gate, and
`RequestStateProcessing` is never written anywhere in the submitqueue
domain, so both would be synthetic moments invented to fill the enum.
## Test Plan
✅ `bazel test //submitqueue/... //service/... //platform/...` — 70 pass
New coverage for the parts that are easy to get wrong:
- `materializer_test.go` — the tier guarantee: a `building` or `built`
entry is inserted into the log but leaves `RequestSummary.Status`
untouched, while the position after it still wins.
- `log_test.go` — the occurrence appears in the message id, `""`
reproduces today's id exactly, and one fan-out shares its occurrence
across members.
- `speculate` — `speculated` fires for a passed path with unsettled
assumptions; a dependency that resolves against that path puts the
members back to `speculating`; `livePassedPath` recognises the waiting
window that `mergeablePath` rejects.
- `buildsignal` — one `building` per build carrying `build_url`, `built`
only on success, nothing on an unchanged poll, and nothing recorded when
the report fails.
- `merge` — the `landing` fan-out is published before the runway
dispatch, and a failed report stops the run before runway hears about
the merge.
The e2e happy path now asserts the full trail as an ordered subsequence
and checks that the summary never reported an event status.
## Issue
Closes CODEM-426
## Issues1 parent 581118c commit 61db11f
41 files changed
Lines changed: 1327 additions & 216 deletions
File tree
- api/submitqueue/gateway
- protopb
- proto
- service/submitqueue/gateway/server/mapper
- submitqueue
- core/request
- entity
- extension/storage/mysql
- schema
- gateway/controller
- log
- orchestrator/controller
- batch
- buildsignal
- mergeconflictsignal
- merge
- speculate
- start
- validate
- test
- e2e/submitqueue
- integration/submitqueue/gateway
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
170 | 170 | | |
171 | 171 | | |
172 | 172 | | |
173 | | - | |
| 173 | + | |
174 | 174 | | |
175 | 175 | | |
176 | 176 | | |
177 | 177 | | |
178 | 178 | | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
179 | 185 | | |
180 | 186 | | |
181 | 187 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
33 | 37 | | |
34 | 38 | | |
35 | 39 | | |
36 | 40 | | |
37 | 41 | | |
| 42 | + | |
38 | 43 | | |
| 44 | + | |
39 | 45 | | |
40 | 46 | | |
41 | 47 | | |
| |||
0 commit comments