Skip to content

feat(storage): path-build link store for per-path builds - #502

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/speculation-path-build-store
Aug 7, 2026
Merged

feat(storage): path-build link store for per-path builds#502
behinddwalls merged 1 commit into
mainfrom
preetam/speculation-path-build-store

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

Speculation needs one build per path, and something must record which build belongs to which path attempt. The runner chooses the build ID, so a caller holding a path cannot derive it; a keyed record is the key-value contract's mechanism for a reverse lookup.

What?

entity.PathBuild is new: the (pathID, attempt) -> buildID link, with storage.PathBuildStore, a MySQL implementation, schema, and mocks.

The record is write-once: created only once the runner has named the build, never updated. Absent means no build is recorded for the attempt; present names its build permanently, and a retried path is a new attempt under a different key. Because creation is the only write, the first insert also decides concurrent dispatches for the same attempt — ErrAlreadyExists tells the loser the attempt's build is someone else's. There is deliberately no reservation state and no version column: the stage that watches builds (later in this stack) stops any build whose path no longer wants it, so no reader needs to distinguish "idle" from "mid-dispatch".

entity.Build gains PathID and Attempt, and entity.SpeculationPath gains Base() — the dependencies the path assumes will succeed — so every consumer derives the base one way.

Test Plan

bazel test //submitqueue/entity/... //submitqueue/extension/storage/...

bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test — an attempt resolving to its build, ErrNotFound when undispatched, and a duplicate Create refused so the first link stands.

make fmt, make gazelle, make mocks

Issues

@behinddwalls
behinddwalls marked this pull request as ready for review August 3, 2026 17:11
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 3, 2026 17:11
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 5692021 to 4443963 Compare August 3, 2026 23:03
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 4443963 to 471898d Compare August 4, 2026 01:58
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 471898d to edc7d3a Compare August 4, 2026 05:11
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from edc7d3a to 28db660 Compare August 5, 2026 03:15
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 28db660 to b528041 Compare August 5, 2026 03:26
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from b528041 to 51fe6ef Compare August 5, 2026 03:37
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 51fe6ef to 226cfe9 Compare August 5, 2026 03:43
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 226cfe9 to 994904d Compare August 5, 2026 04:11
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 7bad238 to 34add68 Compare August 5, 2026 17:05
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 34add68 to 0ecb87d Compare August 5, 2026 17:28
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 0ecb87d to 4fe0ed0 Compare August 5, 2026 19:05
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 4fe0ed0 to 6138ba9 Compare August 6, 2026 02:15
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch 2 times, most recently from e9be5be to 68e2cf0 Compare August 6, 2026 03:04
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 68e2cf0 to 51812bc Compare August 6, 2026 03:17
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch 2 times, most recently from 06bb174 to de8085d Compare August 7, 2026 03:15
Base automatically changed from preetam/speculation-path-store to main August 7, 2026 03:30
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from de8085d to 9d4492c Compare August 7, 2026 03:30
## Summary

### Why?

Speculation needs one build per *path*, and something must record which build belongs to which path attempt. The runner chooses the build ID, so a caller holding a path cannot derive it; a keyed record is the key-value contract's mechanism for a reverse lookup.

### What?

`entity.PathBuild` is new: the `(pathID, attempt) -> buildID` link, with `storage.PathBuildStore`, a MySQL implementation, schema, and mocks.

The record is write-once: created only once the runner has named the build, never updated. Absent means no build is recorded for the attempt; present names its build permanently, and a retried path is a new attempt under a different key. Because creation is the only write, the first insert also decides concurrent dispatches for the same attempt — `ErrAlreadyExists` tells the loser the attempt's build is someone else's. There is deliberately no reservation state and no version column: the stage that watches builds (later in this stack) stops any build whose path no longer wants it, so no reader needs to distinguish "idle" from "mid-dispatch".

`entity.Build` gains `PathID` and `Attempt`, and `entity.SpeculationPath` gains `Base()` — the dependencies the path assumes will succeed — so every consumer derives the base one way.

## Test Plan

✅ `bazel test //submitqueue/entity/... //submitqueue/extension/storage/...`

✅ `bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test` — an attempt resolving to its build, `ErrNotFound` when undispatched, and a duplicate `Create` refused so the first link stands.

✅ `make fmt`, `make gazelle`, `make mocks`

# Conflicts:
#	submitqueue/extension/storage/BUILD.bazel
#	submitqueue/extension/storage/mock/BUILD.bazel
#	submitqueue/extension/storage/mock/storage_mock.go
#	submitqueue/extension/storage/mysql/BUILD.bazel
#	submitqueue/extension/storage/mysql/build_store.go
#	submitqueue/extension/storage/mysql/build_store_test.go
#	submitqueue/extension/storage/mysql/storage.go
#	submitqueue/extension/storage/storage.go

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto d79dd62e
# Last command done (1 command done):
#    pick 06bb174 # feat(storage): path-build link store for per-path builds
# No commands remaining.
# You are currently rebasing branch 'preetam/speculation-path-build-store' on 'd79dd62e'.
#
# Changes to be committed:
#	modified:   submitqueue/entity/BUILD.bazel
#	modified:   submitqueue/entity/build.go
#	new file:   submitqueue/entity/path_build.go
#	modified:   submitqueue/entity/speculation.go
#	modified:   submitqueue/extension/storage/BUILD.bazel
#	modified:   submitqueue/extension/storage/mock/BUILD.bazel
#	new file:   submitqueue/extension/storage/mock/path_build_store_mock.go
#	modified:   submitqueue/extension/storage/mock/storage_mock.go
#	modified:   submitqueue/extension/storage/mysql/BUILD.bazel
#	modified:   submitqueue/extension/storage/mysql/build_store.go
#	modified:   submitqueue/extension/storage/mysql/build_store_test.go
#	new file:   submitqueue/extension/storage/mysql/path_build_store.go
#	modified:   submitqueue/extension/storage/mysql/schema/build.sql
#	new file:   submitqueue/extension/storage/mysql/schema/path_build.sql
#	modified:   submitqueue/extension/storage/mysql/storage.go
#	new file:   submitqueue/extension/storage/path_build_store.go
#	modified:   submitqueue/extension/storage/storage.go
#	modified:   test/integration/submitqueue/extension/storage/suite.go
#

# Conflicts:
#	submitqueue/extension/storage/mysql/storage.go

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto 2544f02
# Last command done (1 command done):
#    pick 9d4492c # feat(storage): path-build link store for per-path builds
# No commands remaining.
# You are currently rebasing branch 'preetam/speculation-path-build-store' on '2544f02a'.
#
# Changes to be committed:
#	modified:   submitqueue/entity/BUILD.bazel
#	modified:   submitqueue/entity/build.go
#	new file:   submitqueue/entity/path_build.go
#	modified:   submitqueue/entity/speculation.go
#	modified:   submitqueue/extension/storage/BUILD.bazel
#	modified:   submitqueue/extension/storage/mock/BUILD.bazel
#	new file:   submitqueue/extension/storage/mock/path_build_store_mock.go
#	modified:   submitqueue/extension/storage/mock/storage_mock.go
#	modified:   submitqueue/extension/storage/mysql/BUILD.bazel
#	modified:   submitqueue/extension/storage/mysql/build_store.go
#	modified:   submitqueue/extension/storage/mysql/build_store_test.go
#	new file:   submitqueue/extension/storage/mysql/path_build_store.go
#	modified:   submitqueue/extension/storage/mysql/schema/build.sql
#	new file:   submitqueue/extension/storage/mysql/schema/path_build.sql
#	modified:   submitqueue/extension/storage/mysql/storage.go
#	new file:   submitqueue/extension/storage/path_build_store.go
#	modified:   submitqueue/extension/storage/storage.go
#	modified:   test/integration/submitqueue/extension/storage/suite.go
#
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-build-store branch from 9d4492c to 02917dc Compare August 7, 2026 19:48
@behinddwalls
behinddwalls added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit f177f67 Aug 7, 2026
14 checks passed
@behinddwalls
behinddwalls deleted the preetam/speculation-path-build-store branch August 7, 2026 19:54
behinddwalls added a commit that referenced this pull request Aug 7, 2026
…551)

## Summary

### Why?

Speculation never funded a path, so no build was ever dispatched and the
submitqueue e2e hung until Bazel killed it at 300s, having spilled ~11MB
of retry logs.

Two producers build an entity without naming its queue, and both of
their stores are bound to one queue and reject a write that disagrees:

- `speculate/dispatch.go` created a head's first `SpeculationPathSet` as
`{Head: batch.ID}`, so `Create` failed with `queue "" does not match the
store's bound queue`. That is the first write of every newly funded
head, so no head was ever funded.
- `build/build.go` created the `PathBuild` link as `{PathID, Attempt,
BuildID}`, so the link write failed the same way. Without the link a
path is never observed moving to `building`, so it stays `pending` and
every later run re-dispatches it.

Both stores gained their queue-bound guard when their tables were
re-keyed to lead with the queue (#543 for `speculation_path_set`, #502
for `path_build`); the producers were not updated to match.

Unit tests missed it because they use the gomock stores, which do not
enforce the binding, and the storage contract suite missed it because it
builds its own fixtures with the queue set. Only the e2e exercises a
real producer against a real queue-bound store.

### What?

Stamps the queue at both construction sites: the path set takes the
run's queue, the link takes its batch's.

Turns both into regression tests rather than leaving them to the e2e:
the build controller's link expectations are exact struct matches that
now require the queue, and the speculate run test asserts the created
set names it. Each fails against the unfixed code.

## Test Plan

✅ `go test ./submitqueue/... ./service/...`
✅ Verified each new assertion fails when its fix is reverted
✅ `make lint` / `make check-gazelle`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants