Skip to content

feat(storage): add QueueBatchStateStore and shared batch state helpers - #523

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/active-batch
Aug 6, 2026
Merged

feat(storage): add QueueBatchStateStore and shared batch state helpers#523
behinddwalls merged 1 commit into
mainfrom
preetam/active-batch

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

BatchStore.GetByQueueAndStates is the storage contract's only query-by-attribute, backed by idx_queue_state — the only secondary index in the entire schema. The storage README's key-value contract flags exactly this shape: a genuinely needed reverse lookup should be a first-class mapping store keyed by the lookup attribute, so any backend (SQL, DynamoDB, Bigtable) can serve it as a primary-key read. This PR adds that store and the shared helpers controllers will migrate onto; speculate/run.go already carries a TODO for this replacement.

What?

  • entity.QueueBatchState — an advisory membership record filing an in-queue batch under a (queue, state) bucket, existing from batch creation until the batch exits the queue (through terminal states, hence not "active"). Also adds entity.AllBatchStates() for the future conclude-time sweep.
  • storage.QueueBatchStateStoreList(queue, state) / Put / Delete, all idempotent. The MySQL impl is backed by a new queue_batch_state table whose PK (queue, state, batch_id) is the lookup: listing a state bucket is a PK-prefix scan, no secondary index.
  • submitqueue/core/batch — the shared protocol primitives: Transition (batch CAS, then Put the new-bucket record before Deleting the old one, so a batch always has at least one record), EnsureRecord (idempotent repair for redelivery skip branches), and ListByStates (scan requested buckets, dedupe, hydrate by key with bounded concurrency, classify by the hydrated authoritative state — a stale record can misplace a batch but never misreport it).
  • Contract-suite coverage for the new store and a README pointer beside the ChangeRecord mapping-store example.

Scoped to the foundation only: the store has no callers yet, so there is no runtime behavior change. Follow-ups migrate the six batch-transition sites and the two GetByQueueAndStates readers onto the helpers, add conclude-time record deletion, and then remove GetByQueueAndStates + idx_queue_state.

Test Plan

  • bazel test //submitqueue/entity:all //submitqueue/core/batch:all //submitqueue/extension/storage/...
  • bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test (real MySQL; includes the new TestStorage_QueueBatchStateRecordLifecycle contract case)
  • make tidy / make gazelle / make fmt / make mocks — all stable (re-running produces no diffs)
  • Note: make test failures in orchestrator/controller/batch and controller/cancel are pre-existing — they reproduce at clean HEAD db51df4 in a fresh worktree.

@behinddwalls
behinddwalls marked this pull request as ready for review August 6, 2026 01:11
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 6, 2026 01:11
@behinddwalls
behinddwalls changed the base branch from preetam/speculation-wiring to main August 6, 2026 01:11
## Summary

### Why?

`BatchStore.GetByQueueAndStates` is the storage contract's only query-by-attribute, backed by `idx_queue_state` — the only secondary index in the entire schema. The storage README's key-value contract flags exactly this shape: a genuinely needed reverse lookup should be a first-class mapping store keyed by the lookup attribute, so any backend (SQL, DynamoDB, Bigtable) can serve it as a primary-key read. This PR adds that store and the shared helpers controllers will migrate onto; `speculate/run.go` already carries a TODO for this replacement.

### What?

- `entity.QueueBatchState` — an advisory membership record filing an in-queue batch under a (queue, state) bucket, existing from batch creation until the batch exits the queue (through terminal states, hence not "active"). Also adds `entity.AllBatchStates()` for the future conclude-time sweep.
- `storage.QueueBatchStateStore` — `List(queue, state)` / `Put` / `Delete`, all idempotent. The MySQL impl is backed by a new `queue_batch_state` table whose PK (queue, state, batch_id) *is* the lookup: listing a state bucket is a PK-prefix scan, no secondary index.
- `submitqueue/core/batch` — the shared protocol primitives: `Transition` (batch CAS, then Put the new-bucket record before Deleting the old one, so a batch always has at least one record), `EnsureRecord` (idempotent repair for redelivery skip branches), and `ListByStates` (scan requested buckets, dedupe, hydrate by key with bounded concurrency, classify by the hydrated authoritative state — a stale record can misplace a batch but never misreport it).
- Contract-suite coverage for the new store and a README pointer beside the `ChangeRecord` mapping-store example.

Scoped to the foundation only: the store has no callers yet, so there is no runtime behavior change. Follow-ups migrate the six batch-transition sites and the two `GetByQueueAndStates` readers onto the helpers, add conclude-time record deletion, and then remove `GetByQueueAndStates` + `idx_queue_state`.

## Test Plan

- ✅ `bazel test //submitqueue/entity:all //submitqueue/core/batch:all //submitqueue/extension/storage/...`
- ✅ `bazel test //test/integration/submitqueue/extension/storage/mysql:go_default_test` (real MySQL; includes the new `TestStorage_QueueBatchStateRecordLifecycle` contract case)
- ✅ `make tidy` / `make gazelle` / `make fmt` / `make mocks` — all stable (re-running produces no diffs)
- Note: `make test` failures in `orchestrator/controller/batch` and `controller/cancel` are pre-existing — they reproduce at clean HEAD db51df4 in a fresh worktree.
@behinddwalls
behinddwalls force-pushed the preetam/active-batch branch from 6c449c2 to 1608170 Compare August 6, 2026 01:15

@ubettigole ubettigole left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one small comment

Comment thread submitqueue/core/batch/list.go
@behinddwalls
behinddwalls merged commit 503ae22 into main Aug 6, 2026
14 checks passed
@behinddwalls
behinddwalls deleted the preetam/active-batch branch August 6, 2026 14:27
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