Skip to content

Commit 2941d71

Browse files
committed
fix(webapp): harden run-ops split idempotency + realtime batch reads
Follow-ups from review of the read-your-writes / idempotency work. Tests are included here for now and will be split into the stacked tests PR. - Claim TTL floor: the mollifier claim TTL was min(max, keyTTL), so a short customer idempotencyKeyTTL (1-2s) could expire the claim mid-pipeline and let a polling loser re-claim -> cross-DB duplicate under the split. Floor it at a pipeline bound (TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS, default 5), independent of the key TTL. - Publish compare-and-set: publishClaim returns the buffer CAS result; the trigger success path detects a no-op'd publish (claim expired, another claimant moved in) and logs it rather than silently treating its own run as canonical. - Reacquire exhaustion: reacquireClearedGlobalWinner failed OPEN on exhausting the bounded cleared-winner reacquires (or an unfindable winner), creating an unserialised run that can dual-create across DBs. Fail CLOSED with a retryable 503 so the SDK retry re-serialises (mirrors the timed_out branch). - Realtime batch route: findResource re-reads the owning primary on a replica miss, so the Electric ShapeStream consumer (self-hosters), which ignores the retryable-404 header, doesn't strand a live just-created batch on a permanent 404.
1 parent fb8fcfe commit 2941d71

13 files changed

Lines changed: 291 additions & 26 deletions

apps/webapp/app/env.server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,6 +1311,9 @@ const EnvironmentSchema = z
13111311
// claim TTL), how long a waiter blocks before timing out, and the
13121312
// waiter poll interval.
13131313
TRIGGER_MOLLIFIER_CLAIM_TTL_SECONDS: z.coerce.number().int().positive().default(30),
1314+
// Pipeline floor: the claim never shrinks below this even for a short customer key TTL, so it
1315+
// can't expire mid-pipeline and let a loser re-claim (cross-DB duplicate under the split).
1316+
TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS: z.coerce.number().int().positive().default(5),
13141317
TRIGGER_MOLLIFIER_CLAIM_WAIT_MS: z.coerce.number().int().positive().default(5_000),
13151318
TRIGGER_MOLLIFIER_CLAIM_POLL_MS: z.coerce.number().int().positive().default(25),
13161319

apps/webapp/app/routes/realtime.v1.batches.$batchId.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { z } from "zod";
22
import { getRequestAbortSignal } from "~/services/httpAsyncStorage.server";
33
import { resolveRealtimeStreamClient } from "~/services/realtime/resolveRealtimeStreamClient.server";
44
import { anyResource, createLoaderApiRoute } from "~/services/routeBuilders/apiBuilder.server";
5-
import { runStore } from "~/v3/runStore.server";
5+
import { resolveBatchTaskRunForRealtime } from "~/v3/realtime/resolveBatchForRealtime.server";
66

77
const ParamsSchema = z.object({
88
batchId: z.string(),
@@ -13,14 +13,13 @@ export const loader = createLoaderApiRoute(
1313
params: ParamsSchema,
1414
allowJWT: true,
1515
corsStrategy: "all",
16-
// A just-created batch may not yet have replicated to the read replica this client-less
17-
// findBatchTaskRunByFriendlyId lookup routes to; return a retryable 404 so the SDK retries through
18-
// replica lag rather than stranding a live batch on a permanent 404 (mirrors the run-get routes,
19-
// e.g. api.v3.runs.$runId).
16+
// A just-created batch may not yet have replicated to the read replica the client-less lookup uses.
17+
// shouldRetryNotFound stamps a retryable 404 for the zodfetch GET; the realtime resolver ALSO
18+
// re-reads the owning primary on a replica miss, so the Electric ShapeStream consumer (which ignores
19+
// x-should-retry) doesn't strand a live batch on a permanent 404. Mirrors the run-get routes.
2020
shouldRetryNotFound: true,
21-
findResource: (params, auth) => {
22-
return runStore.findBatchTaskRunByFriendlyId(params.batchId, auth.environment.id);
23-
},
21+
findResource: (params, auth) =>
22+
resolveBatchTaskRunForRealtime(params.batchId, auth.environment.id),
2423
authorization: {
2524
action: "read",
2625
// See sibling note in api.v1.batches.$batchId.ts — `{type: "runs"}`

apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { shouldIdempotencyKeyBeCleared } from "~/v3/taskStatus";
99
import { getMollifierBuffer } from "~/v3/mollifier/mollifierBuffer.server";
1010
import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.server";
1111
import { claimOrAwait, resetResolvedClaim } from "~/v3/mollifier/idempotencyClaim.server";
12+
import { computeClaimTtlSeconds } from "~/v3/mollifier/claimTtl";
1213
import { makeResolveMollifierFlag } from "~/v3/mollifier/mollifierGate.server";
1314
import { runStore } from "~/v3/runStore.server";
1415
import { runOpsLegacyPrisma, runOpsNewPrisma } from "~/db.server";
@@ -268,13 +269,12 @@ export class IdempotencyKeyConcern {
268269
| undefined) ?? null,
269270
}))));
270271
if (claimEligible) {
271-
const ttlSeconds = Math.max(
272-
1,
273-
Math.min(
274-
env.TRIGGER_MOLLIFIER_CLAIM_TTL_SECONDS,
275-
Math.ceil((idempotencyKeyExpiresAt.getTime() - Date.now()) / 1000)
276-
)
277-
);
272+
const ttlSeconds = computeClaimTtlSeconds({
273+
keyExpiresAt: idempotencyKeyExpiresAt,
274+
now: Date.now(),
275+
minTtlSeconds: env.TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS,
276+
maxTtlSeconds: env.TRIGGER_MOLLIFIER_CLAIM_TTL_SECONDS,
277+
});
278278
const outcome = await claimOrAwait({
279279
envId: request.environment.id,
280280
taskIdentifier: request.taskId,
@@ -519,8 +519,8 @@ export class IdempotencyKeyConcern {
519519
// (returning its claim to publish); the rest resolve to the fresh run. If a
520520
// re-claim resolves to ANOTHER cleared winner we advance and loop, bounded
521521
// by MAX_CLEARED_WINNER_REACQUIRES; on exhaustion (or an unfindable
522-
// resolution) we fall open to the create, matching the initial claim's
523-
// fail-open posture (PG unique index as backstop).
522+
// resolution) we fail CLOSED with a retryable 503 so the SDK retry re-serialises,
523+
// rather than fall open to an unserialised cross-DB create.
524524
private async reacquireClearedGlobalWinner(
525525
request: TriggerTaskRequest,
526526
parentStore: string | undefined,
@@ -584,7 +584,14 @@ export class IdempotencyKeyConcern {
584584
}
585585
staleRunId = outcome.runId;
586586
}
587-
return { isCached: false, idempotencyKey, idempotencyKeyExpiresAt };
587+
// Exhausted the bounded reacquires (or the winner was unfindable). Rather than fall through to an
588+
// UNSERIALISED create — which under global-scope-split can dual-create across DBs (the per-DB unique
589+
// index can't dedup cross-residency) — fail closed with a retryable 503 so the SDK retry re-serialises
590+
// through a fresh claim (mirrors the timed_out branch above).
591+
throw new ServiceValidationError(
592+
"Idempotency claim could not be re-serialised after repeated cleared winners",
593+
503
594+
);
588595
}
589596

590597
// Resolve a claim winner (a run friendlyId) across both split DBs. Classify

apps/webapp/app/runEngine/services/triggerTask.server.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,14 +752,25 @@ export class RunEngineTriggerTaskService {
752752
// Pipeline returned successfully — publish the claim if we held
753753
// one. Waiters polling for our key resolve to this runId.
754754
if (idempotencyClaim && result?.run?.friendlyId) {
755-
await publishMollifierClaim({
755+
const published = await publishMollifierClaim({
756756
envId: idempotencyClaim.envId,
757757
taskIdentifier: idempotencyClaim.taskIdentifier,
758758
idempotencyKey: idempotencyClaim.idempotencyKey,
759759
token: idempotencyClaim.token,
760760
runId: result.run.friendlyId,
761761
ttlSeconds: env.TRIGGER_MOLLIFIER_CLAIM_TTL_SECONDS,
762762
});
763+
if (!published) {
764+
// Our claim expired mid-pipeline and another claimant took it, so this publish no-op'd: a
765+
// different run is now canonical for this key while we return ours (a cross-DB dup under the
766+
// split). Rare now the claim TTL is floored (C1); surfaced for monitoring pending auto-
767+
// convergence (re-resolve the current winner + cancel this orphan).
768+
logger.warn("mollifier claim publish no-op'd; winner lost the claim mid-pipeline", {
769+
envId: idempotencyClaim.envId,
770+
taskIdentifier: idempotencyClaim.taskIdentifier,
771+
runId: result.run.friendlyId,
772+
});
773+
}
763774
}
764775
return result;
765776
} catch (err) {
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// The claim is a serialization lock that must outlive the winner's create-and-publish pipeline. A short
2+
// customer key TTL must NOT shrink it below a pipeline floor (else the claim expires mid-pipeline and a
3+
// polling loser re-claims → cross-DB duplicate). Floor at `minTtlSeconds` independent of key TTL; cap at max.
4+
export function computeClaimTtlSeconds(input: {
5+
keyExpiresAt: Date;
6+
now: number;
7+
minTtlSeconds: number;
8+
maxTtlSeconds: number;
9+
}): number {
10+
const keyTtlSeconds = Math.ceil((input.keyExpiresAt.getTime() - input.now) / 1000);
11+
return Math.min(input.maxTtlSeconds, Math.max(input.minTtlSeconds, keyTtlSeconds));
12+
}

apps/webapp/app/v3/mollifier/idempotencyClaim.server.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,12 +163,14 @@ export async function publishClaim(input: {
163163
runId: string;
164164
ttlSeconds?: number;
165165
buffer?: MollifierBuffer | null;
166-
}): Promise<void> {
166+
}): Promise<boolean> {
167167
const buffer = input.buffer === undefined ? getMollifierBuffer() : input.buffer;
168-
if (!buffer) return;
168+
if (!buffer) return true;
169169
const ttlSeconds = input.ttlSeconds ?? DEFAULT_CLAIM_TTL_SECONDS;
170170
try {
171-
await buffer.publishClaim({
171+
// false = compare-and-set no-op: a stale claimant (our TTL expired) already moved in, so our
172+
// publish did NOT set the winner. The caller decides how to converge.
173+
return await buffer.publishClaim({
172174
envId: input.envId,
173175
taskIdentifier: input.taskIdentifier,
174176
idempotencyKey: input.idempotencyKey,
@@ -182,6 +184,8 @@ export async function publishClaim(input: {
182184
taskIdentifier: input.taskIdentifier,
183185
err: err instanceof Error ? err.message : String(err),
184186
});
187+
// Unknown publish state (transient error) — the claim TTL is the safety net; don't signal a no-op.
188+
return true;
185189
}
186190
}
187191

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { prisma } from "~/db.server";
2+
import { runStore } from "~/v3/runStore.server";
3+
4+
type BatchStore = Pick<typeof runStore, "findBatchTaskRunByFriendlyId">;
5+
6+
// The realtime batch route reads the batch client-less (replica), which can miss a just-created batch
7+
// under replica lag. `shouldRetryNotFound` covers the zodfetch GET, but the Electric ShapeStream
8+
// consumer (self-hosters) ignores `x-should-retry`, so re-read the owning primary on a miss — passing a
9+
// non-replica writer flips each store leg to its own primary — to avoid a permanent 404.
10+
export function resolveBatchTaskRunForRealtime(
11+
friendlyId: string,
12+
environmentId: string,
13+
deps?: { store?: BatchStore; writer?: unknown }
14+
) {
15+
const store = deps?.store ?? runStore;
16+
const writer = deps?.writer ?? prisma;
17+
return store
18+
.findBatchTaskRunByFriendlyId(friendlyId, environmentId)
19+
.then(
20+
(onReplica) =>
21+
onReplica ??
22+
store.findBatchTaskRunByFriendlyId(friendlyId, environmentId, undefined, writer as never)
23+
);
24+
}

apps/webapp/test/claimTtl.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, expect, it } from "vitest";
2+
import { computeClaimTtlSeconds } from "~/v3/mollifier/claimTtl";
3+
4+
// The mollifier idempotency claim is a serialization lock that must outlive the winner's
5+
// create-and-publish pipeline. If its TTL is derived from the customer's key TTL alone, a short key
6+
// TTL expires the claim mid-pipeline and a polling loser re-claims → a second creator → cross-DB dup.
7+
describe("computeClaimTtlSeconds", () => {
8+
const now = Date.parse("2026-01-01T00:00:00.000Z");
9+
const minTtlSeconds = 5;
10+
const maxTtlSeconds = 30;
11+
12+
it("floors a short key TTL at the pipeline minimum (claim can't expire mid-pipeline)", () => {
13+
expect(
14+
computeClaimTtlSeconds({
15+
keyExpiresAt: new Date(now + 2_000),
16+
now,
17+
minTtlSeconds,
18+
maxTtlSeconds,
19+
})
20+
).toBe(5);
21+
});
22+
23+
it("caps a long key TTL at the maximum", () => {
24+
expect(
25+
computeClaimTtlSeconds({
26+
keyExpiresAt: new Date(now + 3_600_000),
27+
now,
28+
minTtlSeconds,
29+
maxTtlSeconds,
30+
})
31+
).toBe(30);
32+
});
33+
34+
it("uses the key TTL when it sits between the floor and the cap", () => {
35+
expect(
36+
computeClaimTtlSeconds({
37+
keyExpiresAt: new Date(now + 12_000),
38+
now,
39+
minTtlSeconds,
40+
maxTtlSeconds,
41+
})
42+
).toBe(12);
43+
});
44+
45+
it("floors an already-expired key at the minimum", () => {
46+
expect(
47+
computeClaimTtlSeconds({
48+
keyExpiresAt: new Date(now - 1_000),
49+
now,
50+
minTtlSeconds,
51+
maxTtlSeconds,
52+
})
53+
).toBe(5);
54+
});
55+
});

apps/webapp/test/mollifierClaimResolution.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,30 @@ describe("IdempotencyKeyConcern · claim resolution", () => {
156156
h.orgFlag = true; // restore for any later tests in this file
157157
}
158158
});
159+
160+
it("floors the claim TTL for a short idempotency-key TTL so the claim can't expire mid-pipeline", async () => {
161+
// A 2s customer key TTL must NOT shrink the claim below the pipeline floor — else the claim expires
162+
// while the winner is still creating and a polling loser re-claims (cross-DB dup under the split).
163+
// Capture the ttlSeconds the concern hands the buffer's claim.
164+
let capturedTtl: number | undefined;
165+
h.buffer = {
166+
claimIdempotency: vi.fn(async (input: { ttlSeconds: number }) => {
167+
capturedTtl = input.ttlSeconds;
168+
return { kind: "claimed" as const };
169+
}),
170+
lookupIdempotency: vi.fn(async () => null),
171+
} as unknown as MollifierBuffer;
172+
173+
const findFirst = vi.fn(async () => null);
174+
const concern = makeConcern({ findFirst });
175+
176+
const request = makeRequest();
177+
(request.body.options as { idempotencyKeyTTL?: string }).idempotencyKeyTTL = "2s";
178+
179+
const result = await concern.handleTriggerRequest(request, undefined);
180+
181+
expect(result.isCached).toBe(false);
182+
// Floored at TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS (default 5), NOT the ~2s key TTL.
183+
expect(capturedTtl).toBeGreaterThanOrEqual(5);
184+
});
159185
});

apps/webapp/test/mollifierIdempotencyClaim.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,21 +181,21 @@ describe("publishClaim", () => {
181181
expect(buffer.publishClaim).toHaveBeenCalledOnce();
182182
});
183183

184-
it("no-op when buffer is null", async () => {
184+
it("no buffer → true (nothing to converge, not a no-op'd publish)", async () => {
185185
await expect(
186186
publishClaim({ ...baseInput, token: "owner-token", runId: "run_X", buffer: null })
187-
).resolves.toBeUndefined();
187+
).resolves.toBe(true);
188188
});
189189

190-
it("swallows errors so trigger pipeline isn't broken by Redis hiccups", async () => {
190+
it("swallows errors so trigger pipeline isn't broken by Redis hiccups (returns true, unknown state)", async () => {
191191
const buffer = {
192192
publishClaim: vi.fn(async () => {
193193
throw new Error("ECONNREFUSED");
194194
}),
195195
} as unknown as MollifierBuffer;
196196
await expect(
197197
publishClaim({ ...baseInput, token: "owner-token", runId: "run_X", buffer })
198-
).resolves.toBeUndefined();
198+
).resolves.toBe(true);
199199
});
200200
});
201201

0 commit comments

Comments
 (0)