-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(webapp): schedule run-ops mint-kind flips to avoid duplicate runs #4208
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
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. 🔍 Grace stamp is lost when all flags are cleared via the v2 route When the v2 admin route receives (Refers to lines 110-114) Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,11 @@ import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/server-r | |
| import { json } from "@remix-run/server-runtime"; | ||
| import { Prisma } from "@trigger.dev/database"; | ||
| import { z } from "zod"; | ||
| import { env } from "~/env.server"; | ||
| import { prisma } from "~/db.server"; | ||
| import { requireUser } from "~/services/session.server"; | ||
| import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server"; | ||
| import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace"; | ||
| import { flags as getGlobalFlags } from "~/v3/featureFlags.server"; | ||
| import { | ||
| FEATURE_FLAG, | ||
|
|
@@ -119,6 +121,17 @@ export async function action({ request, params }: ActionFunctionArgs) { | |
| ); | ||
| } | ||
| featureFlags = validationResult.data; | ||
|
|
||
| const existingOrg = await prisma.organization.findFirst({ | ||
| where: { id: organizationId }, | ||
| select: { featureFlags: true }, | ||
| }); | ||
| featureFlags = stampMintKindFlip( | ||
| existingOrg?.featureFlags as Record<string, unknown> | null | undefined, | ||
| featureFlags, | ||
| Date.now(), | ||
|
Collaborator
Author
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. suggestion: use control plane db time |
||
| env.RUN_OPS_MINT_FLIP_GRACE_MS | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { effectiveMintKind, stampMintKindFlip, type MintFlagResolution } from "./mintFlipGrace"; | ||
|
|
||
| // GRACE-LINGER: during [flippedAt, flippedAt + GRACE) every process — stale or fresh — | ||
| // must resolve to the SAME (old) kind; at/after the cutover every process resolves to | ||
| // the SAME (new) kind. This collapses the cross-process divergence window. | ||
| const GRACE_MS = 90_000; | ||
| const T = 1_000_000; | ||
|
|
||
| describe("effectiveMintKind", () => { | ||
| beforeEach(() => vi.useFakeTimers()); | ||
| afterEach(() => vi.useRealTimers()); | ||
|
|
||
| it("returns r.kind directly when prev is missing", () => { | ||
| const r: MintFlagResolution = { kind: "cuid" }; | ||
| expect(effectiveMintKind(r, T, GRACE_MS)).toBe("cuid"); | ||
| }); | ||
|
|
||
| it("returns r.kind directly when flippedAtMs is missing", () => { | ||
| const r: MintFlagResolution = { kind: "runOpsId", prev: "cuid" }; | ||
| expect(effectiveMintKind(r, T, GRACE_MS)).toBe("runOpsId"); | ||
| }); | ||
|
|
||
| it("CORE: stale and fresh resolutions agree at every instant during grace, then both flip to the new kind at cutover", () => { | ||
| const stale: MintFlagResolution = { kind: "cuid" }; | ||
| const fresh: MintFlagResolution = { kind: "runOpsId", prev: "cuid", flippedAtMs: T }; | ||
|
|
||
| for (const now of [T, T + 1, T + 1_000, T + 45_000, T + GRACE_MS - 1]) { | ||
| const staleResolved = effectiveMintKind(stale, now, GRACE_MS); | ||
| const freshResolved = effectiveMintKind(fresh, now, GRACE_MS); | ||
| expect(staleResolved).toBe("cuid"); | ||
| expect(freshResolved).toBe("cuid"); | ||
| } | ||
|
|
||
| for (const now of [T + GRACE_MS, T + GRACE_MS + 1, T + GRACE_MS + 60_000]) { | ||
| expect(effectiveMintKind(fresh, now, GRACE_MS)).toBe("runOpsId"); | ||
| } | ||
| }); | ||
|
|
||
| it("boundary: exactly at flippedAt + GRACE resolves to the NEW kind", () => { | ||
| const fresh: MintFlagResolution = { kind: "runOpsId", prev: "cuid", flippedAtMs: T }; | ||
| expect(effectiveMintKind(fresh, T + GRACE_MS - 1, GRACE_MS)).toBe("cuid"); | ||
| expect(effectiveMintKind(fresh, T + GRACE_MS, GRACE_MS)).toBe("runOpsId"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("stampMintKindFlip", () => { | ||
| beforeEach(() => vi.useFakeTimers()); | ||
| afterEach(() => vi.useRealTimers()); | ||
|
|
||
| it("a genuine flip (cuid -> runOpsId) stamps prev and flippedAt", () => { | ||
| const existing = { runOpsMintKind: "cuid" }; | ||
| const outgoing = { runOpsMintKind: "runOpsId" }; | ||
| const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("runOpsId"); | ||
| expect(result.runOpsMintKindPrev).toBe("cuid"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString()); | ||
| }); | ||
|
|
||
| it("defaults the existing effective kind to cuid when existing flags are null", () => { | ||
| const outgoing = { runOpsMintKind: "runOpsId" }; | ||
| const result = stampMintKindFlip(null, outgoing, T, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("runOpsId"); | ||
| expect(result.runOpsMintKindPrev).toBe("cuid"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString()); | ||
| }); | ||
|
|
||
| it("resubmitting the same target kind mid-grace carries the stamp forward untouched (does not reset the cutover clock)", () => { | ||
| const existing = { | ||
| runOpsMintKind: "runOpsId", | ||
| runOpsMintKindPrev: "cuid", | ||
| runOpsMintKindFlippedAt: new Date(T).toISOString(), | ||
| }; | ||
| const now = T + 10_000; | ||
| const outgoing = { runOpsMintKind: "runOpsId", someOtherFlag: true }; | ||
| const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("runOpsId"); | ||
| expect(result.runOpsMintKindPrev).toBe("cuid"); | ||
| // Unrelated re-save: the cutover time must NOT slide forward. | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString()); | ||
| expect(result.someOtherFlag).toBe(true); | ||
| }); | ||
|
|
||
| it("an unchanged save after grace has elapsed carries the settled stamp forward and preserves unrelated flags", () => { | ||
| const existing = { | ||
| runOpsMintKind: "runOpsId", | ||
| runOpsMintKindPrev: "cuid", | ||
| runOpsMintKindFlippedAt: new Date(T).toISOString(), | ||
| }; | ||
| const outgoing = { runOpsMintKind: "runOpsId", someOtherFlag: true }; | ||
| const result = stampMintKindFlip(existing, outgoing, T + GRACE_MS + 5_000, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("runOpsId"); | ||
| expect(result.someOtherFlag).toBe(true); | ||
| expect(result.runOpsMintKindPrev).toBe("cuid"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString()); | ||
| }); | ||
|
|
||
| it("a flip-back requested after the original grace has elapsed stamps prev := the new settled (now-effective) kind, timestamped now", () => { | ||
| const existing = { | ||
| runOpsMintKind: "runOpsId", | ||
| runOpsMintKindPrev: "cuid", | ||
| runOpsMintKindFlippedAt: new Date(T).toISOString(), | ||
| }; | ||
| const now = T + GRACE_MS + 1_000; | ||
| const outgoing = { runOpsMintKind: "cuid" }; | ||
| const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("cuid"); | ||
| expect(result.runOpsMintKindPrev).toBe("runOpsId"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(now).toISOString()); | ||
| }); | ||
|
|
||
| it("a flip-back mid-grace re-stamps prev to the still-effective old kind, so it keeps serving that kind (no divergence)", () => { | ||
| const existing = { | ||
| runOpsMintKind: "runOpsId", | ||
| runOpsMintKindPrev: "cuid", | ||
| runOpsMintKindFlippedAt: new Date(T).toISOString(), | ||
| }; | ||
| const now = T + 20_000; | ||
| const outgoing = { runOpsMintKind: "cuid" }; | ||
| const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("cuid"); | ||
| expect(result.runOpsMintKindPrev).toBe("cuid"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(now).toISOString()); | ||
| }); | ||
|
|
||
| it("defaults outgoing kind to 'cuid' when runOpsMintKind is absent", () => { | ||
| const existing = { runOpsMintKind: "runOpsId" }; | ||
| const outgoing: Record<string, unknown> = {}; | ||
| const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("cuid"); | ||
| expect(result.runOpsMintKindPrev).toBe("runOpsId"); | ||
| expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString()); | ||
| }); | ||
|
|
||
| it("treats a malformed existing flippedAt as no stamp", () => { | ||
| const existing = { | ||
| runOpsMintKind: "runOpsId", | ||
| runOpsMintKindPrev: "cuid", | ||
| runOpsMintKindFlippedAt: "not-a-date", | ||
| }; | ||
| const outgoing = { runOpsMintKind: "runOpsId" }; | ||
| const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS); | ||
|
|
||
| expect(result.runOpsMintKind).toBe("runOpsId"); | ||
| // The malformed flippedAt is carried forward verbatim, but an unparseable timestamp is | ||
| // inert when resolved (Date.parse -> NaN -> effectiveMintKind returns the target kind). | ||
| expect(result.runOpsMintKindFlippedAt).toBe("not-a-date"); | ||
| expect( | ||
| effectiveMintKind({ kind: "runOpsId", prev: "cuid", flippedAtMs: NaN }, T, GRACE_MS) | ||
| ).toBe("runOpsId"); | ||
| }); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import type { RunIdMintKind } from "./runOpsMintKind.server"; | ||
|
|
||
| export type { RunIdMintKind }; | ||
|
|
||
| export type MintFlagResolution = { | ||
| kind: RunIdMintKind; | ||
| prev?: RunIdMintKind; | ||
| flippedAtMs?: number; | ||
| }; | ||
|
|
||
| const DEFAULT_MINT_KIND: RunIdMintKind = "cuid"; | ||
|
|
||
| // Deterministic wall-clock cutover: during [flippedAt, flippedAt + graceMs) every | ||
| // process — stale (hasn't re-read the flag) or fresh (has the new stamp) — resolves to | ||
| // the OLD kind. At/after the cutover, every process resolves to the NEW kind. | ||
| export function effectiveMintKind( | ||
| r: MintFlagResolution, | ||
| nowMs: number, | ||
| graceMs: number | ||
| ): RunIdMintKind { | ||
| if (r.prev === undefined || r.flippedAtMs === undefined) { | ||
| return r.kind; | ||
| } | ||
| return nowMs < r.flippedAtMs + graceMs ? r.prev : r.kind; | ||
| } | ||
|
|
||
| function readMintKind(flags: Record<string, unknown>, key: string): RunIdMintKind | undefined { | ||
| const value = flags[key]; | ||
| return value === "cuid" || value === "runOpsId" ? value : undefined; | ||
| } | ||
|
|
||
| function resolveEffectiveFromFlags( | ||
| flags: Record<string, unknown> | null | undefined, | ||
| nowMs: number, | ||
| graceMs: number | ||
| ): RunIdMintKind { | ||
| const source = flags ?? {}; | ||
| const kind = readMintKind(source, "runOpsMintKind") ?? DEFAULT_MINT_KIND; | ||
| const prev = readMintKind(source, "runOpsMintKindPrev"); | ||
| const flippedAtRaw = source.runOpsMintKindFlippedAt; | ||
| const parsed = typeof flippedAtRaw === "string" ? Date.parse(flippedAtRaw) : NaN; | ||
| const flippedAtMs = Number.isNaN(parsed) ? undefined : parsed; | ||
|
|
||
| return effectiveMintKind({ kind, prev, flippedAtMs }, nowMs, graceMs); | ||
| } | ||
|
|
||
| // Stamps a grace window only when the outgoing TARGET kind differs from the stored one (a | ||
| // genuine flip); prev := the currently-effective kind. A save that leaves the target kind | ||
| // unchanged carries any in-flight stamp forward, so it can't reset the cutover clock. | ||
| export function stampMintKindFlip( | ||
| existingFlags: Record<string, unknown> | null | undefined, | ||
| outgoingFlags: Record<string, unknown>, | ||
| nowMs: number, | ||
| graceMs: number | ||
| ): Record<string, unknown> { | ||
| const storedKind = readMintKind(existingFlags ?? {}, "runOpsMintKind") ?? DEFAULT_MINT_KIND; | ||
| const outgoingKind = readMintKind(outgoingFlags, "runOpsMintKind") ?? DEFAULT_MINT_KIND; | ||
| outgoingFlags.runOpsMintKind = outgoingKind; | ||
|
|
||
| if (outgoingKind !== storedKind) { | ||
| // Genuine target change: serve the currently-effective kind through the new grace window. | ||
| outgoingFlags.runOpsMintKindPrev = resolveEffectiveFromFlags(existingFlags, nowMs, graceMs); | ||
| outgoingFlags.runOpsMintKindFlippedAt = new Date(nowMs).toISOString(); | ||
| return outgoingFlags; | ||
| } | ||
|
|
||
| const existing = existingFlags ?? {}; | ||
| const existingPrev = existing.runOpsMintKindPrev; | ||
| const existingFlippedAt = existing.runOpsMintKindFlippedAt; | ||
| if (existingPrev !== undefined) { | ||
| outgoingFlags.runOpsMintKindPrev = existingPrev; | ||
| } | ||
| if (existingFlippedAt !== undefined) { | ||
| outgoingFlags.runOpsMintKindFlippedAt = existingFlippedAt; | ||
| } | ||
| return outgoingFlags; | ||
| } |
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.
issue: Use control plane DB time