-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(core,webapp,run-engine): stamp a shard key onto run, batch and waitpoint ids #4788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
65a354d
0dae1c5
b20792f
845ab06
bf32052
4359bf8
19731d4
25b2119
6d85a15
d28aec2
13f6124
45ee043
f9ad14c
b969f8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| * whole webapp service graph). The handlers wire the production defaults; tests | ||
| * inject per-container stores/replicas, so these helpers never import db.server. | ||
| */ | ||
| import { resolveShard } from "@trigger.dev/core/v3/isomorphic"; | ||
| import type { CompleteBatchResult } from "@internal/run-engine"; | ||
| import type { RunOpsPrismaClient } from "@internal/run-ops-database"; | ||
| import type { RunStore } from "@internal/run-store"; | ||
|
|
@@ -82,8 +83,25 @@ export async function resolveBatchRunOpsWriter( | |
| newReplica: RunOpsPrismaClient; | ||
| newWriter: RunOpsPrismaClient; | ||
| legacyWriter: RunOpsPrismaClient; | ||
| shards?: ReadonlyArray<{ key: string; writer: RunOpsPrismaClient }>; | ||
| } | ||
| ): Promise<RunOpsPrismaClient> { | ||
| // A gen-2 batch names its own shard in its id, so route by that and never probe. The | ||
| // probe below is binary — NEW, else assume LEGACY — so a gen-2 batch would fall through | ||
| // to a store that holds no such row, and the completion update would throw before the | ||
| // batch waitpoint could complete, leaving the parent run blocked with nothing logged. | ||
| const shardKey = resolveShard(batchId); | ||
| if (shardKey !== "new" && shardKey !== "legacy") { | ||
| const shard = deps.shards?.find((s) => s.key === shardKey); | ||
| if (!shard) { | ||
| // Writing to a guessed store is what strands a run. Fail loud instead. | ||
| throw new Error( | ||
| `resolveBatchRunOpsWriter: batch "${batchId}" names shard "${shardKey}", which is not configured` | ||
| ); | ||
| } | ||
| return shard.writer; | ||
|
Comment on lines
+93
to
+102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Add a crumb for shard writer selection. Record the batch ID and resolved shard key before this branch selects the writer. This new branch changes the database destination and has no temporary routing crumb. As per coding guidelines, “Add crumbs as you write code” and use an existing namespace or ask before creating one. Source: Coding guidelines |
||
| } | ||
|
|
||
| const onNew = await deps.newReplica.batchTaskRun.findFirst({ | ||
| where: { id: batchId }, | ||
| select: { id: true }, | ||
|
|
@@ -105,6 +123,7 @@ export type BatchCompletionDeps = { | |
| newReplica: RunOpsPrismaClient; | ||
| newWriter: RunOpsPrismaClient; | ||
| legacyWriter: RunOpsPrismaClient; | ||
| shards?: ReadonlyArray<{ key: string; writer: RunOpsPrismaClient }>; | ||
| tryCompleteBatch: (batchId: string) => Promise<unknown>; | ||
| }; | ||
|
|
||
|
|
@@ -135,6 +154,7 @@ export async function handleBatchCompletion( | |
| newReplica: deps.newReplica, | ||
| newWriter: deps.newWriter, | ||
| legacyWriter: deps.legacyWriter, | ||
| shards: deps.shards, | ||
| }); | ||
|
|
||
| try { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
| import { classifyKind, mintWaitpointIdFor, resolveShard } from "@trigger.dev/core/v3/isomorphic"; | ||
| import { resolveInheritedMintKind } from "./resolveInheritedMintKind.server"; | ||
| import { | ||
| mintAnchoredRunFriendlyId, | ||
| mintFriendlyIdForKind, | ||
| } from "./mintAnchoredRunFriendlyId.server"; | ||
| import { batchIdForMintKind } from "./mintBatchFriendlyId.server"; | ||
| import { resolveRunMintTarget } from "./resolveRunMintTarget.server"; | ||
|
|
||
| // The gate is off when RUN_OPS_SHARDS is unset OR runOpsMintShardSet is empty. Either way | ||
| // resolveMintShard answers "new", so no shard char reaches a MintTarget. Every assertion | ||
| // below is "the id is what it was before gen-2 existed". | ||
| const offShard = vi.fn().mockResolvedValue("new" as const); | ||
| const environment = { organizationId: "org_1", id: "env_1", orgFeatureFlags: {} }; | ||
|
|
||
| describe("gate off — run mint paths", () => { | ||
| it("a root run on the run-ops path mints a gen-1 v1 id", async () => { | ||
| const target = await resolveRunMintTarget({ | ||
| environment, | ||
| region: "us-east-1", | ||
| deps: { | ||
| resolveRunIdMintKind: vi.fn().mockResolvedValue("runOpsId"), | ||
| resolveMintShard: offShard, | ||
| }, | ||
| }); | ||
| const body = mintFriendlyIdForKind(target).slice(4); | ||
| expect(body.length).toBe(26); | ||
| expect(body[24]).toBe("e"); // the region char, as today | ||
| expect(body[25]).toBe("1"); | ||
| }); | ||
|
|
||
| it("a root run on a non-cut-over org mints a cuid", async () => { | ||
| const target = await resolveRunMintTarget({ | ||
| environment, | ||
| deps: { | ||
| resolveRunIdMintKind: vi.fn().mockResolvedValue("cuid"), | ||
| resolveMintShard: offShard, | ||
| }, | ||
| }); | ||
| expect(mintFriendlyIdForKind(target).slice(4).length).toBe(25); | ||
| }); | ||
|
|
||
| it("a child of a gen-1 parent keeps the caller's region char", async () => { | ||
| // The pre-split code passed the region on BOTH arms, so a child run stamped the | ||
| // requested region. Dropping it on the inherited arm would silently stamp the default. | ||
| const target = await resolveRunMintTarget({ | ||
| environment, | ||
| parentRunFriendlyId: `run_${"a".repeat(24)}01`, | ||
| region: "us-east-1", | ||
| deps: { | ||
| resolveRunIdMintKind: vi.fn().mockResolvedValue("runOpsId"), | ||
| resolveMintShard: offShard, | ||
| }, | ||
| }); | ||
| const body = mintFriendlyIdForKind(target).slice(4); | ||
| expect(body[24]).toBe("e"); | ||
| expect(body[25]).toBe("1"); | ||
| }); | ||
|
|
||
| it("a gen-2 parent's shard still outranks the caller's region", async () => { | ||
| const target = await resolveRunMintTarget({ | ||
| environment, | ||
| parentRunFriendlyId: `run_${"a".repeat(24)}a2`, | ||
| region: "us-east-1", | ||
| deps: { | ||
| resolveRunIdMintKind: vi.fn().mockResolvedValue("runOpsId"), | ||
| resolveMintShard: offShard, | ||
| }, | ||
| }); | ||
| expect(mintFriendlyIdForKind(target).slice(4)[24]).toBe("a"); | ||
| }); | ||
|
|
||
| it("a child of a gen-1 parent mints a gen-1 v1 id", () => { | ||
| const body = mintFriendlyIdForKind(resolveInheritedMintKind(`run_${"a".repeat(24)}01`)).slice( | ||
| 4 | ||
| ); | ||
| expect(body[25]).toBe("1"); | ||
| }); | ||
|
|
||
| it("a child of a cuid parent mints a cuid", () => { | ||
| expect( | ||
| mintFriendlyIdForKind(resolveInheritedMintKind(`run_${"b".repeat(25)}`)).slice(4).length | ||
| ).toBe(25); | ||
| }); | ||
| }); | ||
|
|
||
| describe("gate off — batch and item paths", () => { | ||
| it("a batch with no shard char mints a gen-1 v1 id", () => { | ||
| const r = batchIdForMintKind({ kind: "runOpsId" }); | ||
| expect(r.id.length).toBe(26); | ||
| expect(r.id[25]).toBe("1"); | ||
| expect(classifyKind(r.id)).toBe("runOpsId"); | ||
| }); | ||
|
|
||
| it("a batch on a non-cut-over org mints a cuid", () => { | ||
| expect(batchIdForMintKind({ kind: "cuid" }).id.length).toBe(25); | ||
| }); | ||
|
|
||
| it("a batch item anchored on a gen-1 batch mints a gen-1 v1 id", () => { | ||
| const body = mintAnchoredRunFriendlyId(`batch_${"a".repeat(24)}01`).slice(4); | ||
| expect(body[25]).toBe("1"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("gate off — waitpoint paths", () => { | ||
| it("every gen-1 or legacy anchor yields a cuid waitpoint id", () => { | ||
| for (const anchor of [`${"a".repeat(24)}01`, "c".repeat(25), undefined]) { | ||
| const r = mintWaitpointIdFor(anchor); | ||
| expect(r.id.length).toBe(25); | ||
| expect(resolveShard(r.id)).toBe("legacy"); | ||
| } | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,22 @@ | ||
| import { generateRunOpsId, RunId, type ResidencyKind } from "@trigger.dev/core/v3/isomorphic"; | ||
| import { generateRunOpsId, generateRunOpsIdV2, RunId } from "@trigger.dev/core/v3/isomorphic"; | ||
| import type { MintTarget } from "./mintTarget"; | ||
| import { resolveInheritedMintKind } from "./resolveInheritedMintKind.server"; | ||
|
|
||
| // Shared id-generation branch for every run-mint path: "runOpsId" -> NEW store, "cuid" -> LEGACY. | ||
| export function mintFriendlyIdForKind(mintKind: ResidencyKind, region?: string): string { | ||
| return mintKind === "runOpsId" | ||
| ? RunId.toFriendlyId(generateRunOpsId(region)) | ||
| : RunId.generate().friendlyId; | ||
| // Shared id-generation branch for every run-mint path: "runOpsId" -> a dedicated store, | ||
| // "cuid" -> LEGACY. A shardChar selects one gen-2 shard and takes index 24; without one, | ||
| // the region takes that slot exactly as it does today. | ||
| export function mintFriendlyIdForKind(target: MintTarget): string { | ||
| if (target.kind !== "runOpsId") { | ||
| return RunId.generate().friendlyId; | ||
| } | ||
|
|
||
| return RunId.toFriendlyId( | ||
| target.shardChar ? generateRunOpsIdV2(target.shardChar) : generateRunOpsId(target.region) | ||
| ); | ||
| } | ||
|
|
||
| // Anchor a batch item's mint on the BATCH's friendlyId (id-shape, zero I/O), never the per-org | ||
| // flag, so the item and its BatchTaskRun stay co-resident across a mid-batch flag flip. | ||
| export function mintAnchoredRunFriendlyId(batchFriendlyId: string, region?: string): string { | ||
| return mintFriendlyIdForKind(resolveInheritedMintKind(batchFriendlyId), region); | ||
| return mintFriendlyIdForKind({ ...resolveInheritedMintKind(batchFriendlyId), region }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Standalone waitpoint tags not shard-aware
A standalone token now mints its id onto a gen-2 shard via
standaloneShardKey, but its tags are still created with only the coarse NEW/LEGACYresidency.upsertWaitpointTagroutes tags to the gen-1 NEW store while the waitpoint row lands on the shard, so once a shard is configured the tag rows and the waitpoint row diverge across databases. Inert today becauseresolveMintShardreturns "new" whileRUN_OPS_SHARDSis empty. Confirm the sharded tag path accounts for this.(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.