Skip to content

Commit ca8e793

Browse files
committed
fix(webapp): warn about the read-gate case a shard can actually reach
A shard with no replicaUrl takes its own writer as its replica handle, so its reads go to its primary. That is the per-shard analogue of the existing legacy warning, and it is reachable today. The control-plane identity check stays as a regression guard, now marked as unreachable by construction: a non-aliased shard always gets a freshly built client. It exists so a future control-plane fallback for shards cannot silently route a shard's reads to another database.
1 parent a684713 commit ca8e793

3 files changed

Lines changed: 65 additions & 4 deletions

File tree

apps/webapp/app/db.server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ export const runOpsSplitReadEnabled: boolean = computeRunOpsSplitReadEnabled({
598598
// Empty unless RUN_OPS_SHARDS is configured.
599599
shardHandles: runOpsShardHandles.map((handle) => ({
600600
key: handle.key,
601+
writer: handle.writer,
601602
replica: handle.replica,
602603
// The DECLARED field, not client identity: an aliased shard shares its target's client by
603604
// reference, so identity comparison cannot tell the two apart.

apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ export function computeRunOpsSplitReadEnabled(args: {
99
hasNewUrl: boolean;
1010
hasLegacyUrl: boolean;
1111
/**
12-
* Gen-2 shard replica handles. Observability only: a non-distinct shard handle WARNS and never
13-
* changes the returned verdict. The distinctness sentinel already fail-closes the boot on the
14-
* same condition, and a gen-2 fault must not disable the proven gen-1 read fan-out as well.
12+
* Gen-2 shard handles. Observability only: a non-distinct shard handle WARNS and never changes the
13+
* returned verdict. A gen-2 fault must not disable the proven gen-1 read fan-out, and the
14+
* distinctness sentinel already fail-closes the boot when two stores share a database.
1515
*/
16-
shardHandles?: Array<{ key: string; replica: unknown; aliasOf?: "new" }>;
16+
shardHandles?: Array<{ key: string; writer?: unknown; replica: unknown; aliasOf?: "new" }>;
1717
logger?: { warn: (msg: string, meta?: Record<string, unknown>) => void };
1818
}): boolean {
1919
const newIsDistinctDedicatedClient =
@@ -34,6 +34,20 @@ export function computeRunOpsSplitReadEnabled(args: {
3434
// state and never a fault. Keyed on the declared field, not on object identity.
3535
for (const shard of args.shardHandles ?? []) {
3636
if (shard.aliasOf !== undefined) continue;
37+
38+
// A shard with no replica URL takes its own writer as its replica handle, so its reads go to
39+
// its primary. This is the per-shard analogue of the existing legacy-primary warning.
40+
if (shard.writer !== undefined && shard.replica === shard.writer) {
41+
args.logger?.warn(
42+
`run-ops shard ${shard.key} has no read replica handle; reads for that shard will hit the ` +
43+
"shard primary. Set the shard's replicaUrl to keep replica reads off its primary."
44+
);
45+
continue;
46+
}
47+
48+
// Unreachable by construction today: a non-aliased shard always gets a freshly built client.
49+
// Kept as a regression guard, so a future control-plane fallback for shards cannot silently
50+
// route a shard's reads to another database.
3751
if (
3852
shard.replica === args.controlPlaneWriter ||
3953
shard.replica === args.controlPlaneReplica ||

apps/webapp/test/runOpsSplitReadGate.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,52 @@ describe("computeRunOpsSplitReadEnabled shard handles", () => {
233233
).toBe(true);
234234
});
235235

236+
// The reachable case. A shard with no replicaUrl gets its own WRITER as its replica handle, so its
237+
// reads go to its primary. selectRunOpsTopology does exactly that (db.server.ts), which makes this
238+
// the per-shard analogue of the existing legacy "reads will hit the legacy primary" warning.
239+
it("warns when a shard has no distinct replica handle, so its reads hit its primary", () => {
240+
const warn = vi.fn();
241+
computeRunOpsSplitReadEnabled({
242+
...base,
243+
shardHandles: [{ key: "a", writer: shardA, replica: shardA }],
244+
logger: { warn },
245+
});
246+
expect(warn).toHaveBeenCalledTimes(1);
247+
expect(warn.mock.calls[0][0]).toMatch(/shard a/i);
248+
expect(warn.mock.calls[0][0]).toMatch(/primary/i);
249+
});
250+
251+
it("does not warn when a shard has its own distinct replica handle", () => {
252+
const warn = vi.fn();
253+
computeRunOpsSplitReadEnabled({
254+
...base,
255+
shardHandles: [{ key: "a", writer: shardA, replica: shardB }],
256+
logger: { warn },
257+
});
258+
expect(warn).not.toHaveBeenCalled();
259+
});
260+
261+
it("does NOT warn about primary reads for an aliased shard", () => {
262+
const warn = vi.fn();
263+
computeRunOpsSplitReadEnabled({
264+
...base,
265+
shardHandles: [
266+
{ key: "z", writer: dedicatedNew, replica: dedicatedNew, aliasOf: "new" as const },
267+
],
268+
logger: { warn },
269+
});
270+
expect(warn).not.toHaveBeenCalled();
271+
});
272+
273+
it("keeps the gen-1 verdict when a shard reads from its primary", () => {
274+
expect(
275+
computeRunOpsSplitReadEnabled({
276+
...base,
277+
shardHandles: [{ key: "a", writer: shardA, replica: shardA }],
278+
})
279+
).toBe(true);
280+
});
281+
236282
it("warns once per offending shard", () => {
237283
const warn = vi.fn();
238284
computeRunOpsSplitReadEnabled({

0 commit comments

Comments
 (0)