Skip to content

Commit 2c8a051

Browse files
committed
fix(webapp): retry the distinct-database probe before failing closed
The probe fails closed, so one store being briefly unreachable collapsed the deployment to single-DB and the boot interlock then refused the boot. With more than two stores configured that turns a transient blip on any one of them into a fleet-wide startup failure. Each target now gets a bounded number of attempts with a short backoff before the probe gives up. Failing closed is unchanged once the budget is exhausted: "distinct" stays a positive claim a failed probe cannot support. A genuine duplicate is a final answer and is never retried, so a misconfigured deployment still fails on the first pass. Retries are per target, so one slow store does not re-probe the stores that already answered.
1 parent 6973ca5 commit 2c8a051

2 files changed

Lines changed: 129 additions & 5 deletions

File tree

apps/webapp/app/v3/runOpsMigration/distinctDbSentinel.server.ts

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,48 @@ export async function probeControlPlaneCoresidency(
6464

6565
export type DistinctTarget = { id: string; url: string };
6666

67+
/** Injection seam for the retry tests: no containers, no real waiting. */
68+
export type DistinctProbeOptions = {
69+
logger?: { warn: (msg: string, meta?: Record<string, unknown>) => void };
70+
readFingerprint?: (url: string) => Promise<DatabaseFingerprint>;
71+
/** Total attempts per target, including the first. Bounded so boot latency stays bounded. */
72+
attempts?: number;
73+
sleep?: (ms: number) => Promise<void>;
74+
};
75+
76+
const DEFAULT_PROBE_ATTEMPTS = 3;
77+
const RETRY_BASE_DELAY_MS = 250;
78+
79+
const defaultSleep = (ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms));
80+
81+
/**
82+
* Read one fingerprint, retrying a bounded number of times.
83+
*
84+
* The probe fails CLOSED, and that must not change: "distinct" is a positive claim a failed probe
85+
* cannot support. But failing closed on the first blip means one shard being briefly unreachable
86+
* collapses the deployment to single-DB, and the boot interlock then refuses the boot for the whole
87+
* fleet. A transient error deserves a retry; a persistent one still fails closed, just later.
88+
*/
89+
async function readFingerprintWithRetry(
90+
url: string,
91+
read: (url: string) => Promise<DatabaseFingerprint>,
92+
attempts: number,
93+
sleep: (ms: number) => Promise<void>
94+
): Promise<DatabaseFingerprint> {
95+
let lastError: unknown;
96+
for (let attempt = 1; attempt <= attempts; attempt++) {
97+
try {
98+
return await read(url);
99+
} catch (error) {
100+
lastError = error;
101+
if (attempt < attempts) {
102+
await sleep(RETRY_BASE_DELAY_MS * attempt);
103+
}
104+
}
105+
}
106+
throw lastError;
107+
}
108+
67109
/**
68110
* Set uniqueness over every store that owns its own database. Fail-closed: a probe that cannot
69111
* answer returns NOT distinct, because "distinct" is a positive claim a failed probe cannot support.
@@ -77,14 +119,22 @@ export type DistinctTarget = { id: string; url: string };
77119
*/
78120
export async function probeDistinctStores(
79121
targets: DistinctTarget[],
80-
opts?: { logger?: { warn: (msg: string, meta?: Record<string, unknown>) => void } }
122+
opts?: DistinctProbeOptions
81123
): Promise<{ distinct: true } | { distinct: false; reason: string }> {
82124
if (targets.length < 2) {
83125
return { distinct: true };
84126
}
85127

128+
const read = opts?.readFingerprint ?? readDatabaseFingerprint;
129+
const attempts = opts?.attempts ?? DEFAULT_PROBE_ATTEMPTS;
130+
const sleep = opts?.sleep ?? defaultSleep;
131+
86132
try {
87-
const fingerprints = await Promise.all(targets.map((t) => readDatabaseFingerprint(t.url)));
133+
// Retry per TARGET, not around the whole set: one slow shard must not re-probe the stores that
134+
// already answered. A duplicate verdict below is final and is never retried.
135+
const fingerprints = await Promise.all(
136+
targets.map((t) => readFingerprintWithRetry(t.url, read, attempts, sleep))
137+
);
88138

89139
const seen = new Map<string, string>();
90140
for (const [index, target] of targets.entries()) {
@@ -104,7 +154,9 @@ export async function probeDistinctStores(
104154

105155
return { distinct: true };
106156
} catch (error) {
107-
const reason = `distinct-db sentinel probe failed; failing closed (single-DB). ${String(error)}`;
157+
const reason =
158+
`distinct-db sentinel probe failed after ${opts?.attempts ?? DEFAULT_PROBE_ATTEMPTS} ` +
159+
`attempt(s); failing closed (single-DB). ${String(error)}`;
108160
opts?.logger?.warn(reason, { error });
109161
return { distinct: false, reason };
110162
}
@@ -115,7 +167,7 @@ export async function probeDistinctStores(
115167
export async function probeDistinctDatabases(
116168
legacyUrl: string,
117169
newUrl: string,
118-
opts?: { logger?: { warn: (msg: string, meta?: Record<string, unknown>) => void } }
170+
opts?: DistinctProbeOptions
119171
): Promise<{ distinct: true } | { distinct: false; reason: string }> {
120172
return probeDistinctStores(
121173
[

apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { heteroPostgresTest } from "@internal/testcontainers";
22
import { PrismaClient } from "@trigger.dev/database";
3-
import { describe, expect, vi } from "vitest";
3+
import { describe, expect, it, vi } from "vitest";
44
import {
55
probeDistinctDatabases,
66
probeDistinctStores,
@@ -66,6 +66,78 @@ describe("probeDistinctDatabases", () => {
6666
);
6767
});
6868

69+
// A transient failure on ONE store must not refuse the boot fleet-wide. The probe fails closed,
70+
// so an unretried blip on any shard collapses the whole deployment to single-DB and the boot
71+
// interlock then throws. Retry a bounded number of times, then fail closed exactly as before.
72+
describe("probeDistinctStores bounded retry", () => {
73+
const fp = (sysId: string, db: string) => ({ systemIdentifier: sysId, databaseName: db });
74+
75+
it("recovers when a transient failure clears within the retry budget", async () => {
76+
let calls = 0;
77+
const readFingerprint = vi.fn(async (url: string) => {
78+
calls++;
79+
if (calls === 2) throw new Error("ECONNREFUSED");
80+
return fp("sys", url);
81+
});
82+
const result = await probeDistinctStores(
83+
[
84+
{ id: "new", url: "a" },
85+
{ id: "shard-a", url: "b" },
86+
],
87+
{ readFingerprint, attempts: 3, sleep: async () => {} }
88+
);
89+
expect(result).toEqual({ distinct: true });
90+
});
91+
92+
it("fails closed once the retry budget is exhausted", async () => {
93+
const readFingerprint = vi.fn(async () => {
94+
throw new Error("ECONNREFUSED");
95+
});
96+
const result = await probeDistinctStores(
97+
[
98+
{ id: "new", url: "a" },
99+
{ id: "shard-a", url: "b" },
100+
],
101+
{ readFingerprint, attempts: 3, sleep: async () => {} }
102+
);
103+
expect(result).toMatchObject({ distinct: false });
104+
});
105+
106+
it("bounds the attempts it makes", async () => {
107+
const readFingerprint = vi.fn(async () => {
108+
throw new Error("ECONNREFUSED");
109+
});
110+
await probeDistinctStores(
111+
[
112+
{ id: "a", url: "a" },
113+
{ id: "b", url: "b" },
114+
],
115+
{
116+
readFingerprint,
117+
attempts: 3,
118+
sleep: async () => {},
119+
}
120+
);
121+
// 2 targets x 3 attempts each, and no more.
122+
expect(readFingerprint).toHaveBeenCalledTimes(6);
123+
});
124+
125+
// A duplicate is a correct, final answer. Retrying it would delay every boot of a genuinely
126+
// misconfigured deployment for no benefit.
127+
it("does not retry a genuine duplicate", async () => {
128+
const readFingerprint = vi.fn(async () => fp("sys", "same"));
129+
const result = await probeDistinctStores(
130+
[
131+
{ id: "new", url: "a" },
132+
{ id: "shard-a", url: "b" },
133+
],
134+
{ readFingerprint, attempts: 3, sleep: async () => {} }
135+
);
136+
expect(result).toMatchObject({ distinct: false });
137+
expect(readFingerprint).toHaveBeenCalledTimes(2);
138+
});
139+
});
140+
69141
describe("probeDistinctStores (set uniqueness at N)", () => {
70142
heteroPostgresTest(
71143
"reports distinct for two separate physical clusters",

0 commit comments

Comments
 (0)