Skip to content

feat(storage): speculation path set store - #501

Open
behinddwalls wants to merge 1 commit into
preetam/speculation-standardfrom
preetam/speculation-path-store
Open

feat(storage): speculation path set store#501
behinddwalls wants to merge 1 commit into
preetam/speculation-standardfrom
preetam/speculation-path-store

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

The speculation RFC has the speculate controller read every materialized path set at the start of a run and persist the paths it funds at the end. entity.SpeculationPathSet has existed since the path entities landed, but nothing can store one: there is no interface, no schema, no implementation. This is the first of the pieces the controller integration needs, and it is useful to review on its own because the contract shape is the part worth arguing about.

What?

Adds storage.SpeculationPathSetStore, keyed on the head batch ID, with a MySQL implementation, schema, mocks, and contract tests.

The mutation is a conditional whole-item put — Update takes the entity and replaces it, guarded on version — rather than the field-level UpdateState/UpdateDependents shape the other stores use. A set is replaced wholesale, so the entity is the unit of replacement, and a conditional put on a key is the primitive every backend offers directly: DynamoDB PutItem with a ConditionExpression, a conditional row write in Bigtable. A field-level update would be the one shape each non-SQL backend has to emulate with a read-modify-write. Version arguments come last, and the entity's own Version field is ignored — oldVersion guards and newVersion is written, so version arithmetic stays with the caller per the storage README.

The table has no secondary index. Callers that want a queue's live sets enumerate the heads from the batch listing they already hold and read each set by key, which is the "domain state is already the index" branch of the storage README's decision path.

One version per set is deliberate. Speculate, build, and buildsignal all mutate the same row, so each does read, modify, compare-and-swap, and a loser gets the retryable ErrVersionMismatch and re-reads on redelivery.

Test Plan

bazel test //submitqueue/extension/storage/... — sqlmock unit tests covering get/create/update, ErrNotFound, ErrAlreadyExists, ErrVersionMismatch, malformed stored JSON, and a test pinning that Update ignores the entity's Version field in favour of the explicit arguments.

bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test — four new cases in the shared storage contract suite, run against real MySQL and confirmed executing under -test.v: round-trip (asserting each stored ID still equals the hash of its stored path, so an encoding that dropped or reordered assumptions would fail), missing head, duplicate create, and a compare-and-swap race where the loser must not restore the path the winner dropped.

make fmt, make gazelle, make mocks

Issues

@behinddwalls
behinddwalls marked this pull request as ready for review August 3, 2026 16:46
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 3, 2026 16:46
## Summary

### Why?

The speculation RFC has the speculate controller read every materialized path set at the start of a run and persist the paths it funds at the end. `entity.SpeculationPathSet` has existed since the path entities landed, but nothing can store one: there is no interface, no schema, no implementation. This is the first of the pieces the controller integration needs, and it is useful to review on its own because the contract shape is the part worth arguing about.

### What?

Adds `storage.SpeculationPathSetStore`, keyed on the head batch ID, with a MySQL implementation, schema, mocks, and contract tests.

The mutation is a conditional whole-item put — `Update` takes the entity and replaces it, guarded on version — rather than the field-level `UpdateState`/`UpdateDependents` shape the other stores use. A set is replaced wholesale, so the entity is the unit of replacement, and a conditional put on a key is the primitive every backend offers directly: DynamoDB `PutItem` with a `ConditionExpression`, a conditional row write in Bigtable. A field-level update would be the one shape each non-SQL backend has to emulate with a read-modify-write. Version arguments come last, and the entity's own `Version` field is ignored — `oldVersion` guards and `newVersion` is written, so version arithmetic stays with the caller per the storage README.

The table has no secondary index. Callers that want a queue's live sets enumerate the heads from the batch listing they already hold and read each set by key, which is the "domain state is already the index" branch of the storage README's decision path.

One version per set is deliberate. Speculate, build, and buildsignal all mutate the same row, so each does read, modify, compare-and-swap, and a loser gets the retryable `ErrVersionMismatch` and re-reads on redelivery.

## Test Plan

✅ `bazel test //submitqueue/extension/storage/...` — sqlmock unit tests covering get/create/update, `ErrNotFound`, `ErrAlreadyExists`, `ErrVersionMismatch`, malformed stored JSON, and a test pinning that `Update` ignores the entity's `Version` field in favour of the explicit arguments.

✅ `bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test` — four new cases in the shared storage contract suite, run against real MySQL and confirmed executing under `-test.v`: round-trip (asserting each stored ID still equals the hash of its stored path, so an encoding that dropped or reordered assumptions would fail), missing head, duplicate create, and a compare-and-swap race where the loser must not restore the path the winner dropped.

✅ `make fmt`, `make gazelle`, `make mocks`
@behinddwalls
behinddwalls force-pushed the preetam/speculation-path-store branch from 46e9005 to c56ac02 Compare August 3, 2026 23:03
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.

1 participant