You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 920892b
Browse filesBrowse the repository at this point in the historyBrowse files
feat(webapp,run-store): gen-2 shard arms in read-through and idempotency (#4781)
Gives read-through and idempotency their gen-2 shard arms, so an id that
names its own shard is read there and nowhere else.
#4764 has landed, so this now targets `main` directly and no longer
depends on an unmerged branch. It builds on what that PR supplied:
`resolveShard`, `runOpsShardHandles` and the keyed router.
TRI-13431
## What changes
**Read-through routes by `resolveShard`, not by the binary residency
classifier.** A gen-2 id reads its own shard's replica once and probes
no other store. A gen-1 v1 id still reads new only.
**Callers now declare `idKind`.** A cuid gives no way to tell a run id
from a waitpoint id, and the two must route differently:
- a legacy-classified **run** id reads the legacy replica only — there
is no cuid run migration, so the new-store probe cannot find it;
- a cuid **waitpoint** keeps the new-first pair probe, which is
load-bearing because a cuid waitpoint can be co-located with its run on
the new store.
There is no default, because a default would pick one of those arms
silently. The field `runId` is renamed to `id`, since it carried both
kinds already.
**`ReadThroughResult` carries `found`.** `source` is an open-ended union
once shards exist, so a consumer testing found-ness by listing the hit
sources reads a gen-2 hit as a miss. One consumer did exactly that.
Discriminating on `found` makes that class of bug a compile error rather
than something a reviewer has to spot.
**Idempotency resolves its client through one shard-keyed map.** Both
call sites go through `clientForShardKey`, so they cannot disagree about
which store owns an id. An absent key takes an explicit logged branch to
the fallback, not a silent legacy default. The `classify` seam is
retyped to return a `ShardKey`: `Residency` (`"NEW"`) and the reserved
shard keys (`"new"`) differ only by case, and `ShardKey` collapses to
`string`, so the compiler would not have caught feeding one into the
other.
The dead `isMigrated` branch is deleted. Nothing implemented it, and the
one production comment recorded that omitting it was deliberate.
**`PostgresRunStore._residency` widens to `ShardKey`.** Still unused;
the store stays unaware of its siblings.
## Two behaviour fixes found while doing the above
**An unconfigured shard key logs and returns not-found instead of
throwing.** The waitpoint route takes the id from a URL parameter, and
any base32hex core plus `[a-z0-9]` plus `"2"` parses as gen-2. The route
turns a throw into a 500, so throwing here would let any authenticated
client generate 500s and error logs by guessing shard chars, of which
there are 36. An error-logged not-found is neither silent nor a
misroute. Throwing stays correct on the router path, where ids are
minted rather than received.
**The two cross-seam batch hydration sites were gen-2 blind.**
`hydrateRunsAcrossSeam` and `ApiBatchResultsPresenter` classified with
the binary `ownerEngine`, so a gen-2 run id joined the gen-1 `new`
group, missed there, and — classifying dedicated-family — never reached
the legacy probe either. The id was dropped from a bulk-action page and
from batch results with no error. Both now partition ids by shard key
and read each configured shard once.
Also: a gen-2 waitpoint that missed its shard replica fell back to the
gen-1 new writer, a different database, silently disabling
read-your-writes for the freshly minted token that fallback exists to
serve. It now falls back to its own shard's writer.
## Merge safety
Inert while `RUN_OPS_SHARDS` is unset: the shard maps are empty, so
every gen-2 arm is unreachable, and gen-2 minting is not live yet.
The one live change is the gen-1 run arm, and it removes work rather
than adding it. `RoutingRunStore.findRun` never forwards the caller's
client object — it routes by id and reads only the client's presence and
replica brand — so `readRunForEvent`'s "new" closure already resolved a
legacy-classified run id to the legacy store. The arm removes a
duplicated read of the legacy replica. A test pins this, because a
future caller passing a raw client and a run id would lose the
pre-cutover 27-char case, which is new-resident but classifies legacy.
## Testing
14 tests added, testcontainers throughout, no mocks. 22 affected test
files pass; typecheck, lint, format and knip are clean.
Both arms were verified by neutralising them and confirming the new
tests fail. The batch-results test needed rewriting after that check:
the first version passed with the fix neutralised, because it used one
container as both the gen-1 new client and the shard replica, so it was
not testing what it claimed.
Note for review: run testcontainer suites in small batches. Sixteen at
once starves Docker and everything times out at 60 seconds.
The run-ops legacy-guard baseline is refreshed in its own commit. The
baseline is keyed by line number, so partitioning the batch-results read
shifted four pre-existing entries and added one. Baselined violations in
that file go from four to five, all reads; the new one is the shard read
beside two gen-1 reads already there.
No changeset and no `.server-changes` entry: a user notices nothing
while the flag is unset.
0 commit comments