From 6865d478b569a6b25432ba4ffbf98e8202746621 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Wed, 19 Aug 2026 11:59:09 +0100 Subject: [PATCH 1/2] perf(webapp): resolve schedule list run times per expression, not per row Listing schedules walked the cron expression three times for every row: once backwards to approximate last run, and twice forwards to get the next run and the interval after it. Each walk steps the calendar unit by unit, so a full page of timezone-aware schedules could block the event loop for seconds. Run times now resolve for the whole page at once. Nominal times are cached per (cron, timezone) against a single pinned now, so cost scales with the number of distinct expressions rather than the number of rows. The backwards walk is opt-in and only the dashboard, which renders the column, asks for it. Windowless schedules take one step instead of two, since with no window the interval to the following occurrence cannot affect the result. --- .../v3/ScheduleListPresenter.server.ts | 58 ++- .../route.tsx | 1 + apps/webapp/app/v3/scheduleTimings.server.ts | 181 +++++++++ .../v3/utils/calculateNextSchedule.server.ts | 19 +- .../webapp/test/scheduleTimings.bench.test.ts | 166 +++++++++ apps/webapp/test/scheduleTimings.test.ts | 342 ++++++++++++++++++ 6 files changed, 727 insertions(+), 40 deletions(-) create mode 100644 apps/webapp/app/v3/scheduleTimings.server.ts create mode 100644 apps/webapp/test/scheduleTimings.bench.test.ts create mode 100644 apps/webapp/test/scheduleTimings.test.ts diff --git a/apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts b/apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts index e81918ddb03..a4af281a8ec 100644 --- a/apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts @@ -5,9 +5,9 @@ import { getTaskIdentifiers } from "~/models/task.server"; import { getCurrentPlan, getPlans } from "~/services/platform.v3.server"; import { findCurrentWorkerFromEnvironment } from "~/v3/models/workerDeployment.server"; import { ServiceValidationError } from "~/v3/services/baseService.server"; -import { calculateNextScheduleRunTimes, formatScheduleWindow } from "~/v3/scheduleWindow.server"; +import { formatScheduleWindow } from "~/v3/scheduleWindow.server"; import { CheckScheduleService } from "~/v3/services/checkSchedule.server"; -import { previousScheduledTimestamp } from "~/v3/utils/calculateNextSchedule.server"; +import { resolveScheduleTimings } from "~/v3/scheduleTimings.server"; import { env } from "~/env.server"; import { BasePresenter } from "./basePresenter.server"; @@ -16,6 +16,12 @@ type ScheduleListOptions = { environmentId: string; userId?: string; pageSize?: number; + /** + * Walking each cron backwards to approximate "last run" costs an order of + * magnitude more than everything else here, so it is opt-in: only the + * dashboard renders the column. Defaults off. + */ + includeLastRun?: boolean; } & ScheduleListFilters; const DEFAULT_PAGE_SIZE = 20; @@ -54,6 +60,7 @@ export class ScheduleListPresenter extends BasePresenter { page, type, pageSize = DEFAULT_PAGE_SIZE, + includeLastRun = false, }: ScheduleListOptions) { const hasFilters = type !== undefined || tasks !== undefined || (search !== undefined && search !== ""); @@ -274,46 +281,33 @@ export class ScheduleListPresenter extends BasePresenter { skip: (page - 1) * pageSize, }); - const schedules: ScheduleListItem[] = rawSchedules.map((schedule) => { - // Approximate "last run" from the cron's previous slot. Skip inactive - // schedules — the cron's previous slot reflects what *would* have - // fired, but a deactivated schedule didn't actually fire there. Skip - // when the cron's previous slot predates `updatedAt`: any config - // change (cron edited, timezone changed, deactivate/reactivate) - // bumps updatedAt, and a slot from before the most recent change - // didn't fire under the current configuration. cron-parser throws - // on malformed expressions, so degrade to undefined per-row rather - // than failing the whole list. UI is best-effort; the runs page is - // the source of truth. - let lastRun: Date | undefined; - if (schedule.active) { - try { - const cronPrev = previousScheduledTimestamp( - schedule.generatorExpression, - schedule.timezone - ); - lastRun = cronPrev.getTime() > schedule.updatedAt.getTime() ? cronPrev : undefined; - } catch { - lastRun = undefined; - } - } - + const instances = rawSchedules.map((schedule) => { const instance = schedule.instances.find( (instance) => instance.environmentId === environmentId ); if (!instance) { throw new Error(`Schedule instance not found for environment: ${environmentId}`); } - const [nextRun] = calculateNextScheduleRunTimes({ + return instance; + }); + + const timings = resolveScheduleTimings( + rawSchedules.map((schedule, index) => ({ cron: schedule.generatorExpression, timezone: schedule.timezone, deduplicationKey: schedule.deduplicationKey, environmentId, - schedulePhase: instance.schedulePhase, - phaseSecret: env.ENCRYPTION_KEY, + schedulePhase: instances[index].schedulePhase, windowDurationSeconds: schedule.windowDurationSeconds, windowPercentage: schedule.windowPercentage, - }); + active: schedule.active, + updatedAt: schedule.updatedAt, + })), + { phaseSecret: env.ENCRYPTION_KEY, includeLastRun } + ); + + const schedules: ScheduleListItem[] = rawSchedules.map((schedule, index) => { + const { nextRun, nextRunEffectiveAt, lastRun } = timings[index]; return { id: schedule.id, @@ -329,8 +323,8 @@ export class ScheduleListPresenter extends BasePresenter { active: schedule.active, externalId: schedule.externalId, lastRun, - nextRun: nextRun.nominalAt, - nextRunEffectiveAt: nextRun.effectiveAt, + nextRun, + nextRunEffectiveAt, environments: schedule.instances.map((instance) => { const environment = project.environments.find((env) => env.id === instance.environmentId); if (!environment) { diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx index 60a3aaf1043..1e377cd4558 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx @@ -205,6 +205,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { tasks: [task.slug], page: schedulesPage, pageSize: 25, + includeLastRun: true, }) .catch(() => null); diff --git a/apps/webapp/app/v3/scheduleTimings.server.ts b/apps/webapp/app/v3/scheduleTimings.server.ts new file mode 100644 index 00000000000..b30b7050d3b --- /dev/null +++ b/apps/webapp/app/v3/scheduleTimings.server.ts @@ -0,0 +1,181 @@ +import { + MINIMUM_SCHEDULE_RANGE_MS, + calculateEffectiveScheduleTime, + calculateSchedulePhase, +} from "@internal/schedule-engine"; +import { type NormalizedScheduleWindow } from "@trigger.dev/core/v3"; +import { + nextScheduledTimestamps, + previousScheduledTimestamp, +} from "./utils/calculateNextSchedule.server"; + +/** + * Everything a single row needs to have its run times resolved. Deliberately + * free of Prisma types so this stays testable and benchmarkable on its own. + */ +export type ScheduleTimingInput = { + cron: string; + timezone: string | null; + deduplicationKey: string; + environmentId: string; + schedulePhase: number | null; + windowDurationSeconds: number | null; + windowPercentage: number | null; + active: boolean; + updatedAt: Date; +}; + +export type ScheduleTiming = { + nextRun: Date; + nextRunEffectiveAt: Date; + /** Only ever set when the caller asked for it AND the schedule is active. */ + lastRun: Date | undefined; +}; + +export type ResolveScheduleTimingsOptions = { + phaseSecret: string; + /** + * Walking the cron backwards to approximate "last run" is by far the most + * expensive thing here, and only the dashboard renders it. Callers that + * don't show the column (the public API) leave this off and skip the walk. + */ + includeLastRun: boolean; + /** + * Fixed reference point for the whole batch. Pinning it once is what makes + * the cron walks cacheable across rows, and it stops rows in one response + * disagreeing about "now". + */ + now?: Date; +}; + +/** + * Resolves run times for a page of schedules. + * + * The cron walk (`cron-parser`) dominates this path: one step costs tens of + * microseconds for a plain UTC expression and milliseconds for a sparse one in + * a named timezone, because the library walks the calendar unit by unit + * through luxon. At 100 rows that is enough to block the event loop for + * seconds. + * + * Two properties keep it cheap: + * + * 1. Nominal run times depend only on (cron, timezone, now). With `now` pinned + * for the batch, rows sharing an expression share an answer, so cost is + * O(distinct crons) rather than O(rows) — projects tend to run the same + * handful of expressions across many schedules. + * 2. Everything that genuinely varies per row (phase, window, effectiveAt) is + * arithmetic over the cached nominal times, not another walk. + * 3. Windowless schedules take one step instead of two. The second step exists + * only to measure the interval to the following occurrence, and the + * interval reaches `calculateEffectiveScheduleTime`'s result solely through + * `min(intervalMs, max(MINIMUM_SCHEDULE_RANGE_MS, windowMs))`. With no + * window `windowMs` is 0, and `CronPattern` rejects expressions with a + * seconds field, so consecutive occurrences are always at least + * `MINIMUM_SCHEDULE_RANGE_MS` apart and that `min` can never bind. Stepping + * a second time would change nothing, and it is the more expensive of the + * two steps because it walks a whole period rather than the remainder of + * the current one. + * + * Caches live for one call only: every entry is valid solely against this + * batch's `now`. + */ +export function resolveScheduleTimings( + inputs: ScheduleTimingInput[], + { phaseSecret, includeLastRun, now = new Date() }: ResolveScheduleTimingsOptions +): ScheduleTiming[] { + const nominalCache = new Map(); + const previousCache = new Map(); + + return inputs.map((input) => { + const window: NormalizedScheduleWindow | undefined = + input.windowPercentage !== null + ? { type: "percentage", percentage: input.windowPercentage } + : input.windowDurationSeconds !== null + ? { type: "duration", durationSeconds: input.windowDurationSeconds } + : undefined; + + const steps = window ? 2 : 1; + const key = `${cacheKey(input.cron, input.timezone)}\n${steps}`; + + let nominalTimes = nominalCache.get(key); + if (!nominalTimes) { + nominalTimes = nextScheduledTimestamps(input.cron, input.timezone, now, steps); + nominalCache.set(key, nominalTimes); + } + + const nominalAt = nominalTimes[0]; + const nextNominalAt = + nominalTimes[1] ?? new Date(nominalAt.getTime() + MINIMUM_SCHEDULE_RANGE_MS); + + const phase = + input.schedulePhase ?? + calculateSchedulePhase({ + secret: phaseSecret, + environmentId: input.environmentId, + deduplicationKey: input.deduplicationKey, + }); + + const { effectiveAt } = calculateEffectiveScheduleTime({ + nominalAt, + nextNominalAt, + schedulePhase: phase, + window, + }); + + return { + nextRun: nominalAt, + nextRunEffectiveAt: effectiveAt, + lastRun: includeLastRun ? resolveLastRun(input, now, previousCache) : undefined, + }; + }); +} + +/** + * Approximates "last run" from the cron's previous slot. + * + * Skips inactive schedules — the previous slot reflects what *would* have + * fired. Skips slots that predate `updatedAt`: any config change (cron edited, + * timezone changed, deactivate/reactivate) bumps `updatedAt`, and a slot from + * before the most recent change didn't fire under the current configuration. + * + * `cron-parser` throws on malformed expressions, so this degrades to undefined + * per row rather than failing the whole list. Best-effort by design; the runs + * page is the source of truth. + */ +function resolveLastRun( + input: ScheduleTimingInput, + now: Date, + cache: Map +): Date | undefined { + if (!input.active) { + return undefined; + } + + const key = cacheKey(input.cron, input.timezone); + + let previous: Date | undefined; + if (cache.has(key)) { + previous = cache.get(key); + } else { + try { + previous = previousScheduledTimestamp(input.cron, input.timezone, now); + } catch { + previous = undefined; + } + cache.set(key, previous); + } + + if (!previous) { + return undefined; + } + + return previous.getTime() > input.updatedAt.getTime() ? previous : undefined; +} + +/** + * Newline separator: an IANA timezone name cannot contain one, so no + * (cron, timezone) pair can collide with another by straddling the boundary. + */ +function cacheKey(cron: string, timezone: string | null): string { + return `${timezone ?? ""}\n${cron}`; +} diff --git a/apps/webapp/app/v3/utils/calculateNextSchedule.server.ts b/apps/webapp/app/v3/utils/calculateNextSchedule.server.ts index 5b645807580..52a3423aad0 100644 --- a/apps/webapp/app/v3/utils/calculateNextSchedule.server.ts +++ b/apps/webapp/app/v3/utils/calculateNextSchedule.server.ts @@ -36,23 +36,26 @@ export function previousScheduledTimestamp( .toDate(); } +/** + * Steps one parsed expression `count` times, rather than re-parsing and + * re-walking the calendar from scratch for every step. + */ export function nextScheduledTimestamps( cron: string, timezone: string | null, lastScheduledTimestamp: Date, count: number = 1 ) { + const interval = parseExpression(cron, { + currentDate: lastScheduledTimestamp, + utc: timezone === null, + tz: timezone ?? undefined, + }); + const result: Array = []; - let nextScheduledTimestamp = lastScheduledTimestamp; for (let i = 0; i < count; i++) { - nextScheduledTimestamp = calculateNextScheduledTimestamp( - cron, - timezone, - nextScheduledTimestamp - ); - - result.push(nextScheduledTimestamp); + result.push(interval.next().toDate()); } return result; diff --git a/apps/webapp/test/scheduleTimings.bench.test.ts b/apps/webapp/test/scheduleTimings.bench.test.ts new file mode 100644 index 00000000000..7e909fc77b9 --- /dev/null +++ b/apps/webapp/test/scheduleTimings.bench.test.ts @@ -0,0 +1,166 @@ +import { calculateEffectiveScheduleTime, calculateSchedulePhase } from "@internal/schedule-engine"; +import { parseExpression } from "cron-parser"; +import { describe, expect, it } from "vitest"; +import { resolveScheduleTimings, type ScheduleTimingInput } from "~/v3/scheduleTimings.server"; + +const PHASE_SECRET = "bench-phase-secret"; +const PAGE_SIZE = 100; + +/** + * The shape this path had before the optimization: for every row, walk the + * cron backwards once for `lastRun` and forwards twice for the next two + * nominal times, re-parsing the expression each time. + */ +function legacyResolve(inputs: ScheduleTimingInput[], now: Date) { + return inputs.map((input) => { + let lastRun: Date | undefined; + if (input.active) { + try { + const previous = parseExpression(input.cron, { + currentDate: now, + utc: input.timezone === null, + tz: input.timezone ?? undefined, + }) + .prev() + .toDate(); + lastRun = previous.getTime() > input.updatedAt.getTime() ? previous : undefined; + } catch { + lastRun = undefined; + } + } + + const nominalTimes: Date[] = []; + let cursor = now; + for (let i = 0; i < 2; i++) { + cursor = parseExpression(input.cron, { + currentDate: cursor, + utc: input.timezone === null, + tz: input.timezone ?? undefined, + }) + .next() + .toDate(); + nominalTimes.push(cursor); + } + + const phase = + input.schedulePhase ?? + calculateSchedulePhase({ + secret: PHASE_SECRET, + environmentId: input.environmentId, + deduplicationKey: input.deduplicationKey, + }); + + const { effectiveAt } = calculateEffectiveScheduleTime({ + nominalAt: nominalTimes[0], + nextNominalAt: nominalTimes[1], + schedulePhase: phase, + window: undefined, + }); + + return { nextRun: nominalTimes[0], nextRunEffectiveAt: effectiveAt, lastRun }; + }); +} + +function page(crons: Array<[string, string | null]>): ScheduleTimingInput[] { + return Array.from({ length: PAGE_SIZE }, (_, index) => { + const [cron, timezone] = crons[index % crons.length]; + return { + cron, + timezone, + deduplicationKey: `dedup-${index}`, + environmentId: `env-${index % 5}`, + schedulePhase: null, + windowDurationSeconds: null, + windowPercentage: null, + active: true, + updatedAt: new Date("2020-01-01T00:00:00.000Z"), + }; + }); +} + +function timeIt(label: string, fn: () => unknown): number { + fn(); + const started = process.hrtime.bigint(); + fn(); + const ms = Number(process.hrtime.bigint() - started) / 1e6; + // eslint-disable-next-line no-console + console.log(` ${label.padEnd(34)} ${ms.toFixed(1).padStart(8)} ms / ${PAGE_SIZE} rows`); + return ms; +} + +const SCENARIOS: Array<{ + name: string; + crons: Array<[string, string | null]>; + minSpeedup: number; +}> = [ + { + name: "one shared UTC expression", + crons: [["0 0 * * *", null]], + minSpeedup: 30, + }, + { + name: "one shared timezone expression", + crons: [["0 9 * * 1-5", "Europe/London"]], + minSpeedup: 100, + }, + { + name: "a handful of timezone expressions", + crons: [ + ["0 0 * * *", "America/New_York"], + ["0 9 * * 1-5", "Europe/London"], + ["30 2 1 * *", "Asia/Tokyo"], + ["15 3 * * 0", "Australia/Sydney"], + ], + minSpeedup: 50, + }, + { + name: "sparse expressions (worst case)", + crons: [ + ["0 0 1 1 *", "America/New_York"], + ["0 0 29 2 *", "America/New_York"], + ], + minSpeedup: 20, + }, + { + name: "every row distinct (no cache hits)", + crons: Array.from( + { length: PAGE_SIZE }, + (_, index) => + [`${index % 60} ${Math.floor(index / 60)} * * *`, "Europe/London"] as [ + string, + string | null, + ] + ), + minSpeedup: 4, + }, +]; + +describe("resolveScheduleTimings CPU", () => { + const now = new Date("2024-06-15T09:17:23.000Z"); + + it.each(SCENARIOS)( + "beats the pre-optimization shape by at least $minSpeedup x: $name", + ({ name, crons, minSpeedup }) => { + const inputs = page(crons); + + // eslint-disable-next-line no-console + console.log(`\n${name}`); + const legacy = timeIt("legacy (per-row, with lastRun)", () => legacyResolve(inputs, now)); + const optimized = timeIt("optimized (API path)", () => + resolveScheduleTimings(inputs, { phaseSecret: PHASE_SECRET, includeLastRun: false, now }) + ); + const withLastRun = timeIt("optimized (dashboard path)", () => + resolveScheduleTimings(inputs, { phaseSecret: PHASE_SECRET, includeLastRun: true, now }) + ); + + // eslint-disable-next-line no-console + console.log( + ` => ${(legacy / optimized).toFixed(1)}x faster on the API path, ` + + `${(legacy / withLastRun).toFixed(1)}x with lastRun` + ); + + expect(legacy / optimized).toBeGreaterThan(minSpeedup); + expect(withLastRun).toBeLessThan(legacy * 1.25); + } + ); +}); diff --git a/apps/webapp/test/scheduleTimings.test.ts b/apps/webapp/test/scheduleTimings.test.ts new file mode 100644 index 00000000000..dc5a0e7c7d1 --- /dev/null +++ b/apps/webapp/test/scheduleTimings.test.ts @@ -0,0 +1,342 @@ +import { + MINIMUM_SCHEDULE_RANGE_MS, + calculateEffectiveScheduleTime, + calculateSchedulePhase, +} from "@internal/schedule-engine"; +import { CronPattern } from "~/v3/schedules"; +import { type NormalizedScheduleWindow } from "@trigger.dev/core/v3"; +import { parseExpression } from "cron-parser"; +import { describe, expect, it } from "vitest"; +import { + nextScheduledTimestamps, + previousScheduledTimestamp, +} from "~/v3/utils/calculateNextSchedule.server"; +import { resolveScheduleTimings, type ScheduleTimingInput } from "~/v3/scheduleTimings.server"; + +const PHASE_SECRET = "test-phase-secret"; + +const CRONS: Array<[string, string | null]> = [ + ["*/5 * * * *", null], + ["0 * * * *", null], + ["0 0 * * *", null], + ["0 0 * * *", "America/New_York"], + ["0 9 * * 1-5", "Europe/London"], + ["30 2 1 * *", "Asia/Tokyo"], + ["15 3 * * 0", "Australia/Sydney"], + ["0 0 1 1 *", "America/New_York"], + ["0 0 29 2 *", "America/New_York"], + ["0 0 31 * *", "UTC"], + ["*/13 */7 * * *", "Pacific/Chatham"], +]; + +/** + * The pre-optimization implementation: re-parse and re-walk from scratch for + * every step. Kept here so the optimized version is checked against the exact + * behaviour it replaced rather than against hand-written expectations. + */ +function referenceNextScheduledTimestamps( + cron: string, + timezone: string | null, + from: Date, + count: number +): Date[] { + const result: Date[] = []; + let cursor = from; + + for (let i = 0; i < count; i++) { + cursor = parseExpression(cron, { + currentDate: cursor, + utc: timezone === null, + tz: timezone ?? undefined, + }) + .next() + .toDate(); + + result.push(cursor); + } + + return result; +} + +/** Naive per-row resolution, with no caching and no skipping. */ +function referenceResolve( + inputs: ScheduleTimingInput[], + now: Date, + includeLastRun: boolean +): Array<{ nextRun: Date; nextRunEffectiveAt: Date; lastRun: Date | undefined }> { + return inputs.map((input) => { + const nominalTimes = referenceNextScheduledTimestamps(input.cron, input.timezone, now, 2); + + const phase = + input.schedulePhase ?? + calculateSchedulePhase({ + secret: PHASE_SECRET, + environmentId: input.environmentId, + deduplicationKey: input.deduplicationKey, + }); + + const window: NormalizedScheduleWindow | undefined = + input.windowPercentage !== null + ? { type: "percentage", percentage: input.windowPercentage } + : input.windowDurationSeconds !== null + ? { type: "duration", durationSeconds: input.windowDurationSeconds } + : undefined; + + const { effectiveAt } = calculateEffectiveScheduleTime({ + nominalAt: nominalTimes[0], + nextNominalAt: nominalTimes[1], + schedulePhase: phase, + window, + }); + + let lastRun: Date | undefined; + if (includeLastRun && input.active) { + try { + const previous = previousScheduledTimestamp(input.cron, input.timezone, now); + lastRun = previous.getTime() > input.updatedAt.getTime() ? previous : undefined; + } catch { + lastRun = undefined; + } + } + + return { nextRun: nominalTimes[0], nextRunEffectiveAt: effectiveAt, lastRun }; + }); +} + +function input(overrides: Partial = {}): ScheduleTimingInput { + return { + cron: "0 0 * * *", + timezone: null, + deduplicationKey: "dedup-1", + environmentId: "env-1", + schedulePhase: null, + windowDurationSeconds: null, + windowPercentage: null, + active: true, + updatedAt: new Date("2020-01-01T00:00:00.000Z"), + ...overrides, + }; +} + +describe("nextScheduledTimestamps", () => { + it.each(CRONS)("matches the re-parsing implementation for %s (%s)", (cron, timezone) => { + const from = new Date("2024-03-07T12:34:56.000Z"); + + for (const count of [1, 2, 3, 5]) { + expect(nextScheduledTimestamps(cron, timezone, from, count)).toEqual( + referenceNextScheduledTimestamps(cron, timezone, from, count) + ); + } + }); + + it.each([ + ["spring forward (US)", "2024-03-10T05:00:00.000Z", "America/New_York"], + ["fall back (US)", "2024-11-03T04:00:00.000Z", "America/New_York"], + ["spring forward (EU)", "2024-03-31T00:00:00.000Z", "Europe/London"], + ["fall back (EU)", "2024-10-27T00:00:00.000Z", "Europe/London"], + ["southern DST", "2024-04-07T14:00:00.000Z", "Australia/Sydney"], + ])("matches across a DST transition: %s", (_label, iso, timezone) => { + const from = new Date(iso); + + for (const cron of ["0 * * * *", "30 2 * * *", "*/15 * * * *", "0 0 * * *"]) { + expect(nextScheduledTimestamps(cron, timezone, from, 6)).toEqual( + referenceNextScheduledTimestamps(cron, timezone, from, 6) + ); + } + }); + + it("returns strictly increasing times", () => { + const times = nextScheduledTimestamps("*/5 * * * *", "Europe/London", new Date(), 10); + + for (let i = 1; i < times.length; i++) { + expect(times[i].getTime()).toBeGreaterThan(times[i - 1].getTime()); + } + }); +}); + +describe("resolveScheduleTimings", () => { + const now = new Date("2024-06-15T09:17:23.000Z"); + + it("matches a naive per-row resolution", () => { + const inputs = CRONS.map(([cron, timezone], index) => + input({ + cron, + timezone, + deduplicationKey: `dedup-${index}`, + environmentId: `env-${index % 3}`, + windowDurationSeconds: index % 3 === 0 ? 600 : null, + windowPercentage: index % 3 === 1 ? 25 : null, + }) + ); + + expect( + resolveScheduleTimings(inputs, { phaseSecret: PHASE_SECRET, includeLastRun: true, now }) + ).toEqual(referenceResolve(inputs, now, true)); + }); + + it("caching does not change results when rows repeat an expression", () => { + const repeated = Array.from({ length: 20 }, (_, index) => + input({ + cron: "0 9 * * 1-5", + timezone: "Europe/London", + deduplicationKey: `dedup-${index}`, + environmentId: `env-${index % 4}`, + schedulePhase: index % 2 === 0 ? null : index * 1000, + windowPercentage: index % 5 === 0 ? 40 : null, + }) + ); + + expect( + resolveScheduleTimings(repeated, { phaseSecret: PHASE_SECRET, includeLastRun: true, now }) + ).toEqual(referenceResolve(repeated, now, true)); + }); + + it("gives every row in a batch the same nominal time for the same expression", () => { + const rows = Array.from({ length: 50 }, (_, index) => + input({ cron: "*/5 * * * *", deduplicationKey: `dedup-${index}` }) + ); + + const timings = resolveScheduleTimings(rows, { + phaseSecret: PHASE_SECRET, + includeLastRun: false, + now, + }); + + const distinct = new Set(timings.map((timing) => timing.nextRun.getTime())); + expect(distinct.size).toBe(1); + }); + + it("still varies effectiveAt per row inside the window", () => { + const rows = Array.from({ length: 25 }, (_, index) => + input({ + cron: "0 * * * *", + windowPercentage: 100, + deduplicationKey: `dedup-${index}`, + }) + ); + + const timings = resolveScheduleTimings(rows, { + phaseSecret: PHASE_SECRET, + includeLastRun: false, + now, + }); + + const distinct = new Set(timings.map((timing) => timing.nextRunEffectiveAt.getTime())); + expect(distinct.size).toBeGreaterThan(1); + + for (const timing of timings) { + expect(timing.nextRunEffectiveAt.getTime()).toBeGreaterThanOrEqual(timing.nextRun.getTime()); + } + }); + + it("omits lastRun entirely when the caller does not ask for it", () => { + const rows = CRONS.map(([cron, timezone]) => input({ cron, timezone })); + + const timings = resolveScheduleTimings(rows, { + phaseSecret: PHASE_SECRET, + includeLastRun: false, + now, + }); + + expect(timings.every((timing) => timing.lastRun === undefined)).toBe(true); + }); + + it("skips lastRun for inactive schedules", () => { + const [timing] = resolveScheduleTimings([input({ active: false })], { + phaseSecret: PHASE_SECRET, + includeLastRun: true, + now, + }); + + expect(timing.lastRun).toBeUndefined(); + }); + + it("skips lastRun when the previous slot predates the last config change", () => { + const [stale] = resolveScheduleTimings([input({ cron: "0 0 * * *", updatedAt: now })], { + phaseSecret: PHASE_SECRET, + includeLastRun: true, + now, + }); + expect(stale.lastRun).toBeUndefined(); + + const [fresh] = resolveScheduleTimings( + [input({ cron: "0 0 * * *", updatedAt: new Date("2020-01-01T00:00:00.000Z") })], + { phaseSecret: PHASE_SECRET, includeLastRun: true, now } + ); + expect(fresh.lastRun).toEqual(new Date("2024-06-15T00:00:00.000Z")); + }); + + it("degrades to undefined lastRun for a malformed expression rather than throwing", () => { + const rows = [input({ cron: "0 0 * * *" }), input({ cron: "not a cron" })]; + + expect(() => + resolveScheduleTimings([rows[1]], { + phaseSecret: PHASE_SECRET, + includeLastRun: false, + now, + }) + ).toThrow(); + + const [valid] = resolveScheduleTimings([rows[0]], { + phaseSecret: PHASE_SECRET, + includeLastRun: true, + now, + }); + expect(valid.lastRun).toBeDefined(); + }); + + it("honours a caller-supplied schedulePhase over the derived one", () => { + const rows = [input({ schedulePhase: 0, windowPercentage: 100, cron: "0 * * * *" })]; + + const [timing] = resolveScheduleTimings(rows, { + phaseSecret: PHASE_SECRET, + includeLastRun: false, + now, + }); + + expect(timing.nextRunEffectiveAt).toEqual(timing.nextRun); + }); + + it("taking one step for windowless rows matches taking two", () => { + const windowless = CRONS.map(([cron, timezone], index) => + input({ cron, timezone, deduplicationKey: `dedup-${index}` }) + ); + + expect( + resolveScheduleTimings(windowless, { phaseSecret: PHASE_SECRET, includeLastRun: false, now }) + ).toEqual( + referenceResolve(windowless, now, false).map((timing) => ({ ...timing, lastRun: undefined })) + ); + }); + + it.each(CRONS)( + "consecutive occurrences of %s (%s) are never closer than the minimum schedule range", + (cron, timezone) => { + const times = nextScheduledTimestamps(cron, timezone, now, 12); + + for (let i = 1; i < times.length; i++) { + expect(times[i].getTime() - times[i - 1].getTime()).toBeGreaterThanOrEqual( + MINIMUM_SCHEDULE_RANGE_MS + ); + } + } + ); + + it("rejects cron expressions with a seconds field, keeping the minimum interval at one minute", () => { + expect(CronPattern.safeParse("*/30 * * * * *").success).toBe(false); + expect(CronPattern.safeParse("* * * * *").success).toBe(true); + + const everyMinute = nextScheduledTimestamps("* * * * *", "America/New_York", now, 5); + for (let i = 1; i < everyMinute.length; i++) { + expect(everyMinute[i].getTime() - everyMinute[i - 1].getTime()).toBe( + MINIMUM_SCHEDULE_RANGE_MS + ); + } + }); + + it("returns an empty array for no rows", () => { + expect( + resolveScheduleTimings([], { phaseSecret: PHASE_SECRET, includeLastRun: true, now }) + ).toEqual([]); + }); +}); From 388cd122151b91f660e45b17a0401331e8714c36 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Wed, 19 Aug 2026 12:40:01 +0100 Subject: [PATCH 2/2] test(webapp): run schedule timing benchmarks on demand, not in CI Wall-clock speedup ratios are single-sample and swing well past the asserted bounds on a shared runner, so the benchmarks now live behind a *.perf.test.ts suffix with their own config and a test:perf script, following the pattern the e2e suites already use. Correctness stays in the ordinary suite. Also splits the malformed-expression test, whose name claimed a lastRun degradation that its assertions did not cover. --- apps/webapp/package.json | 1 + ...h.test.ts => scheduleTimings.perf.test.ts} | 0 apps/webapp/test/scheduleTimings.test.ts | 27 ++++++++++--------- apps/webapp/vitest.config.ts | 2 +- apps/webapp/vitest.perf.config.ts | 22 +++++++++++++++ 5 files changed, 39 insertions(+), 13 deletions(-) rename apps/webapp/test/{scheduleTimings.bench.test.ts => scheduleTimings.perf.test.ts} (100%) create mode 100644 apps/webapp/vitest.perf.config.ts diff --git a/apps/webapp/package.json b/apps/webapp/package.json index 6fff042a7e7..cabd55d1d7b 100644 --- a/apps/webapp/package.json +++ b/apps/webapp/package.json @@ -24,6 +24,7 @@ "db:seed:webhooks": "tsx seed-webhook-deliveries.ts", "upload:sourcemaps": "bash ./upload-sourcemaps.sh", "test": "vitest --no-file-parallelism", + "test:perf": "vitest --config ./vitest.perf.config.ts --run", "eval:dev": "evalite watch" }, "dependencies": { diff --git a/apps/webapp/test/scheduleTimings.bench.test.ts b/apps/webapp/test/scheduleTimings.perf.test.ts similarity index 100% rename from apps/webapp/test/scheduleTimings.bench.test.ts rename to apps/webapp/test/scheduleTimings.perf.test.ts diff --git a/apps/webapp/test/scheduleTimings.test.ts b/apps/webapp/test/scheduleTimings.test.ts index dc5a0e7c7d1..79ee1cf734c 100644 --- a/apps/webapp/test/scheduleTimings.test.ts +++ b/apps/webapp/test/scheduleTimings.test.ts @@ -266,23 +266,26 @@ describe("resolveScheduleTimings", () => { expect(fresh.lastRun).toEqual(new Date("2024-06-15T00:00:00.000Z")); }); - it("degrades to undefined lastRun for a malformed expression rather than throwing", () => { - const rows = [input({ cron: "0 0 * * *" }), input({ cron: "not a cron" })]; - - expect(() => - resolveScheduleTimings([rows[1]], { - phaseSecret: PHASE_SECRET, - includeLastRun: false, - now, - }) - ).toThrow(); + it("throws for a malformed expression, matching the previous behaviour", () => { + for (const includeLastRun of [false, true]) { + expect(() => + resolveScheduleTimings([input({ cron: "not a cron" })], { + phaseSecret: PHASE_SECRET, + includeLastRun, + now, + }) + ).toThrow(); + } + }); - const [valid] = resolveScheduleTimings([rows[0]], { + it("resolves lastRun for a valid expression", () => { + const [valid] = resolveScheduleTimings([input({ cron: "0 0 * * *" })], { phaseSecret: PHASE_SECRET, includeLastRun: true, now, }); - expect(valid.lastRun).toBeDefined(); + + expect(valid.lastRun).toEqual(new Date("2024-06-15T00:00:00.000Z")); }); it("honours a caller-supplied schedulePhase over the derived one", () => { diff --git a/apps/webapp/vitest.config.ts b/apps/webapp/vitest.config.ts index 80ce4c0a275..995c1742896 100644 --- a/apps/webapp/vitest.config.ts +++ b/apps/webapp/vitest.config.ts @@ -27,7 +27,7 @@ export default defineConfig({ // *.e2e.test.ts: smoke matrix, run via vitest.e2e.config.ts. // *.e2e.full.test.ts: full auth suite, runs via vitest.e2e.full.config.ts // (needs a globalSetup-spawned webapp + Postgres container). - exclude: ["test/**/*.e2e.test.ts", "test/**/*.e2e.full.test.ts"], + exclude: ["test/**/*.e2e.test.ts", "test/**/*.e2e.full.test.ts", "test/**/*.perf.test.ts"], globals: true, pool: "forks", setupFiles: ["./test/setup.ts"], // load apps/webapp/.env diff --git a/apps/webapp/vitest.perf.config.ts b/apps/webapp/vitest.perf.config.ts new file mode 100644 index 00000000000..9d5205cd151 --- /dev/null +++ b/apps/webapp/vitest.perf.config.ts @@ -0,0 +1,22 @@ +import { defineConfig } from "vitest/config"; +import tsconfigPaths from "vite-tsconfig-paths"; + +export default defineConfig({ + test: { + include: ["test/**/*.perf.test.ts"], + globals: true, + pool: "forks", + /** + * These compare wall-clock timings between two implementations. Single + * samples on a shared CI runner swing by more than the ratios being + * asserted, so they are kept out of the default suite and run on demand + * with `pnpm run test:perf`. Correctness is covered by the ordinary + * suites; these exist to show the shape of the win and to catch a + * large regression locally. + */ + fileParallelism: false, + testTimeout: 120_000, + }, + // @ts-ignore + plugins: [tsconfigPaths({ projects: ["./tsconfig.json"] })], +});