Skip to content

Commit 2544f02

Browse files
authored
feat(storage): shard every table by queue (#543)
## Summary ### Why? An audit of all 22 `schema/*.sql` files found five tables that are not shardable by queue: their primary key does not lead with the queue, so one queue's rows stay reachable through another queue's binding and the tables cannot be split across shards. The five are `speculation_path_set`, `counter`, `request_summary`, `request_log`, and `change_uri_request_mapping`. Three of those — the gateway read-model tables — were declared permanently unshardable by `submitqueue/extension/storage/storage.go` and the schema README, on the grounds that their lookups "start from identifiers that arrive without queue context". This change removes that exception at its source rather than working around it: the gateway read APIs now require the queue alongside the sqid or change URI, so the queue is always available at the call site and no identifier ever has to be parsed to recover one. The five platform `messagequeue` tables are deliberately out of scope. They are a message-queue backend keyed by `(consumer_group, topic, partition_key)`, not a domain table set, and sharding them is a separate problem (a global `AUTO_INCREMENT` offset, and a subscriber-heartbeat table whose fair-leasing logic genuinely needs the whole live subscriber set). ### What? All 17 in-scope tables now lead their primary key with the queue. **Independent re-keys.** `speculation_path_set` moves to `(queue, head)` — a batch ID is unique only within its queue, so the head alone was never a safe key. `counter` moves to `(queue, domain)` and gains the standard extension `Config`/`Factory` shape; per the extension contract the factory *implementations* live in the three service `main.go` files, not under `platform/extension/`. **Minted identifiers are unchanged.** Counter domains simplify to bare `"request"` / `"batch"`, but every emitted ID keeps its exact current format: `{queue}/{seq}`, `{queue}/batch/{n}`, and stovepipe's `request/{queue}/{seq}`. Stovepipe previously reused its counter domain *as* the ID prefix, which would have silently turned `request/queueA/7` into `request/7`; the two are now written independently so they cannot drift into each other. Note that re-keying `counter` restarts every sequence at 1, so its table must be recreated in the same cutover as `request`, `batch`, `request_summary`, and `request_log` — never on its own, or freshly minted sqids will collide with surviving rows. **Queue required in the gateway contract.** `CancelRequest`, `GetRequestSummaryByIDRequest`, `GetRequestSummaryByChangeURIRequest`, `GetRequestHistoryByIDRequest`, and `GetRequestHistoryByChangeURIRequest` each gain a `queue` field, validated on entry through the existing `validateQueueIdentifier`. Two behaviour changes follow. The change-URI lookups are now scoped to one queue, so the same URI landed into several queues needs one call per queue. And `Cancel` no longer overrides the caller's queue with the stored one — a mismatched queue yields `NotFound`, since a sqid is simply not resolvable outside its own queue. **Read-model tables re-keyed and folded in.** `request_summary`, `request_log`, and `change_uri_request_mapping` move to queue-leading keys and join the `Storage` aggregate; the `SetGlobalStores` seam and the "deliberately not part of this aggregate" carve-out are deleted. The queue travels as an explicit field on `entity.RequestLog` and `entity.RequestURI`, stamped by producers that already know it — no ID parsing is introduced anywhere. **Enforcement.** A new `//tool/linter/queueshard` walks the schema directories and fails if any primary key does not lead with the queue, or if any secondary index does not — a non-queue-leading index would reintroduce exactly the cross-queue access path the primary key just closed. It is wired into `make lint`. Schema changes are clean recreates, not online migrations, on the schema README's statement that these tables are created empty at rollout and never backfilled. ## Test Plan - ✅ `make test` — 93/93 pass, including the new linter's own tests - ✅ `bazel test //test/integration/...` — 8/8 suites pass against real MySQL, including new cross-queue isolation coverage in the counter, storage, request-URI and request-log contract suites - ✅ `bazel test //test/e2e/...` — both suites pass against the full Docker Compose stack (land → landed, plus status/history/cancel/list through the queue-scoped APIs) - ✅ `make lint` — fmt, license headers, and `queueshard` ("All 17 tables are shardable by queue") - ✅ `make fmt` / `make gazelle` / `make mocks` / `make tidy` — no drift The e2e and image-building integration targets need `--sandbox_writable_path=$HOME/.docker` when run locally; `make e2e-test` does not pass it, which is a pre-existing local-only issue unrelated to this change.
1 parent 9467629 commit 2544f02

82 files changed

Lines changed: 1703 additions & 640 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Makefile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ integration-test-submitqueue-orchestrator: ## Run Orchestrator integration tests
172172
license-fix: ## Add missing license headers to source files
173173
@$(BAZEL) run //tool/linter/licenseheader -- --fix
174174

175-
lint: lint-fmt lint-license ## Run all linters
175+
lint: lint-fmt lint-license lint-queue-shard ## Run all linters
176176
@echo "All lint checks passed."
177177

178178
lint-fmt: fmt ## Check code formatting (fails if unformatted)
@@ -182,6 +182,9 @@ lint-fmt: fmt ## Check code formatting (fails if unformatted)
182182
lint-license: ## Check license headers on all source files
183183
@$(BAZEL) run //tool/linter/licenseheader -- --check
184184

185+
lint-queue-shard: ## Check every table's primary key leads with the queue column
186+
@$(BAZEL) run //tool/linter/queueshard
187+
185188
local-submitqueue-clean: ## Stop and remove all local services, volumes, and images
186189
@echo "Cleaning all services and data..."
187190
@$(COMPOSE) -f $(COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down -v --rmi local

api/submitqueue/gateway/proto/gateway.proto

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ message CancelRequest {
7474
string sqid = 1;
7575
// Optional human-readable reason for the cancellation. Recorded for observability.
7676
string reason = 2;
77+
// Name of the queue processing the request. Required. A sqid is only resolvable within its own queue,
78+
// so naming a queue the request does not belong to is reported as not found.
79+
string queue = 3;
7780
}
7881

7982
// CancelResponse defines the response to a cancel request. Empty on success.
@@ -106,6 +109,9 @@ message RequestSummary {
106109
message GetRequestSummaryByIDRequest {
107110
// Globally unique identifier for the request.
108111
string sqid = 1;
112+
// Name of the queue processing the request. Required. A sqid is only resolvable within its own queue,
113+
// so naming a queue the request does not belong to is reported as not found.
114+
string queue = 2;
109115
}
110116

111117
// GetRequestSummaryByIDResponse contains the current materialized request view.
@@ -118,6 +124,9 @@ message GetRequestSummaryByIDResponse {
118124
message GetRequestSummaryByChangeURIRequest {
119125
// Exact change URI supplied in a Land request.
120126
string change_uri = 1;
127+
// Name of the queue to search. Required. Results are scoped to this queue: a change URI landed
128+
// into several queues is looked up one queue at a time.
129+
string queue = 2;
121130
}
122131

123132
// GetRequestSummaryByChangeURIResponse contains matching requests newest first.
@@ -152,6 +161,9 @@ message ListResponse {
152161
message GetRequestHistoryByIDRequest {
153162
// Globally unique identifier for the request.
154163
string sqid = 1;
164+
// Name of the queue processing the request. Required. A sqid is only resolvable within its own queue,
165+
// so naming a queue the request does not belong to is reported as not found.
166+
string queue = 2;
155167
}
156168

157169
// HistoryEvent is one retained append-only request-log event.
@@ -176,6 +188,9 @@ message GetRequestHistoryByIDResponse {
176188
message GetRequestHistoryByChangeURIRequest {
177189
// Exact change URI supplied in a Land request.
178190
string change_uri = 1;
191+
// Name of the queue to search. Required. Results are scoped to this queue: a change URI landed
192+
// into several queues is looked up one queue at a time.
193+
string queue = 2;
179194
}
180195

181196
// RequestHistory groups retained events for one request.

api/submitqueue/gateway/protopb/gateway.pb.go

Lines changed: 70 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)