diff --git a/apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts b/apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts index 67ef45ebd27..abe04948083 100644 --- a/apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts @@ -1,5 +1,5 @@ import type { BatchTaskRunExecutionResult } from "@trigger.dev/core/v3"; -import { ownerEngine } from "@trigger.dev/core/v3/isomorphic"; +import { resolveShard, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import { $replica, type PrismaClientOrTransaction, @@ -13,6 +13,8 @@ import { runStore as defaultRunStore } from "~/v3/runStore.server"; import { BasePresenter } from "./basePresenter.server"; import { boundedIn } from "@trigger.dev/database"; +import { runOpsShardReplicas } from "~/v3/runOpsMigration/shardHandles.server"; +import { logger } from "~/services/logger.server"; /** * Run-ops read-through wiring. All optional; absent (or `splitEnabled` falsy) collapses `call` to * passthrough. `legacyReplica` is a READ REPLICA handle only — there is NO legacy-primary field. @@ -21,6 +23,8 @@ type ApiBatchResultsReadThroughDeps = { splitEnabled?: boolean; newClient?: PrismaReplicaClient; legacyReplica?: PrismaReplicaClient; + /** Gen-2 shard replicas by shard char; empty (RUN_OPS_SHARDS unset) keeps today's behaviour. */ + shardReplicas?: ReadonlyMap; isPastRetention?: (runId: string) => boolean; }; @@ -181,16 +185,57 @@ export class ApiBatchResultsPresenter extends BasePresenter { const taskRunIds = batchRun.items.map((item) => item.taskRunId); - const newRows = (await newClient.taskRun.findMany({ - where: { id: { in: boundedIn(taskRunIds) } }, - select: memberRunSelect, - })) as TaskRunWithAttempts[]; + // A gen-2 id is directly routable to its own shard, so it must not join the gen-1 read: + // it would miss there, and (being dedicated-family) never reach the legacy probe either. + const shardReplicas = this.readThrough?.shardReplicas ?? runOpsShardReplicas; + const genOneIds: string[] = []; + const idsByShard = new Map(); + for (const id of taskRunIds) { + const shardKey = resolveShard(id); + if (shardKey === "new" || shardKey === "legacy") { + genOneIds.push(id); + } else if (shardReplicas.has(shardKey)) { + const group = idsByShard.get(shardKey); + group ? group.push(id) : idsByShard.set(shardKey, [id]); + } else { + // Not routable and not a gen-1 shape. A gen-1 store is the wrong database, and a + // dedicated-family id never reaches the legacy probe, so falling back there would + // drop the member silently. Drop it loudly instead. + logger.error("ApiBatchResultsPresenter: gen-2 member on an unconfigured shard key", { + runId: id, + shardKey, + configured: [...shardReplicas.keys()], + }); + } + } + + const newRows = ( + genOneIds.length > 0 + ? ((await newClient.taskRun.findMany({ + where: { id: { in: boundedIn(genOneIds) } }, + select: memberRunSelect, + })) as TaskRunWithAttempts[]) + : [] + ).concat( + ( + await Promise.all( + [...idsByShard.entries()].map( + async ([shardKey, ids]) => + (await shardReplicas.get(shardKey)!.taskRun.findMany({ + where: { id: { in: boundedIn(ids) } }, + select: memberRunSelect, + })) as TaskRunWithAttempts[] + ) + ) + ).flat() + ); const runsById = new Map(newRows.map((run) => [run.id, run])); - // A run-ops id can only live on NEW, so only misses that AREN'T run-ops-shaped are candidates - // for the legacy probe — mirrors readThroughRun's per-id "NEW residency skips legacy" rule. - const legacyCandidateIds = taskRunIds.filter( - (id) => !runsById.has(id) && ownerEngine(id) !== "NEW" + // A dedicated-family id (gen-1 v1 or gen-2) can only live on its own store, so only + // misses that AREN'T dedicated-shaped are candidates for the legacy probe — mirrors + // readThroughRun's per-id "dedicated residency skips legacy" rule. + const legacyCandidateIds = genOneIds.filter( + (id) => !runsById.has(id) && resolveShard(id) === "legacy" ); if (legacyCandidateIds.length > 0) { const legacyRows = (await legacyReplica.taskRun.findMany({ diff --git a/apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts b/apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts index ea1ebab0679..566ffc05876 100644 --- a/apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts +++ b/apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts @@ -38,7 +38,14 @@ const { action } = createActionApiRoute( }); if (!waitpoint) { - throw json({ error: "Waitpoint not found" }, { status: 404 }); + // Retryable: a miss here can be replica lag. resolveWaitpointThroughReadThrough + // deliberately does not read the legacy primary, so it relies on the caller retrying. + // A plain 404 is not retried by the SDK, which would turn a transient miss into a + // permanent failure. + throw json( + { error: "Waitpoint not found" }, + { status: 404, headers: { "x-should-retry": "true" } } + ); } const _result = await engine.blockRunWithWaitpoint({ @@ -55,6 +62,11 @@ const { action } = createActionApiRoute( { status: 200 } ); } catch (error) { + // A Response thrown inside the try is a deliberate status (the 404 above), not a + // failure. Re-throw it untouched, or every intentional 4xx here becomes a 500. + if (error instanceof Response) { + throw error; + } logger.error("Failed to wait for waitpoint", { runId, waitpointId, error }); throw json({ error: "Failed to wait for waitpoint token" }, { status: 500 }); } diff --git a/apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts b/apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts index f6696865e94..dfa2d4f5845 100644 --- a/apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts +++ b/apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts @@ -1,4 +1,4 @@ -import { ownerEngine, RunId } from "@trigger.dev/core/v3/isomorphic"; +import { resolveShard, RunId, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import type { PrismaClientOrTransaction, TaskRun, Waitpoint } from "@trigger.dev/database"; import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; @@ -13,9 +13,10 @@ import { computeClaimTtlSeconds } from "~/v3/mollifier/claimTtl"; import { makeResolveMollifierFlag } from "~/v3/mollifier/mollifierGate.server"; import { runStore } from "~/v3/runStore.server"; import { runOpsLegacyPrisma, runOpsNewPrisma } from "~/db.server"; +import { runOpsShardWriters } from "~/v3/runOpsMigration/shardHandles.server"; import { isSplitEnabled } from "~/v3/runOpsMigration/splitMode.server"; import { resolveRunIdMintKind } from "~/v3/engineVersion.server"; -import { resolveIdempotencyDedupClient } from "./idempotencyResidency.server"; +import { clientForShardKey, resolveIdempotencyDedupClient } from "./idempotencyResidency.server"; import type { TraceEventConcern, TriggerTaskRequest } from "../types"; // In-memory per-org mollifier-enabled check, shared with `evaluateGate` @@ -32,6 +33,16 @@ const resolveOrgMollifierFlag = makeResolveMollifierFlag(); // PG's unique index as the backstop. const MAX_CLEARED_WINNER_REACQUIRES = 5; +// The store that owns a shard key. A function, not a map: the handles are module constants and +// `runOpsShardWriters` is already keyed, so a second structure would add an allocation and, if +// memoised, mutable module state. Reading them lazily also keeps this module importable by +// triggerTask under a `~/db.server` mock that omits them. +function idempotencyClientFor(shardKey: ShardKey): PrismaClientOrTransaction | undefined { + if (shardKey === "legacy") return runOpsLegacyPrisma; + if (shardKey === "new") return runOpsNewPrisma; + return runOpsShardWriters.get(shardKey); +} + // Claim ownership context returned to the caller when the // IdempotencyKeyConcern won a pre-gate claim. Caller MUST publish the // winning runId on pipeline success (`publishClaim`) or release the @@ -172,12 +183,9 @@ export class IdempotencyKeyConcern { { isSplitEnabled, fallbackClient: this.prisma, - newClient: runOpsNewPrisma, - legacyClient: runOpsLegacyPrisma, + clientFor: idempotencyClientFor, resolveMintKind: resolveRunIdMintKind, - // `isMigrated` is intentionally omitted: until a child of a swept - // legacy-id parent can be born on the new DB, the swept-marker override - // would never change the answer, so a child routes by parent id-shape. + logger, } ); @@ -640,12 +648,15 @@ export class IdempotencyKeyConcern { } catch { return null; } - let client: PrismaClientOrTransaction; - try { - client = ownerEngine(internalId) === "NEW" ? runOpsNewPrisma : runOpsLegacyPrisma; - } catch { - client = this.prisma; - } + // The routing store routes by id and never forwards this object, so its identity only + // signals read-your-writes. Resolving it through the shard map keeps the two idempotency + // call sites in agreement and stops this reading as gen-2-unaware. + const client = clientForShardKey( + resolveShard(internalId), + idempotencyClientFor, + this.prisma, + logger + ); return runStore.findRun( { id: internalId, runtimeEnvironmentId: environmentId }, { include: { associatedWaitpoint: true } }, diff --git a/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.test.ts b/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.test.ts index 39b806a0f71..976a4ffd784 100644 --- a/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.test.ts +++ b/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { RunId } from "@trigger.dev/core/v3/isomorphic"; import { + clientForShardKey, resolveIdempotencyDedupClient, type ResolveIdempotencyClientDeps, } from "./idempotencyResidency.server"; @@ -9,20 +10,30 @@ import { const FALLBACK = { __tag: "fallback" } as never; const NEW_CLIENT = { __tag: "new" } as never; const LEGACY_CLIENT = { __tag: "legacy" } as never; +const SHARD_A_CLIENT = { __tag: "shard-a" } as never; + +function clientMap() { + return new Map([ + ["new", NEW_CLIENT], + ["legacy", LEGACY_CLIENT], + ["a", SHARD_A_CLIENT], + ]); +} function makeDeps(over: Partial): ResolveIdempotencyClientDeps { return { isSplitEnabled: async () => true, fallbackClient: FALLBACK, - newClient: NEW_CLIENT, - legacyClient: LEGACY_CLIENT, + clientFor: (key) => clientMap().get(key), resolveMintKind: async () => "runOpsId", + // Kept as an injected seam: the real resolveShard is total, so only an injected + // classifier can exercise the throw-to-fallback arm below. classify: (id) => { - if (id.length === 26 && id[25] === "1") return "NEW"; - if (id.length === 25) return "LEGACY"; + if (id.length === 26 && id[25] === "2") return id[24]!; + if (id.length === 26 && id[25] === "1") return "new"; + if (id.length === 25) return "legacy"; throw new Error(`unclassifiable: ${id.length}`); }, - isMigrated: undefined, ...over, }; } @@ -72,29 +83,51 @@ describe("resolveIdempotencyDedupClient", () => { expect(client).toBe(LEGACY_CLIENT); }); - it("routes a swept (migrated) cuid-parent child to the NEW client", async () => { - const cuidParent = RunId.toFriendlyId("c".repeat(25)); + it("falls back to the fallback client when a present parent id is unclassifiable", async () => { const client = await resolveIdempotencyDedupClient( - { environmentForMint: env, parentRunFriendlyId: cuidParent }, - makeDeps({ isMigrated: async () => true }) + { environmentForMint: env, parentRunFriendlyId: "run_not-a-valid-length" }, + makeDeps({}) ); - expect(client).toBe(NEW_CLIENT); + expect(client).toBe(FALLBACK); }); - it("routes a non-migrated cuid-parent child to the LEGACY client even when isMigrated is provided", async () => { - const cuidParent = RunId.toFriendlyId("d".repeat(25)); + it("routes a child to its OWN SHARD client when the parent is a gen-2 id", async () => { + const genTwoParent = RunId.toFriendlyId("e".repeat(24) + "a2"); const client = await resolveIdempotencyDedupClient( - { environmentForMint: env, parentRunFriendlyId: cuidParent }, - makeDeps({ isMigrated: async () => false }) + { environmentForMint: env, parentRunFriendlyId: genTwoParent }, + makeDeps({ resolveMintKind: async () => "cuid" }) // mint flag must NOT win for a child ); - expect(client).toBe(LEGACY_CLIENT); + expect(client).toBe(SHARD_A_CLIENT); }); - it("falls back to the fallback client when a present parent id is unclassifiable", async () => { + it("falls back and logs when a gen-2 parent names an unconfigured shard key", async () => { + const errors: unknown[] = []; + const genTwoParent = RunId.toFriendlyId("f".repeat(24) + "z2"); const client = await resolveIdempotencyDedupClient( - { environmentForMint: env, parentRunFriendlyId: "run_not-a-valid-length" }, - makeDeps({}) + { environmentForMint: env, parentRunFriendlyId: genTwoParent }, + makeDeps({ logger: { error: (_m, meta) => errors.push(meta) } }) ); expect(client).toBe(FALLBACK); + expect(errors).toHaveLength(1); + }); +}); + +describe("clientForShardKey", () => { + it("selects the same client the map holds for each reserved key and shard key", () => { + const clients = clientMap(); + const clientFor = (key: string) => clients.get(key); + expect(clientForShardKey("new", clientFor, FALLBACK)).toBe(NEW_CLIENT); + expect(clientForShardKey("legacy", clientFor, FALLBACK)).toBe(LEGACY_CLIENT); + expect(clientForShardKey("a", clientFor, FALLBACK)).toBe(SHARD_A_CLIENT); + }); + + it("returns the fallback and logs for a key the map does not hold", () => { + const errors: unknown[] = []; + const map = clientMap(); + const client = clientForShardKey("z", (key) => map.get(key), FALLBACK, { + error: (_m, meta) => errors.push(meta), + }); + expect(client).toBe(FALLBACK); + expect(errors).toHaveLength(1); }); }); diff --git a/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.ts b/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.ts index 86f1435654b..f2a731e61ca 100644 --- a/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.ts +++ b/apps/webapp/app/runEngine/concerns/idempotencyResidency.server.ts @@ -1,22 +1,44 @@ -import { ownerEngine, RunId, type Residency } from "@trigger.dev/core/v3/isomorphic"; +import { resolveShard, RunId, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import type { PrismaClientOrTransaction } from "@trigger.dev/database"; type MintKind = "cuid" | "runOpsId"; +type Logger = { error: (message: string, meta?: Record) => void }; + export type ResolveIdempotencyClientDeps = { isSplitEnabled: () => Promise; fallbackClient: PrismaClientOrTransaction; - newClient: PrismaClientOrTransaction; - legacyClient: PrismaClientOrTransaction; + /** The store that owns a shard key: the reserved `legacy`/`new`, or a gen-2 shard. */ + clientFor: (shardKey: ShardKey) => PrismaClientOrTransaction | undefined; resolveMintKind: (environment: { organizationId: string; id: string; orgFeatureFlags?: unknown; }) => Promise; - classify?: (id: string) => Residency; - isMigrated?: (id: string) => Promise; + classify?: (id: string) => ShardKey; + logger?: Logger; }; +/** + * The one place an id becomes a client. `ShardKey` collapses to `string`, so the compiler + * cannot catch a wrong key here — an absent key takes an explicit logged branch to the + * fallback rather than a silent `?? legacy`. The configured set is not repeated in the log: + * boot already prints the shard table. + */ +export function clientForShardKey( + shardKey: ShardKey, + clientFor: (shardKey: ShardKey) => PrismaClientOrTransaction | undefined, + fallback: PrismaClientOrTransaction, + logger?: Logger +): PrismaClientOrTransaction { + const client = clientFor(shardKey); + if (client === undefined) { + logger?.error("idempotency: no client configured for shard key", { shardKey }); + return fallback; + } + return client; +} + export async function resolveIdempotencyDedupClient( args: { environmentForMint: { organizationId: string; id: string; orgFeatureFlags?: unknown }; @@ -28,9 +50,9 @@ export async function resolveIdempotencyDedupClient( return deps.fallbackClient; } - const classify = deps.classify ?? ownerEngine; - const clientFor = (residency: Residency): PrismaClientOrTransaction => - residency === "NEW" ? deps.newClient : deps.legacyClient; + const classify = deps.classify ?? resolveShard; + const clientFor = (shardKey: ShardKey): PrismaClientOrTransaction => + clientForShardKey(shardKey, deps.clientFor, deps.fallbackClient, deps.logger); if (args.parentRunFriendlyId) { let parentInternalId: string; @@ -39,18 +61,18 @@ export async function resolveIdempotencyDedupClient( } catch { return deps.fallbackClient; } - let residency: Residency; + let shardKey: ShardKey; try { - residency = classify(parentInternalId); + shardKey = classify(parentInternalId); } catch { return deps.fallbackClient; } - if (residency === "LEGACY" && deps.isMigrated && (await deps.isMigrated(parentInternalId))) { - return deps.newClient; - } - return clientFor(residency); + return clientFor(shardKey); } + // Mint kind, not an id: there is no shard to decode, so this keeps resolving to the + // gen-1 pair exactly as before. Which shard a gen-2 env mints into is the mint layer's + // decision, and this client is a read-your-writes signal rather than a correctness gate. const kind = await deps.resolveMintKind(args.environmentForMint); - return clientFor(kind === "runOpsId" ? "NEW" : "LEGACY"); + return clientFor(kind === "runOpsId" ? "new" : "legacy"); } diff --git a/apps/webapp/app/runEngine/concerns/resolveWaitpointThroughReadThrough.server.ts b/apps/webapp/app/runEngine/concerns/resolveWaitpointThroughReadThrough.server.ts index ec5adc13a6c..b1c7a2bd05d 100644 --- a/apps/webapp/app/runEngine/concerns/resolveWaitpointThroughReadThrough.server.ts +++ b/apps/webapp/app/runEngine/concerns/resolveWaitpointThroughReadThrough.server.ts @@ -1,3 +1,4 @@ +import { resolveShard, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import type { PrismaReplicaClient } from "~/db.server"; import { runOpsLegacyReplica as defaultLegacyReplica, @@ -5,12 +6,18 @@ import { runOpsNewReplica as defaultNewClient, runOpsSplitReadEnabled as defaultSplitReadEnabled, } from "~/db.server"; +import { + runOpsShardReplicas as defaultShardReplicas, + runOpsShardWriters as defaultShardWriters, +} from "~/v3/runOpsMigration/shardHandles.server"; import { readThroughRun } from "~/v3/runOpsMigration/readThrough.server"; type ResolveWaitpointDeps = { newClient?: PrismaReplicaClient; legacyReplica?: PrismaReplicaClient; newPrimary?: PrismaReplicaClient; + shardReplicas?: ReadonlyMap; + shardWriters?: ReadonlyMap; splitEnabled?: boolean; isPastRetention?: (id: string) => boolean; }; @@ -21,6 +28,8 @@ export type ResolveWaitpointReadThroughDefaults = { newClient: PrismaReplicaClient; legacyReplica: PrismaReplicaClient; newPrimary: PrismaReplicaClient; + shardReplicas: ReadonlyMap; + shardWriters: ReadonlyMap; splitEnabled: boolean; }; @@ -28,6 +37,8 @@ const productionDefaults: ResolveWaitpointReadThroughDefaults = { newClient: defaultNewClient, legacyReplica: defaultLegacyReplica, newPrimary: defaultNewPrimary as unknown as PrismaReplicaClient, + shardReplicas: defaultShardReplicas, + shardWriters: defaultShardWriters as unknown as ReadonlyMap, splitEnabled: defaultSplitReadEnabled, }; @@ -43,7 +54,8 @@ export async function resolveWaitpointThroughReadThrough(opts: { const splitEnabled = opts.deps?.splitEnabled ?? defaults.splitEnabled; const result = await readThroughRun({ - runId: opts.waitpointId, + id: opts.waitpointId, + idKind: "waitpoint", environmentId: opts.environmentId, readNew: (client) => opts.read(client), readLegacy: (replica) => opts.read(replica), @@ -51,22 +63,31 @@ export async function resolveWaitpointThroughReadThrough(opts: { splitEnabled, newClient: opts.deps?.newClient ?? defaults.newClient, legacyReplica: opts.deps?.legacyReplica ?? defaults.legacyReplica, + shardReplicas: opts.deps?.shardReplicas ?? defaults.shardReplicas, isPastRetention: opts.deps?.isPastRetention, }, }); - if (result.source === "new" || result.source === "legacy-replica") { + if (result.found) { return result.value; } // past-retention is an intentional not-found: the token is gone. - if (result.source === "past-retention") { + if (result.reason === "past-retention") { return null; } // Read-your-writes fallback for a token completed immediately after mint, before it replicated: - // re-read from the run-ops PRIMARY only. We deliberately never read the control-plane/legacy + // re-read from the owning store's PRIMARY only. We deliberately never read the control-plane/legacy // primary here (that is the load the replica-only read-through exists to shed), so a legacy-resident // token that misses its replica stays a miss and the caller retries, rather than adding primary load. + const shardKey = resolveShard(opts.waitpointId); + if (shardKey !== "new" && shardKey !== "legacy") { + // A gen-2 token's primary is its OWN shard's writer. The gen-1 new writer is a different + // database, so reading it would miss and silently disable read-your-writes here. + const shardWriter = (opts.deps?.shardWriters ?? defaults.shardWriters).get(shardKey); + return shardWriter ? await opts.read(shardWriter) : null; + } + const fromNewPrimary = await opts.read(opts.deps?.newPrimary ?? defaults.newPrimary); if (fromNewPrimary != null) { return fromNewPrimary; diff --git a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts index a5a808e3195..1f0004dda1a 100644 --- a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts +++ b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts @@ -25,13 +25,14 @@ import { getApiVersion } from "~/api/versions"; import { WORKER_HEADERS } from "@trigger.dev/core/v3/runEngineWorker"; import { ServiceValidationError } from "~/v3/services/common.server"; import { EngineServiceValidationError } from "@internal/run-engine"; +import { unroutableIdResponse } from "./unroutableId.server"; import { tenantContext, tenantContextFromAuthEnvironment } from "~/services/tenantContext.server"; // Client aborts and service-level validation errors aren't bugs — they're // expected at API boundaries. Log them at `warn` so they stay in stdout // without flowing to Sentry via Logger.onError. function logBoundaryError( - message: "Error in loader" | "Error in action", + message: "Error in loader" | "Error in action" | "Unroutable id", error: unknown, url: string ) { @@ -451,6 +452,12 @@ export function createLoaderApiRoute< return await wrapResponse(request, error, corsStrategy !== "none"); } + const unroutable = unroutableIdResponse(error); + if (unroutable) { + logBoundaryError("Unroutable id", error, request.url); + return await wrapResponse(request, unroutable, corsStrategy !== "none"); + } + logBoundaryError("Error in loader", error, request.url); return await wrapResponse( @@ -722,6 +729,12 @@ export function createLoaderPATApiRoute< if (error instanceof Response) { return await wrapResponse(request, error, corsStrategy !== "none"); } + + const unroutable = unroutableIdResponse(error); + if (unroutable) { + logBoundaryError("Unroutable id", error, request.url); + return await wrapResponse(request, unroutable, corsStrategy !== "none"); + } return await wrapResponse( request, json({ error: "Internal Server Error" }, { status: 500 }), @@ -996,6 +1009,12 @@ export function createActionPATApiRoute< return await wrapResponse(request, error, corsStrategy !== "none"); } + const unroutable = unroutableIdResponse(error); + if (unroutable) { + logBoundaryError("Unroutable id", error, request.url); + return await wrapResponse(request, unroutable, corsStrategy !== "none"); + } + logBoundaryError("Error in action", error, request.url); // Typed validation errors map to their own status (default 400); @@ -1346,6 +1365,12 @@ export function createActionApiRoute< return await wrapResponse(request, error, corsStrategy !== "none"); } + const unroutable = unroutableIdResponse(error); + if (unroutable) { + logBoundaryError("Unroutable id", error, request.url); + return await wrapResponse(request, unroutable, corsStrategy !== "none"); + } + logBoundaryError("Error in action", error, request.url); return await wrapResponse( @@ -1612,6 +1637,12 @@ export function createMultiMethodApiRoute< return await wrapResponse(request, error, corsStrategy !== "none"); } + const unroutable = unroutableIdResponse(error); + if (unroutable) { + logBoundaryError("Unroutable id", error, request.url); + return await wrapResponse(request, unroutable, corsStrategy !== "none"); + } + logBoundaryError("Error in action", error, request.url); return await wrapResponse( diff --git a/apps/webapp/app/services/routeBuilders/unroutableId.server.ts b/apps/webapp/app/services/routeBuilders/unroutableId.server.ts new file mode 100644 index 00000000000..dcc2d46d7b6 --- /dev/null +++ b/apps/webapp/app/services/routeBuilders/unroutableId.server.ts @@ -0,0 +1,19 @@ +import { json } from "@remix-run/server-runtime"; +import { UnknownShardKey } from "@internal/run-store"; + +/** + * An id naming a shard the topology has no store for cannot be routed, so a read cannot locate + * the row: that is a 404, and matches what an absent gen-1 or cuid id already returns. It must + * not be a 500 — `resolveShard` is pure id-shape, so any base32hex core plus `[a-z0-9]` plus "2" + * parses as gen-2, which lets any caller induce a 5xx, and a 5xx on a read trips canary rollbacks. + * + * The router still throws. Callers log it before returning this, so a genuine misconfiguration — + * a shard key dropped from a config that is meant to be append-only — still alarms. + */ +export function unroutableIdResponse(error: unknown): Response | undefined { + // Explicitly NOT retryable: an id naming an unconfigured shard is not a transient miss, and + // no number of retries makes a topology grow a store. + return error instanceof UnknownShardKey + ? json({ error: "Not Found" }, { status: 404, headers: { "x-should-retry": "false" } }) + : undefined; +} diff --git a/apps/webapp/app/v3/runEngineHandlersShared.server.ts b/apps/webapp/app/v3/runEngineHandlersShared.server.ts index 4ce8cc2de8a..d8999e2332a 100644 --- a/apps/webapp/app/v3/runEngineHandlersShared.server.ts +++ b/apps/webapp/app/v3/runEngineHandlersShared.server.ts @@ -35,7 +35,8 @@ export async function readRunForEvent( deps: EventReadDeps ): Promise | null> { const result = await readThroughRun>({ - runId, + id: runId, + idKind: "run", environmentId, readNew: (client) => deps.store.findRun({ id: runId }, { select }, client), readLegacy: (replica) => deps.store.findRun({ id: runId }, { select }, replica), @@ -47,7 +48,7 @@ export async function readRunForEvent( }, }); - return result.source === "not-found" || result.source === "past-retention" ? null : result.value; + return result.found ? result.value : null; } /** diff --git a/apps/webapp/app/v3/runOpsMigration/readThrough.server.test.ts b/apps/webapp/app/v3/runOpsMigration/readThrough.server.test.ts index f7f7c43a530..8a657060ef9 100644 --- a/apps/webapp/app/v3/runOpsMigration/readThrough.server.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/readThrough.server.test.ts @@ -13,6 +13,21 @@ vi.setConfig({ testTimeout: 60_000 }); // 25-char cuid body → LEGACY residency. 26-char v1 body (version "1" at index 25) → NEW residency. const LEGACY_RUN_ID = "run_" + "a".repeat(25); const NEW_RUN_ID = "run_" + "b".repeat(24) + "01"; +// 26-char gen-2 body: shard char at index 24, version "2" at index 25. +const SHARD_A_RUN_ID = "run_" + "c".repeat(24) + "a2"; +const SHARD_Z_RUN_ID = "run_" + "c".repeat(24) + "z2"; +const LEGACY_WAITPOINT_ID = "waitpoint_" + "d".repeat(25); + +function throwingClient(label: string) { + return vi.fn(async (): Promise<{ marker: number } | null> => { + throw new Error(`${label} must never be read`); + }); +} + +function collectingLogger() { + const errors: { message: string; meta?: unknown }[] = []; + return { errors, error: (message: string, meta?: unknown) => errors.push({ message, meta }) }; +} // Lightweight real read: a trivial `$queryRaw` that genuinely hits the given container. // `hit` controls whether the read "finds" the run, so we exercise routing without @@ -28,14 +43,7 @@ async function realRead( // A presenter-shaped mapping: both "not-found" and "past-retention" collapse to the // same 404-ish surface, so an old run after termination yields the normal response. function toHttpish(result: ReadThroughResult): { status: number; value?: T } { - switch (result.source) { - case "new": - case "legacy-replica": - return { status: 200, value: result.value }; - case "not-found": - case "past-retention": - return { status: 404 }; - } + return result.found ? { status: 200, value: result.value } : { status: 404 }; } describe("readThroughRun (legacy replica + new DB)", () => { @@ -46,7 +54,8 @@ describe("readThroughRun (legacy replica + new DB)", () => { // read resolving through `legacyReplica` (prisma14) IS the structural guarantee // that the primary is never touched. const result = await readThroughRun({ - runId: LEGACY_RUN_ID, + id: LEGACY_RUN_ID, + idKind: "run", environmentId: "env_1", readNew: (c) => realRead(c, false), readLegacy: (c) => realRead(c, true), @@ -57,7 +66,7 @@ describe("readThroughRun (legacy replica + new DB)", () => { }, }); - expect(result.source).toBe("legacy-replica"); + expect(result.found && result.source).toBe("legacy-replica"); expect(toHttpish(result).status).toBe(200); } ); @@ -66,7 +75,8 @@ describe("readThroughRun (legacy replica + new DB)", () => { "post-termination past-retention returns the normal not-found surface", async ({ prisma14, prisma17 }) => { const pastRetentionResult = await readThroughRun({ - runId: LEGACY_RUN_ID, + id: LEGACY_RUN_ID, + idKind: "run", environmentId: "env_1", readNew: (c) => realRead(c, false), readLegacy: (c) => realRead(c, false), // legacy gone / retention elapsed @@ -78,11 +88,14 @@ describe("readThroughRun (legacy replica + new DB)", () => { }, }); - expect(pastRetentionResult.source).toBe("past-retention"); + expect(pastRetentionResult.found === false && pastRetentionResult.reason).toBe( + "past-retention" + ); // A run that is simply absent (not past retention) yields not-found. const notFoundResult = await readThroughRun({ - runId: LEGACY_RUN_ID, + id: LEGACY_RUN_ID, + idKind: "run", environmentId: "env_1", readNew: (c) => realRead(c, false), readLegacy: (c) => realRead(c, false), @@ -94,7 +107,7 @@ describe("readThroughRun (legacy replica + new DB)", () => { }, }); - expect(notFoundResult.source).toBe("not-found"); + expect(notFoundResult.found === false && notFoundResult.reason).toBe("not-found"); // Both collapse to the same 404-ish surface. expect(toHttpish(pastRetentionResult).status).toBe(toHttpish(notFoundResult).status); expect(toHttpish(pastRetentionResult).status).toBe(404); @@ -110,7 +123,8 @@ describe("readThroughRun (legacy replica + new DB)", () => { const newRead = vi.fn((c: PrismaReplicaClient) => realRead(c, true)); const result = await readThroughRun({ - runId: LEGACY_RUN_ID, + id: LEGACY_RUN_ID, + idKind: "run", environmentId: "env_1", readNew: newRead, readLegacy: throwingLegacy, @@ -121,7 +135,7 @@ describe("readThroughRun (legacy replica + new DB)", () => { }, }); - expect(result.source).toBe("new"); + expect(result.found && result.source).toBe("new"); expect(newRead).toHaveBeenCalledTimes(1); expect(throwingLegacy).not.toHaveBeenCalled(); } @@ -135,7 +149,152 @@ describe("readThroughRun (legacy replica + new DB)", () => { }); const result = await readThroughRun({ - runId: NEW_RUN_ID, + id: NEW_RUN_ID, + idKind: "run", + environmentId: "env_1", + readNew: (c) => realRead(c, true), + readLegacy: throwingLegacy, + deps: { + splitEnabled: true, + newClient: prisma17 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + }, + }); + + expect(result.found && result.source).toBe("new"); + expect(throwingLegacy).not.toHaveBeenCalled(); + } + ); + + heteroPostgresTest( + "gen-2 id reads its OWN shard replica once and probes no other store", + async ({ prisma14, prisma17 }) => { + const throwingNew = throwingClient("the gen-1 new store"); + const throwingLegacy = throwingClient("the legacy replica"); + const shardRead = vi.fn((c: PrismaReplicaClient) => realRead(c, true)); + + const result = await readThroughRun({ + id: SHARD_A_RUN_ID, + idKind: "run", + environmentId: "env_1", + // One closure serves both the gen-1 new store and a shard: a shard is the same + // dedicated schema. The throwing clients prove WHICH client it was handed. + readNew: (c) => shardRead(c), + readLegacy: throwingLegacy, + deps: { + splitEnabled: true, + newClient: throwingNew as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + shardReplicas: new Map([["a", prisma17 as unknown as PrismaReplicaClient]]), + }, + }); + + expect(result.found && result.source).toBe("shard:a"); + expect(shardRead).toHaveBeenCalledTimes(1); + // Identity, not deep equality: a Prisma client is too large to deep-compare. + expect(shardRead.mock.calls[0][0]).toBe(prisma17); + expect(throwingLegacy).not.toHaveBeenCalled(); + } + ); + + heteroPostgresTest( + "gen-2 id on an UNCONFIGURED shard key logs an error and returns not-found, never throws", + async ({ prisma14, prisma17 }) => { + const logger = collectingLogger(); + const throwingLegacy = throwingClient("the legacy replica"); + const newRead = vi.fn((c: PrismaReplicaClient) => realRead(c, true)); + + // Shard "z" is not configured. A 500 here would be inducible by any caller that + // guesses a shard char, so the layer must degrade rather than throw. + const result = await readThroughRun({ + id: SHARD_Z_RUN_ID, + idKind: "run", + environmentId: "env_1", + readNew: newRead, + readLegacy: throwingLegacy, + deps: { + splitEnabled: true, + newClient: prisma17 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + shardReplicas: new Map([["a", prisma17 as unknown as PrismaReplicaClient]]), + logger, + }, + }); + + expect(result.found).toBe(false); + expect(result.found === false && result.reason).toBe("not-found"); + expect(logger.errors).toHaveLength(1); + expect(logger.errors[0].meta).toMatchObject({ shardKey: "z", configured: ["a"] }); + // It must not silently fall back onto a gen-1 store. + expect(newRead).not.toHaveBeenCalled(); + expect(throwingLegacy).not.toHaveBeenCalled(); + } + ); + + heteroPostgresTest( + "gen-1 RUN id reads the legacy replica only and never probes the new store", + async ({ prisma14 }) => { + const throwingNew = throwingClient("the new store"); + const legacyRead = vi.fn((c: PrismaReplicaClient) => realRead(c, true)); + + const result = await readThroughRun({ + id: LEGACY_RUN_ID, + idKind: "run", + environmentId: "env_1", + readNew: throwingNew, + readLegacy: legacyRead, + deps: { + splitEnabled: true, + newClient: prisma14 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + }, + }); + + expect(result.found && result.source).toBe("legacy-replica"); + expect(throwingNew).not.toHaveBeenCalled(); + expect(legacyRead).toHaveBeenCalledTimes(1); + } + ); + + heteroPostgresTest( + "cuid WAITPOINT id keeps the new-FIRST pair probe (frozen: cuid waitpoints co-locate on new)", + async ({ prisma14, prisma17 }) => { + const calls: string[] = []; + const newRead = vi.fn(async (c: PrismaReplicaClient) => { + calls.push("new"); + return realRead(c, false); + }); + const legacyRead = vi.fn(async (c: PrismaReplicaClient) => { + calls.push("legacy"); + return realRead(c, true); + }); + + const result = await readThroughRun({ + id: LEGACY_WAITPOINT_ID, + idKind: "waitpoint", + environmentId: "env_1", + readNew: newRead, + readLegacy: legacyRead, + deps: { + splitEnabled: true, + newClient: prisma17 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + }, + }); + + expect(result.found && result.source).toBe("legacy-replica"); + expect(calls).toEqual(["new", "legacy"]); + } + ); + + heteroPostgresTest( + "a cuid waitpoint found on the new store returns it without touching legacy", + async ({ prisma14, prisma17 }) => { + const throwingLegacy = throwingClient("the legacy replica"); + + const result = await readThroughRun({ + id: LEGACY_WAITPOINT_ID, + idKind: "waitpoint", environmentId: "env_1", readNew: (c) => realRead(c, true), readLegacy: throwingLegacy, @@ -146,7 +305,7 @@ describe("readThroughRun (legacy replica + new DB)", () => { }, }); - expect(result.source).toBe("new"); + expect(result.found && result.source).toBe("new"); expect(throwingLegacy).not.toHaveBeenCalled(); } ); diff --git a/apps/webapp/app/v3/runOpsMigration/readThrough.server.ts b/apps/webapp/app/v3/runOpsMigration/readThrough.server.ts index f15230ec442..6e1beaae62c 100644 --- a/apps/webapp/app/v3/runOpsMigration/readThrough.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/readThrough.server.ts @@ -3,12 +3,18 @@ * (which carries the read load we are shedding). Disabled entirely when isSplitEnabled() * is false (single-DB passthrough). * - * During the retention window, old run-ops rows are served off the legacy read replica. - * Residency is decided purely by id-shape: a run-ops id (NEW) id reads new only, a cuid - * (LEGACY) id reads legacy only. An unclassifiable id falls back to a new-then-legacy - * probe. After termination, past-retention runs return the normal not-found response. - * Patterned on `mollifier/resolveRunForMutation.server.ts` (`?? default` DI), but with - * the legacy-primary/writer fallback deliberately removed: this layer has NO legacy-writer + * Residency is decided purely by id-shape, via `resolveShard`: a gen-2 body names its own + * shard (ONE read there), a gen-1 v1 body reads new only, everything else is legacy and + * routes on `idKind`. + * + * `idKind` is required because 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 is legacy-resident (there + * is no cuid run migration), while a cuid WAITPOINT can be co-located with its run on the + * new store, which is what makes the new-first probe load-bearing for it. No default — + * a default would pick one of those arms silently. + * + * Patterned on `mollifier/resolveRunForMutation.server.ts` (`?? default` DI), but with the + * legacy-primary/writer fallback deliberately removed: this layer has NO legacy-writer * handle at all (structural guarantee). */ import type { PrismaReplicaClient } from "~/db.server"; @@ -17,90 +23,118 @@ import { runOpsNewReplica as defaultNewClient, } from "~/db.server"; import { logger as defaultLogger } from "~/services/logger.server"; -import { ownerEngine, UnclassifiableRunId } from "@trigger.dev/core/v3/isomorphic"; +import { resolveShard, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import { isSplitEnabled } from "./splitMode.server"; +import { runOpsShardReplicas } from "./shardHandles.server"; + +type ShardSource = `shard:${string}`; -type ReadThroughSource = "new" | "legacy-replica"; +type ReadThroughSource = "new" | "legacy-replica" | ShardSource; +/** + * `found` carries hit/miss STRUCTURALLY. `source` is open-ended once shards exist, so a + * consumer testing found-ness by listing hit sources reads a gen-2 hit as a miss; + * discriminating on `found` makes that a compile error instead. + */ export type ReadThroughResult = - | { source: ReadThroughSource; value: T } - | { source: "not-found" } - | { source: "past-retention" }; + | { found: true; source: ReadThroughSource; value: T } + | { found: false; reason: "not-found" | "past-retention" }; type ReadThroughDeps = { newClient?: PrismaReplicaClient; legacyReplica?: PrismaReplicaClient; + /** + * Gen-2 shard replicas by shard char; empty (RUN_OPS_SHARDS unset) makes the gen-2 arm + * unreachable. Load-bearing only for callers whose closures read a client DIRECTLY: + * `RoutingRunStore` never forwards a caller's client, so for store-backed closures the + * client picked here is only a read-your-writes signal. Not dead weight. + */ + shardReplicas?: ReadonlyMap; /** Resolved boot constant; never `await`ed per-request when supplied. */ splitEnabled?: boolean; - isPastRetention?: (runId: string) => boolean; - logger?: { warn: (m: string, meta?: unknown) => void }; + isPastRetention?: (id: string) => boolean; + logger?: { error: (m: string, meta?: Record) => void }; /** Saturation-signal emit hook: called on each legacy-replica hit. */ - onLegacyReplicaRead?: (runId: string) => void; + onLegacyReplicaRead?: (id: string) => void; }; type ReadThroughRunInput = { - runId: string; + id: string; + idKind: "run" | "waitpoint"; environmentId: string; readNew: (client: PrismaReplicaClient) => Promise; readLegacy: (replica: PrismaReplicaClient) => Promise; deps?: ReadThroughDeps; }; +function hit(source: ReadThroughSource, value: T): ReadThroughResult { + return { found: true, source, value }; +} + +function miss(reason: "not-found" | "past-retention"): ReadThroughResult { + return { found: false, reason }; +} + export async function readThroughRun( input: ReadThroughRunInput ): Promise> { - const { runId, deps } = input; + const { id, idKind, deps } = input; const newClient = deps?.newClient ?? defaultNewClient; const legacyReplica = deps?.legacyReplica ?? defaultLegacyReplica; + const shardReplicas = deps?.shardReplicas ?? runOpsShardReplicas; const logger = deps?.logger ?? defaultLogger; const splitEnabled = deps?.splitEnabled ?? (await isSplitEnabled()); - // Passthrough: single plain read against the one collapsed store. No legacy read, - // no second connection. + // Passthrough: single plain read against the one collapsed store. if (!splitEnabled) { const v = await input.readNew(newClient); - return v != null ? { source: "new", value: v } : { source: "not-found" }; + return v != null ? hit("new", v) : miss("not-found"); } - // Split is on. Classify residency; an unclassifiable id is treated as LEGACY - // (conservative — probe rather than drop a real run). - let residency: "LEGACY" | "NEW"; - try { - residency = ownerEngine(runId); - } catch (e) { - if (e instanceof UnclassifiableRunId) { - logger.warn("readThroughRun: UnclassifiableRunId, treating as LEGACY", { - runId, - valueLength: e.valueLength, + // Total: an unclassifiable id resolves to "legacy" (probe rather than drop a real run). + const shardKey = resolveShard(id); + + if (shardKey !== "new" && shardKey !== "legacy") { + const shardReplica = shardReplicas.get(shardKey); + if (shardReplica === undefined) { + // Deliberately not a throw: this id arrives from the caller (a URL param on the + // waitpoint route) and any base32hex core + [a-z0-9] + "2" parses as gen-2, so a + // throw is a 500 any client can induce. An error-logged not-found is neither silent + // nor a misroute. Throwing stays correct on the router path, where ids are minted. + logger.error("readThroughRun: gen-2 id resolved to an unconfigured shard key", { + id, + shardKey, + configured: [...shardReplicas.keys()], }); - residency = "LEGACY"; - } else { - throw e; + return miss("not-found"); } + // A gen-2 shard is a dedicated-schema store, exactly like `new`, so `readNew` fits. + const v = await input.readNew(shardReplica); + return v != null ? hit(`shard:${shardKey}`, v) : miss("not-found"); } - // A run-ops id can only live on the new DB — skip the legacy replica entirely. - if (residency === "NEW") { + if (shardKey === "new") { const v = await input.readNew(newClient); - return v != null ? { source: "new", value: v } : { source: "not-found" }; + return v != null ? hit("new", v) : miss("not-found"); } - // LEGACY (or unclassifiable→LEGACY) fan-out: new first. - const v = await input.readNew(newClient); - if (v != null) { - return { source: "new", value: v }; + if (idKind === "waitpoint") { + const v = await input.readNew(newClient); + if (v != null) { + return hit("new", v); + } } // Legacy READ REPLICA only — never a legacy writer/primary (no such handle exists). const lv = await input.readLegacy(legacyReplica); if (lv != null) { - deps?.onLegacyReplicaRead?.(runId); - return { source: "legacy-replica", value: lv }; + deps?.onLegacyReplicaRead?.(id); + return hit("legacy-replica", lv); } - if (deps?.isPastRetention?.(runId)) { - return { source: "past-retention" }; + if (deps?.isPastRetention?.(id)) { + return miss("past-retention"); } - return { source: "not-found" }; + return miss("not-found"); } diff --git a/apps/webapp/app/v3/runOpsMigration/shardHandles.server.test.ts b/apps/webapp/app/v3/runOpsMigration/shardHandles.server.test.ts new file mode 100644 index 00000000000..b90c1dfe7c4 --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/shardHandles.server.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { buildShardHandleMaps } from "./shardHandles.server"; + +// Two distinct sentinels per shard: the maps must not cross writer and replica. +function handle(key: string) { + return { + key, + writer: { tag: `${key}-writer` } as never, + replica: { tag: `${key}-replica` } as never, + }; +} + +describe("buildShardHandleMaps", () => { + it("yields empty maps when no shard is configured", () => { + const { replicas, writers } = buildShardHandleMaps([]); + + expect(replicas.size).toBe(0); + expect(writers.size).toBe(0); + }); + + it("keys each shard's replica and writer under its shard char", () => { + const { replicas, writers } = buildShardHandleMaps([handle("a"), handle("b")]); + + expect([...replicas.keys()].sort()).toEqual(["a", "b"]); + expect([...writers.keys()].sort()).toEqual(["a", "b"]); + expect(replicas.get("a")).toEqual({ tag: "a-replica" }); + expect(writers.get("a")).toEqual({ tag: "a-writer" }); + expect(replicas.get("b")).toEqual({ tag: "b-replica" }); + expect(writers.get("b")).toEqual({ tag: "b-writer" }); + }); + + it("never places a writer in the replica map", () => { + const { replicas } = buildShardHandleMaps([handle("a")]); + + expect(replicas.get("a")).not.toEqual({ tag: "a-writer" }); + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/shardHandles.server.ts b/apps/webapp/app/v3/runOpsMigration/shardHandles.server.ts new file mode 100644 index 00000000000..cbe827be4dc --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/shardHandles.server.ts @@ -0,0 +1,46 @@ +/** + * Gen-2 shard client handles, keyed by shard char, for the consumers that route by + * `resolveShard` outside the run-store boundary: read-through and the two cross-seam + * batch hydration sites. Both maps are empty unless RUN_OPS_SHARDS is configured, which + * is what keeps every gen-2 arm unreachable today. + */ +import type { PrismaClient } from "@trigger.dev/database"; +import type { ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import type { PrismaReplicaClient } from "~/db.server"; +import { runOpsShardHandles } from "~/db.server"; + +type ShardHandle = { + key: string; + writer: unknown; + replica: unknown; +}; + +export function buildShardHandleMaps(handles: ShardHandle[]): { + replicas: ReadonlyMap; + writers: ReadonlyMap; +} { + const replicas = new Map(); + const writers = new Map(); + for (const handle of handles) { + replicas.set(handle.key, handle.replica as PrismaReplicaClient); + writers.set(handle.key, handle.writer as PrismaClient); + } + return { replicas, writers }; +} + +// A gen-2 shard is the same dedicated subset schema as the gen-1 new store, so these casts +// carry exactly the precedent (and the same residual risk) as `runOpsNewPrisma`'s. +// The try/catch mirrors `runStore.server.ts`'s handle resolution: a minimal `db.server` mock +// does not define this export at all, and accessing an undefined mock export throws. +function resolveShardHandles(): ShardHandle[] { + try { + return runOpsShardHandles ?? []; + } catch { + return []; + } +} + +const maps = buildShardHandleMaps(resolveShardHandles()); + +export const runOpsShardReplicas = maps.replicas; +export const runOpsShardWriters = maps.writers; diff --git a/apps/webapp/app/v3/runOpsMigration/track1-baseline.json b/apps/webapp/app/v3/runOpsMigration/track1-baseline.json index 63d27dbadca..26d038678ea 100644 --- a/apps/webapp/app/v3/runOpsMigration/track1-baseline.json +++ b/apps/webapp/app/v3/runOpsMigration/track1-baseline.json @@ -68,19 +68,19 @@ "WaitpointTag.project" ], "totals": { - "violations": 4, - "detectorI": 4, + "violations": 5, + "detectorI": 5, "detectorII": 0, "detectorIII": 0, "write": 0, - "read": 4, + "read": 5, "files": 1, "legacyAnnotations": 0 }, "violations": [ { "file": "apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts", - "line": 89, + "line": 93, "model": "BatchTaskRun", "delegate": "batchTaskRun", "callKind": "read", @@ -89,7 +89,7 @@ }, { "file": "apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts", - "line": 150, + "line": 154, "model": "BatchTaskRun", "delegate": "batchTaskRun", "callKind": "read", @@ -98,16 +98,25 @@ }, { "file": "apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts", - "line": 184, + "line": 214, "model": "TaskRun", "delegate": "taskRun", "callKind": "read", "detector": "i", - "snippet": "const newRows = (await newClient.taskRun.findMany({" + "snippet": "? ((await newClient.taskRun.findMany({" }, { "file": "apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts", - "line": 196, + "line": 224, + "model": "TaskRun", + "delegate": "taskRun", + "callKind": "read", + "detector": "i", + "snippet": "(await shardReplicas.get(shardKey)!.taskRun.findMany({" + }, + { + "file": "apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts", + "line": 241, "model": "TaskRun", "delegate": "taskRun", "callKind": "read", diff --git a/apps/webapp/app/v3/runOpsMigration/waitpointTokenResolve.server.test.ts b/apps/webapp/app/v3/runOpsMigration/waitpointTokenResolve.server.test.ts index 9ea849c8058..42dca92e1ce 100644 --- a/apps/webapp/app/v3/runOpsMigration/waitpointTokenResolve.server.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/waitpointTokenResolve.server.test.ts @@ -138,7 +138,8 @@ describe("public wait-token resolution across the split boundary", () => { expect(gated?.id).toBe(waitpointId); const passthrough = await readThroughRun({ - runId: waitpointId, + id: waitpointId, + idKind: "waitpoint", environmentId: environment.id, readNew: (c) => read(c), readLegacy: (r) => read(r), @@ -150,7 +151,7 @@ describe("public wait-token resolution across the split boundary", () => { }); expect(gated).not.toBeNull(); - expect(passthrough.source).toBe("not-found"); + expect(passthrough.found === false && passthrough.reason).toBe("not-found"); } ); }); diff --git a/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.test.ts b/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.test.ts index 99d4cfd2dd7..779a7234748 100644 --- a/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.test.ts +++ b/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.test.ts @@ -13,6 +13,8 @@ vi.setConfig({ testTimeout: 60_000 }); // 25-char cuid body → LEGACY residency. 26-char v1 body (version "1" at index 25) → NEW residency. const LEGACY_RUN_ID = "run_" + "a".repeat(25); const NEW_RUN_ID = "run_" + "b".repeat(24) + "01"; +// 26-char gen-2 body: shard char at index 24, version "2" at index 25. +const SHARD_A_RUN_ID = "run_" + "c".repeat(24) + "a2"; type Row = { id: string }; @@ -90,4 +92,109 @@ describe("hydrateRunsAcrossSeam (PG14 legacy replica + PG17 new)", () => { expect(throwingLegacy).not.toHaveBeenCalled(); } ); + + heteroPostgresTest( + "(c) a gen-2 id hydrates from its OWN shard and is never read from the gen-1 stores", + async ({ prisma14, prisma17 }) => { + // Before the shard arm existed a gen-2 id joined the `new` group, missed, and was + // never legacy-probed either — so it vanished from the page with no error. + const onShardA = new Set([SHARD_A_RUN_ID]); + + const readNew = vi.fn(async (client: PrismaReplicaClient, ids: string[]): Promise => { + if ( + ids.includes(SHARD_A_RUN_ID) && + client !== (prisma17 as unknown as PrismaReplicaClient) + ) { + throw new Error("a gen-2 id must only be read on its own shard"); + } + return realReadFiltered(client, ids, onShardA); + }); + const readLegacyReplica = vi.fn( + async (_replica: PrismaReplicaClient, ids: string[]): Promise => { + if (ids.includes(SHARD_A_RUN_ID)) { + throw new Error("a gen-2 id must never reach the legacy probe"); + } + return []; + } + ); + + const rows = await hydrateRunsAcrossSeam({ + runIds: [SHARD_A_RUN_ID], + readNew, + readLegacyReplica, + deps: { + splitEnabled: true, + newClient: prisma14 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + shardReplicas: new Map([["a", prisma17 as unknown as PrismaReplicaClient]]), + }, + }); + + expect(rows.map((r) => r.id)).toEqual([SHARD_A_RUN_ID]); + expect(readLegacyReplica).not.toHaveBeenCalled(); + } + ); + + heteroPostgresTest( + "(d) a mixed gen-1 and gen-2 page hydrates every member", + async ({ prisma14, prisma17 }) => { + const onGenOneNew = new Set([NEW_RUN_ID]); + const onLegacy = new Set([LEGACY_RUN_ID]); + const onShardA = new Set([SHARD_A_RUN_ID]); + + const readNew = vi.fn(async (client: PrismaReplicaClient, ids: string[]): Promise => { + const present = ids.includes(SHARD_A_RUN_ID) ? onShardA : onGenOneNew; + return realReadFiltered(client, ids, present); + }); + const readLegacyReplica = vi.fn( + async (replica: PrismaReplicaClient, ids: string[]): Promise => + realReadFiltered(replica, ids, onLegacy) + ); + + const rows = await hydrateRunsAcrossSeam({ + runIds: [NEW_RUN_ID, LEGACY_RUN_ID, SHARD_A_RUN_ID], + readNew, + readLegacyReplica, + deps: { + splitEnabled: true, + newClient: prisma17 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + shardReplicas: new Map([["a", prisma17 as unknown as PrismaReplicaClient]]), + }, + }); + + expect(rows.map((r) => r.id).sort()).toEqual( + [NEW_RUN_ID, LEGACY_RUN_ID, SHARD_A_RUN_ID].sort() + ); + } + ); + + heteroPostgresTest( + "(e) a gen-2 id on an unconfigured shard is dropped with a logged error, not read elsewhere", + async ({ prisma14, prisma17 }) => { + const errors: unknown[] = []; + const readNew = vi.fn(async (client: PrismaReplicaClient, ids: string[]): Promise => { + if (ids.includes(SHARD_A_RUN_ID)) { + throw new Error("an unconfigured gen-2 id must not fall back to a gen-1 store"); + } + return realReadFiltered(client, ids, new Set([NEW_RUN_ID])); + }); + + const rows = await hydrateRunsAcrossSeam({ + runIds: [NEW_RUN_ID, SHARD_A_RUN_ID], + readNew, + readLegacyReplica: async () => [], + deps: { + splitEnabled: true, + newClient: prisma17 as unknown as PrismaReplicaClient, + legacyReplica: prisma14 as unknown as PrismaReplicaClient, + shardReplicas: new Map(), + logger: { error: (_m, meta) => errors.push(meta) }, + }, + }); + + expect(rows.map((r) => r.id)).toEqual([NEW_RUN_ID]); + expect(errors).toHaveLength(1); + } + ); }); diff --git a/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.ts b/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.ts index c7a0dc735e8..bc476cebf31 100644 --- a/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.ts +++ b/apps/webapp/app/v3/services/bulk/BulkActionV2.batchReadThrough.server.ts @@ -20,7 +20,8 @@ import { runOpsLegacyReplica as defaultLegacyReplica, runOpsNewReplica as defaultNewClient, } from "~/db.server"; -import { ownerEngine, UnclassifiableRunId } from "@trigger.dev/core/v3/isomorphic"; +import { resolveShard, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import { runOpsShardReplicas as defaultShardReplicas } from "~/v3/runOpsMigration/shardHandles.server"; type SeamReadDeps = { /** @@ -30,7 +31,9 @@ type SeamReadDeps = { splitEnabled: boolean; newClient?: PrismaReplicaClient; legacyReplica?: PrismaReplicaClient; - logger?: { warn: (m: string, meta?: unknown) => void }; + /** Gen-2 shard replicas by shard char; empty (RUN_OPS_SHARDS unset) keeps today's behaviour. */ + shardReplicas?: ReadonlyMap; + logger?: { error: (m: string, meta?: Record) => void }; }; type HydrateRunsAcrossSeamInput = { @@ -61,28 +64,30 @@ export async function hydrateRunsAcrossSeam(input: HydrateRunsAcrossSeamInput return input.readNew(newClient, runIds); } - // Split is on. Classify residency; unclassifiable → LEGACY (probe rather than drop). + // Split is on. Partition by shard key; `resolveShard` is total, so an unclassifiable id + // resolves to "legacy" (probe rather than drop). A gen-2 id goes to its OWN shard and to + // no other store: it is directly routable, so it joins neither gen-1 group. + const shardReplicas = deps.shardReplicas ?? defaultShardReplicas; const newIds: string[] = []; const legacyCandidateIds: string[] = []; + const idsByShard = new Map(); for (const runId of runIds) { - let residency: "LEGACY" | "NEW"; - try { - residency = ownerEngine(runId); - } catch (e) { - if (e instanceof UnclassifiableRunId) { - deps.logger?.warn("hydrateRunsAcrossSeam: UnclassifiableRunId, treating as LEGACY", { - runId, - valueLength: e.valueLength, - }); - residency = "LEGACY"; - } else { - throw e; - } - } - if (residency === "NEW") { + const shardKey = resolveShard(runId); + if (shardKey === "new") { newIds.push(runId); - } else { + } else if (shardKey === "legacy") { legacyCandidateIds.push(runId); + } else if (shardReplicas.has(shardKey)) { + const group = idsByShard.get(shardKey); + group ? group.push(runId) : idsByShard.set(shardKey, [runId]); + } else { + // Not routable and not a gen-1 shape. Reading a gen-1 store would query the wrong + // database, so the id is dropped from the page — loudly, never silently. + deps.logger?.error("hydrateRunsAcrossSeam: gen-2 id on an unconfigured shard key", { + runId, + shardKey, + configured: [...shardReplicas.keys()], + }); } } @@ -103,6 +108,16 @@ export async function hydrateRunsAcrossSeam(input: HydrateRunsAcrossSeamInput legacyRows = await input.readLegacyReplica(legacyReplica, legacyToProbe); } + // Each configured shard is read once, in parallel: the groups are disjoint by id, so the + // results need no dedupe. + const shardRows = ( + await Promise.all( + [...idsByShard.entries()].map(([shardKey, ids]) => + input.readNew(shardReplicas.get(shardKey)!, ids) + ) + ) + ).flat(); + // Order within the page is irrelevant (downstream pMap does not depend on it). - return [...newRows, ...legacyRows]; + return [...newRows, ...legacyRows, ...shardRows]; } diff --git a/apps/webapp/test/apiBatchResultsPresenter.dedicatedSeam.test.ts b/apps/webapp/test/apiBatchResultsPresenter.dedicatedSeam.test.ts index eb322c48a1c..ade925f239c 100644 --- a/apps/webapp/test/apiBatchResultsPresenter.dedicatedSeam.test.ts +++ b/apps/webapp/test/apiBatchResultsPresenter.dedicatedSeam.test.ts @@ -3,10 +3,10 @@ // RESULTS READ assembles correctly when one batch's members are genuinely split across the real // dedicated run-ops subset schema (prisma17 / RunOpsPrismaClient) and the full control-plane // schema (prisma14) — not a mirrored full schema on both sides. No mocks. -import { heteroRunOpsPostgresTest } from "@internal/testcontainers"; +import { heteroRunOpsPostgresTest, makeNShardRunOpsPostgresTest } from "@internal/testcontainers"; import type { RunOpsPrismaClient } from "@internal/run-ops-database"; import type { PrismaClient } from "@trigger.dev/database"; -import { generateRunOpsId } from "@trigger.dev/core/v3/isomorphic"; +import { generateRunOpsId, generateRunOpsIdV2 } from "@trigger.dev/core/v3/isomorphic"; import { describe, expect, vi } from "vitest"; import type { PrismaReplicaClient } from "~/db.server"; import type { AuthenticatedEnvironment } from "~/services/apiAuth.server"; @@ -209,6 +209,10 @@ async function seedBatchOnNew( return batch; } +// One real gen-2 shard on its OWN database, so a member seeded there is genuinely absent +// from the gen-1 `new` store rather than merely routed away from it. +const oneShardTest = makeNShardRunOpsPostgresTest(1); + const env = (ctx: SeedCtx) => ({ id: ctx.environment.id, @@ -334,4 +338,141 @@ describe("ApiBatchResultsPresenter split mode — real run-ops dedicated schema expect(result!.items[0]).toMatchObject({ ok: true, id: "run_present" }); } ); + + // A gen-2 member is directly routable to its own shard. Before the shard arm existed it + // joined the gen-1 `new` read, missed there, and — classifying dedicated-family — never + // reached the legacy probe either, so it vanished from the batch results with no error. + oneShardTest( + "a gen-2 member is hydrated from its own shard database alongside a legacy-resident member", + async ({ legacyPrisma, newPrisma, shardPrismas }) => { + const shardPrisma = shardPrismas[0]!; + const ctx = await seedLegacyEnv(legacyPrisma, "gen2-shard"); + await relaxLegacyAttemptFk(legacyPrisma); + await relaxNewBatchItemFk(newPrisma); + + const shardMemberId = generateRunOpsIdV2("a"); + const legacyMemberId = generateLegacyCuid(); + + // The gen-2 member exists ONLY on the shard database. The gen-1 `new` store below is a + // different database, so routing this id there would genuinely miss. + await seedNewMember( + shardPrisma, + { envId: ctx.environment.id, orgId: ctx.organization.id, projectId: ctx.project.id }, + { + id: shardMemberId, + friendlyId: "run_shard_member", + status: "COMPLETED_SUCCESSFULLY", + output: JSON.stringify({ from: "shard-a" }), + } + ); + await seedLegacyMember(legacyPrisma, ctx, { + id: legacyMemberId, + friendlyId: "run_legacy_member", + status: "COMPLETED_WITH_ERRORS", + error: { type: "BUILT_IN_ERROR", name: "Err", message: "boom", stackTrace: "" }, + }); + + const batchFriendlyId = "batch_gen2_shard"; + await seedBatchOnNew(newPrisma, ctx.environment.id, batchFriendlyId, [ + shardMemberId, + legacyMemberId, + ]); + + const presenter = new ApiBatchResultsPresenter(throwingPrisma, throwingPrisma, { + splitEnabled: true, + newClient: newPrisma as unknown as PrismaReplicaClient, + legacyReplica: legacyPrisma as unknown as PrismaReplicaClient, + shardReplicas: new Map([["a", shardPrisma as unknown as PrismaReplicaClient]]), + }); + + const result = await presenter.call(batchFriendlyId, env(ctx)); + + expect(result).toBeDefined(); + expect(result!.items).toHaveLength(2); + const [first, second] = result!.items; + expect(first).toEqual({ + ok: true, + id: "run_shard_member", + taskIdentifier: "my-task", + output: JSON.stringify({ from: "shard-a" }), + outputType: "application/json", + }); + expect(second).toMatchObject({ ok: false, id: "run_legacy_member" }); + }, + 180_000 + ); + + // A gen-2 id naming a shard that is NOT configured must not fall back onto a gen-1 store: + // that reads the wrong database, misses, and (being dedicated-family) never reaches the + // legacy probe, so the member disappears with no error. Drop it, but loudly. + oneShardTest( + "a gen-2 member on an unconfigured shard is dropped without being read from a gen-1 store", + async ({ legacyPrisma, newPrisma, shardPrismas }) => { + const shardPrisma = shardPrismas[0]!; + const ctx = await seedLegacyEnv(legacyPrisma, "gen2-unconfigured"); + await relaxLegacyAttemptFk(legacyPrisma); + await relaxNewBatchItemFk(newPrisma); + + // Shard "z" is not in the configured map; shard "a" is. + const unconfiguredId = generateRunOpsIdV2("z"); + const legacyMemberId = generateLegacyCuid(); + + await seedNewMember( + shardPrisma, + { envId: ctx.environment.id, orgId: ctx.organization.id, projectId: ctx.project.id }, + { id: unconfiguredId, friendlyId: "run_unconfigured", status: "COMPLETED_SUCCESSFULLY" } + ); + await seedLegacyMember(legacyPrisma, ctx, { + id: legacyMemberId, + friendlyId: "run_legacy_member", + status: "COMPLETED_SUCCESSFULLY", + }); + + const batchFriendlyId = "batch_gen2_unconfigured"; + await seedBatchOnNew(newPrisma, ctx.environment.id, batchFriendlyId, [ + unconfiguredId, + legacyMemberId, + ]); + + // A closure-based recorder, not a mock: it records the id sets each store is asked for, + // so the assertion is about real reads rather than about a test double's behaviour. + const askedOf = (label: string, target: RunOpsPrismaClient | PrismaClient) => { + const asked: string[][] = []; + const handle = { + ...target, + taskRun: { + findMany: (args: { where?: { id?: { in?: string[] } } }) => { + asked.push(args.where?.id?.in ?? []); + return (target as unknown as PrismaReplicaClient).taskRun.findMany(args as never); + }, + }, + } as unknown as PrismaReplicaClient; + return { label, asked, handle }; + }; + const genOneNew = askedOf("new", newPrisma); + const legacy = askedOf("legacy", legacyPrisma); + + const presenter = new ApiBatchResultsPresenter(throwingPrisma, throwingPrisma, { + splitEnabled: true, + newClient: genOneNew.handle, + legacyReplica: legacy.handle, + shardReplicas: new Map([["a", shardPrisma as unknown as PrismaReplicaClient]]), + }); + + const result = await presenter.call(batchFriendlyId, env(ctx)); + + // The legacy member still resolves; the unconfigured gen-2 member is dropped. + expect(result).toBeDefined(); + expect(result!.items).toHaveLength(1); + expect(result!.items[0]).toMatchObject({ ok: true, id: "run_legacy_member" }); + + // The unconfigured id was never asked of a gen-1 store. + for (const store of [genOneNew, legacy]) { + for (const ids of store.asked) { + expect(ids).not.toContain(unconfiguredId); + } + } + }, + 180_000 + ); }); diff --git a/apps/webapp/test/readRunForEvent.replicaLag.test.ts b/apps/webapp/test/readRunForEvent.replicaLag.test.ts index 877f920e40f..9817a1b6e55 100644 --- a/apps/webapp/test/readRunForEvent.replicaLag.test.ts +++ b/apps/webapp/test/readRunForEvent.replicaLag.test.ts @@ -195,4 +195,76 @@ describe("readRunForEvent tolerates replica lag on its event-enrichment read", ( expect(onPrimary.friendlyId).toBe("run_rrfe_missing"); } ); + + // (c) SPLIT MODE, the gen-1 run fast path. A cuid run id classifies legacy, and there is no cuid + // run migration, so the new-store probe cannot find it. readRunForEvent declares idKind "run", + // which reads the legacy replica ONLY. The observable difference is the number of reads: one on + // the fast path, two on the old new-then-legacy pair probe. Counted by delegating through the + // real store rather than replacing it. + containerTest( + "readRunForEvent takes ONE read for a cuid run id under split, not a new-then-legacy pair", + async ({ prisma }) => { + const { organization, project, environment } = await seedEnvironment(prisma, "rrfe_split"); + + const runId = "d".repeat(25); // cuid-shaped -> classifies legacy + const friendlyId = "run_rrfe_split"; + + await prisma.taskRun.create({ + data: { + id: runId, + engine: "V2", + status: "COMPLETED_SUCCESSFULLY", + friendlyId, + taskIdentifier: "my-task", + payload: "{}", + payloadType: "application/json", + traceId: "trace_split", + spanId: "span_split", + queue: "task/my-task", + runtimeEnvironmentId: environment.id, + projectId: project.id, + organizationId: organization.id, + environmentType: "DEVELOPMENT", + isTest: false, + taskEventStore: "taskEvent", + }, + }); + + const realStore = new PostgresRunStore({ prisma, readOnlyPrisma: prisma as never }); + let findRunCalls = 0; + const countingStore = new Proxy(realStore, { + get(target, prop, receiver) { + if (prop === "findRun") { + return (...args: unknown[]) => { + findRunCalls += 1; + return (target.findRun as (...a: unknown[]) => unknown)(...args); + }; + } + return Reflect.get(target, prop, receiver); + }, + }); + + // The new side MUST miss for the two arms to be distinguishable: a pair probe that finds the + // row on its first read short-circuits and looks identical to the fast path. `missing` makes + // the new-store read return nothing, exactly as it would for a legacy-resident run. + const missingOnNew = laggingReplica(prisma, [{ model: "taskRun", mode: "missing" }]); + + const deps: EventReadDeps = { + store: countingStore as never, + newReplica: missingOnNew.client as never, + legacyReplica: prisma as never, + splitEnabled: true, + }; + + const run = await readRunForEvent(runId, environment.id, EVENT_SELECT, deps); + + // The run still resolves — the fast path must not cost the read. + expect(run).not.toBeNull(); + expect(run!.id).toBe(runId); + expect(run!.friendlyId).toBe(friendlyId); + + // ONE read. Two would mean the new store was probed first, which is the arm this removes. + expect(findRunCalls).toBe(1); + } + ); }); diff --git a/apps/webapp/test/resolveWaitpointThroughReadThrough.readthrough.test.ts b/apps/webapp/test/resolveWaitpointThroughReadThrough.readthrough.test.ts index c0b627262f7..09c3327ab57 100644 --- a/apps/webapp/test/resolveWaitpointThroughReadThrough.readthrough.test.ts +++ b/apps/webapp/test/resolveWaitpointThroughReadThrough.readthrough.test.ts @@ -1,7 +1,7 @@ import { heteroRunOpsPostgresTest, postgresTest } from "@internal/testcontainers"; import type { RunOpsPrismaClient } from "@internal/run-ops-database"; import type { PrismaClient } from "@trigger.dev/database"; -import { generateRunOpsId } from "@trigger.dev/core/v3/isomorphic"; +import { generateRunOpsId, generateRunOpsIdV2 } from "@trigger.dev/core/v3/isomorphic"; import { describe, expect, vi } from "vitest"; import type { PrismaReplicaClient } from "~/db.server"; import { resolveWaitpointThroughReadThrough } from "~/runEngine/concerns/resolveWaitpointThroughReadThrough.server"; @@ -286,4 +286,105 @@ describe("resolveWaitpointThroughReadThrough (hetero PG14 legacy + dedicated run expect(legacy.calls.length).toBe(0); } ); + + heteroRunOpsPostgresTest( + "gen-2 waitpoint resolves on its OWN shard replica; the gen-1 new store is never read", + async ({ prisma17, prisma14 }) => { + const id = generateRunOpsIdV2("a"); + const environmentId = generateRunOpsId(); + const projectId = generateRunOpsId(); + const seeded = await seedWaitpoint(prisma17, id, { id: environmentId, projectId }); + + // The gen-1 new store and the legacy replica are both forbidden: a gen-2 id must + // take one read on its shard and probe nothing else. + const newClient = recording(prisma14, { forbidden: true }); + const legacyReplica = recording(prisma14, { forbidden: true }); + const shardReplica = recording(prisma17); + + const result = await resolveWaitpointThroughReadThrough({ + waitpointId: id, + environmentId, + read: read(id, environmentId), + deps: { + splitEnabled: true, + newClient: newClient.handle, + legacyReplica: legacyReplica.handle, + newPrimary: newClient.handle, + shardReplicas: new Map([["a", shardReplica.handle]]), + }, + }); + + expect(result).not.toBeNull(); + expect(result!.id).toBe(seeded.id); + expect(shardReplica.calls.length).toBe(1); + expect(newClient.calls.length).toBe(0); + expect(legacyReplica.calls.length).toBe(0); + } + ); + + heteroRunOpsPostgresTest( + "a gen-2 waitpoint missing its shard REPLICA falls back to that shard's WRITER, not the gen-1 new writer", + async ({ prisma17, prisma14 }) => { + // Read-your-writes: a token completed immediately after mint may not have replicated. + // The fallback must read the shard's own primary. Reading the gen-1 new writer would + // query the wrong database and return null. + const id = generateRunOpsIdV2("a"); + const environmentId = generateRunOpsId(); + const projectId = generateRunOpsId(); + const seeded = await seedWaitpoint(prisma17, id, { id: environmentId, projectId }); + + const shardReplica = recording(prisma14); // lags: does not have the row + const shardWriter = recording(prisma17); // has the row + const forbiddenNewPrimary = recording(prisma17, { forbidden: true }); + + const result = await resolveWaitpointThroughReadThrough({ + waitpointId: id, + environmentId, + read: read(id, environmentId), + deps: { + splitEnabled: true, + newClient: recording(prisma14, { forbidden: true }).handle, + legacyReplica: recording(prisma14, { forbidden: true }).handle, + newPrimary: forbiddenNewPrimary.handle, + shardReplicas: new Map([["a", shardReplica.handle]]), + shardWriters: new Map([["a", shardWriter.handle]]), + }, + }); + + expect(result).not.toBeNull(); + expect(result!.id).toBe(seeded.id); + expect(shardReplica.calls.length).toBe(1); + expect(shardWriter.calls.length).toBe(1); + expect(forbiddenNewPrimary.calls.length).toBe(0); + } + ); + + heteroRunOpsPostgresTest( + "a gen-2 waitpoint with no configured shard writer returns null instead of reading a wrong database", + async ({ prisma17, prisma14 }) => { + const id = generateRunOpsIdV2("a"); + const environmentId = generateRunOpsId(); + const projectId = generateRunOpsId(); + await seedWaitpoint(prisma17, id, { id: environmentId, projectId }); + + const forbiddenNewPrimary = recording(prisma17, { forbidden: true }); + + const result = await resolveWaitpointThroughReadThrough({ + waitpointId: id, + environmentId, + read: read(id, environmentId), + deps: { + splitEnabled: true, + newClient: recording(prisma14, { forbidden: true }).handle, + legacyReplica: recording(prisma14, { forbidden: true }).handle, + newPrimary: forbiddenNewPrimary.handle, + shardReplicas: new Map([["a", recording(prisma14).handle]]), + shardWriters: new Map(), + }, + }); + + expect(result).toBeNull(); + expect(forbiddenNewPrimary.calls.length).toBe(0); + } + ); }); diff --git a/apps/webapp/test/unroutableIdStatus.test.ts b/apps/webapp/test/unroutableIdStatus.test.ts new file mode 100644 index 00000000000..89135b44e15 --- /dev/null +++ b/apps/webapp/test/unroutableIdStatus.test.ts @@ -0,0 +1,40 @@ +// `resolveShard` is pure id-shape, so any base32hex core plus `[a-z0-9]` plus "2" parses as gen-2 +// and names a shard — including one a caller invents. The routing store throws for a key it has no +// store for, which is correct and deliberately loud, but a read route that lets it reach the +// boundary answered 500 for caller-supplied input. These tests pin the boundary status. +import { describe, expect, it } from "vitest"; +import { json } from "@remix-run/server-runtime"; +import { UnknownShardKey } from "@internal/run-store"; +import { unroutableIdResponse } from "~/services/routeBuilders/unroutableId.server"; + +describe("unroutableIdResponse", () => { + it("answers 404 for an id naming a shard with no configured store", async () => { + const response = unroutableIdResponse(new UnknownShardKey("z", ["legacy", "new"])); + + expect(response).toBeDefined(); + expect(response!.status).toBe(404); + // Not retryable: no number of retries makes a topology grow a store. Contrast the + // waitpoint wait route, whose 404 IS retryable because a miss there can be replica lag. + expect(response!.headers.get("x-should-retry")).toBe("false"); + await expect(response!.json()).resolves.toEqual({ error: "Not Found" }); + }); + + it("declines an unrelated error so it still reaches the 500 path", () => { + expect(unroutableIdResponse(new Error("db down"))).toBeUndefined(); + expect(unroutableIdResponse(undefined)).toBeUndefined(); + expect(unroutableIdResponse("a string")).toBeUndefined(); + }); + + it("declines a deliberately thrown Response, which carries its own status", () => { + expect(unroutableIdResponse(json({ error: "nope" }, { status: 422 }))).toBeUndefined(); + }); + + it("keeps the key and the configured set on the error for the operator", () => { + // A 404 to the caller must not cost the operator what separates a forged id from a shard + // key dropped out of a config that is meant to be append-only. + const error = new UnknownShardKey("z", ["legacy", "new", "a"]); + + expect(error.shardKey).toBe("z"); + expect(error.configured).toContain("a"); + }); +}); diff --git a/internal-packages/run-store/src/PostgresRunStore.ts b/internal-packages/run-store/src/PostgresRunStore.ts index 33604d01148..a6541f8beeb 100644 --- a/internal-packages/run-store/src/PostgresRunStore.ts +++ b/internal-packages/run-store/src/PostgresRunStore.ts @@ -30,6 +30,7 @@ import type { TaskRunWithWaitpoint, } from "./types.js"; import type { TaskRunError } from "@trigger.dev/core/v3/schemas"; +import type { ShardKey } from "@trigger.dev/core/v3/isomorphic"; // Loose delegate method shape: each generated client types delegate methods as // `(args: PackageLocalArgs) => PrismaPromise<…>` against its own nominal @@ -2669,7 +2670,7 @@ export class PostgresRunStore implements RunStore { data: { environmentId: string; name: string; projectId: string; id?: string }, tx?: PrismaClientOrTransaction, // `residency` selects the store at the router; a single store has one client and ignores it. - _residency?: "NEW" | "LEGACY" + _residency?: ShardKey ): Promise { const prisma = tx ?? this.prisma; diff --git a/internal-packages/run-store/src/runOpsStore.shardMap.test.ts b/internal-packages/run-store/src/runOpsStore.shardMap.test.ts index b6ea71e9f60..8f2cc8c6485 100644 --- a/internal-packages/run-store/src/runOpsStore.shardMap.test.ts +++ b/internal-packages/run-store/src/runOpsStore.shardMap.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { RoutingRunStore } from "./runOpsStore.js"; +import { RoutingRunStore, UnknownShardKey } from "./runOpsStore.js"; import type { ReadClient, RunStore } from "./types.js"; // Pins the routing ALGEBRA: probe order, merge precedence, and the two id-less fallbacks that @@ -278,6 +278,65 @@ describe("RoutingRunStore id-to-shard-key seam", () => { ); expect(trace(log)).toEqual([]); }); + + // The case above injects a resolver. This one does NOT: it uses the real `resolveShard`, which + // the compat constructor defaults to. `resolveShard` is pure id-shape, so a gen-2 shaped id + // names its shard char whatever the topology holds — the two-store compat router therefore + // reaches this throw for any gen-2 id, with no shard configured anywhere. + // + // That matters beyond this class: these ids reach read routes as URL parameters, so whatever + // sits above the router must translate this throw into a 4xx rather than let it surface as a + // 5xx that any caller can induce. + it("reaches the unconfigured-shard throw for a real gen-2 id, even on the compat pair", () => { + const log: Call[] = []; + const router = new RoutingRunStore({ + new: fakeStore("new", log), + legacy: fakeStore("legacy", log), + }); + + const genTwoId = `${"0".repeat(24)}a2`; + + expect(() => router.findRun({ id: genTwoId })).toThrow( + 'no store is configured for shard key "a"' + ); + expect(trace(log)).toEqual([]); + }); + + // Typed, not a bare Error: the API boundary matches on it to answer 404 instead of 500, and + // the operator needs the key and the configured set to tell a forged id from a dropped shard. + it("throws a typed UnknownShardKey carrying the key and the configured set", () => { + const log: Call[] = []; + const router = new RoutingRunStore({ + new: fakeStore("new", log), + legacy: fakeStore("legacy", log), + }); + + let thrown: unknown; + try { + router.findRun({ id: `${"0".repeat(24)}a2` }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(UnknownShardKey); + const error = thrown as UnknownShardKey; + expect(error.name).toBe("UnknownShardKey"); + expect(error.shardKey).toBe("a"); + expect([...error.configured].sort()).toEqual(["legacy", "new"]); + }); + + it("still routes gen-1 shapes on the compat pair with the real resolver", () => { + const log: Call[] = []; + const router = new RoutingRunStore({ + new: fakeStore("new", log), + legacy: fakeStore("legacy", log), + }); + + router.findRun({ id: `${"0".repeat(24)}01` }); + router.findRun({ id: "c".repeat(25) }); + + expect(trace(log)).toEqual(["new:findRun", "legacy:findRun"]); + }); }); function buildNShardRouter(shardKeys: string[], opts: { aliasOf?: Record } = {}) { @@ -301,6 +360,14 @@ function buildNShardRouter(shardKeys: string[], opts: { aliasOf?: Record { + // findRunsByIds reaches #fanOutPartitioned, the third unconfigured-shard guard. It must throw + // the typed error too, or this read path answers 500 where the boundary would give a 404. + it("throws a typed UnknownShardKey from the partitioned id fan-out", async () => { + const { router } = buildNShardRouter(["a"]); + + await expect(router.findRunsByIds(["a:r1", "z:r2"])).rejects.toBeInstanceOf(UnknownShardKey); + }); + it("routes an id to its gen-2 shard", async () => { const { router, log } = buildNShardRouter(["a", "b"]); await router.findRun({ id: "a:run_1" }); @@ -649,6 +716,17 @@ describe("RoutingRunStore countPendingWaitpoints — disjoint-sum partition", () ); }); + // The API boundary answers a non-retryable 404 by matching on the TYPE, so every + // unconfigured-shard guard has to throw the typed error and not a bare Error. Two other guards + // besides #shardStore reach an unconfigured key: this partition, and #fanOutPartitioned below. + it("throws a typed UnknownShardKey from the absent-id partition", async () => { + const { router } = partitionRouter({}); + + await expect( + router.countPendingWaitpoints(["c:w1"], undefined, "a:run") + ).rejects.toBeInstanceOf(UnknownShardKey); + }); + it("returns zero for an id absent everywhere", async () => { const { router } = partitionRouter({}); expect(await router.countPendingWaitpoints(["b:w9"], undefined, "a:run")).toBe(0); diff --git a/internal-packages/run-store/src/runOpsStore.ts b/internal-packages/run-store/src/runOpsStore.ts index 53089da21a5..7551e552b34 100644 --- a/internal-packages/run-store/src/runOpsStore.ts +++ b/internal-packages/run-store/src/runOpsStore.ts @@ -62,6 +62,28 @@ const LEGACY_SHARD: ShardKey = "legacy"; * and a merge lets a gen-2 shard win. A probe MUST iterate #probeOrder and a merge MUST iterate * #precedence. */ +/** + * An id resolved to a shard key the topology has no store for. Typed so a caller above the + * router can answer a 4xx instead of letting a routing failure surface as a 5xx: these ids + * arrive as URL parameters, and `resolveShard` is pure id-shape, so any gen-2 shaped id names + * a shard char whether or not one is configured. + */ +export class UnknownShardKey extends Error { + readonly shardKey: string; + readonly configured: string[]; + + constructor(shardKey: string, configured: string[], subject?: string) { + super( + subject === undefined + ? `RoutingRunStore: no store is configured for shard key "${shardKey}"` + : `RoutingRunStore: ${subject} resolves to unconfigured shard key "${shardKey}"` + ); + this.name = "UnknownShardKey"; + this.shardKey = shardKey; + this.configured = configured; + } +} + export class RoutingRunStore implements RunStore { readonly #shards: ReadonlyMap; // Sequential probe for a lookup with no routable id. The first non-null result wins, and the LAST @@ -173,12 +195,14 @@ export class RoutingRunStore implements RunStore { return client != null && !isReadReplicaClient(client) ? store.primaryReadClient : undefined; } - // The store for a shard key. Unreachable with the compat constructor — #shardKeyOfSafe yields only - // the two reserved keys — so this throw fires only if a caller wires a partial map. + // The store for a shard key. REACHABLE with the compat constructor: it defaults to the real + // `resolveShard`, which is pure id-shape, so any gen-2 shaped id names a shard char even when + // no shard is configured. Fails loud rather than reading the wrong database; the API boundary + // turns `UnknownShardKey` into a 404 so a caller-supplied id cannot induce a 5xx. #shardStore(key: ShardKey): RunStore { const store = this.#shards.get(key); if (store === undefined) { - throw new Error(`RoutingRunStore: no store is configured for shard key "${key}"`); + throw new UnknownShardKey(key, [...this.#shards.keys()]); } return store; } @@ -236,9 +260,7 @@ export class RoutingRunStore implements RunStore { // Fail loud instead (§7 append-only rule). if (key === runKey) return; if (!this.#shards.has(key)) { - throw new Error( - `RoutingRunStore: waitpoint "${id}" resolves to unconfigured shard key "${key}"` - ); + throw new UnknownShardKey(key, [...this.#shards.keys()], `waitpoint "${id}"`); } const bucket = byKey.get(key); if (bucket) bucket.push(id); @@ -393,7 +415,7 @@ export class RoutingRunStore implements RunStore { // An id resolving to a shard nobody configured is UnknownShardKey. Dropping it would silently // omit a row from the hydrated set, so fail loud (§7 append-only rule). if (!this.#shards.has(key)) { - throw new Error(`RoutingRunStore: id "${id}" resolves to unconfigured shard key "${key}"`); + throw new UnknownShardKey(key, [...this.#shards.keys()], `id "${id}"`); } const bucket = byShard.get(key); if (bucket) bucket.push(id);