Skip to content

Commit 6865d47

Browse files
committed
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.
1 parent b939045 commit 6865d47

6 files changed

Lines changed: 727 additions & 40 deletions

File tree

apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { getTaskIdentifiers } from "~/models/task.server";
55
import { getCurrentPlan, getPlans } from "~/services/platform.v3.server";
66
import { findCurrentWorkerFromEnvironment } from "~/v3/models/workerDeployment.server";
77
import { ServiceValidationError } from "~/v3/services/baseService.server";
8-
import { calculateNextScheduleRunTimes, formatScheduleWindow } from "~/v3/scheduleWindow.server";
8+
import { formatScheduleWindow } from "~/v3/scheduleWindow.server";
99
import { CheckScheduleService } from "~/v3/services/checkSchedule.server";
10-
import { previousScheduledTimestamp } from "~/v3/utils/calculateNextSchedule.server";
10+
import { resolveScheduleTimings } from "~/v3/scheduleTimings.server";
1111
import { env } from "~/env.server";
1212
import { BasePresenter } from "./basePresenter.server";
1313

@@ -16,6 +16,12 @@ type ScheduleListOptions = {
1616
environmentId: string;
1717
userId?: string;
1818
pageSize?: number;
19+
/**
20+
* Walking each cron backwards to approximate "last run" costs an order of
21+
* magnitude more than everything else here, so it is opt-in: only the
22+
* dashboard renders the column. Defaults off.
23+
*/
24+
includeLastRun?: boolean;
1925
} & ScheduleListFilters;
2026

2127
const DEFAULT_PAGE_SIZE = 20;
@@ -54,6 +60,7 @@ export class ScheduleListPresenter extends BasePresenter {
5460
page,
5561
type,
5662
pageSize = DEFAULT_PAGE_SIZE,
63+
includeLastRun = false,
5764
}: ScheduleListOptions) {
5865
const hasFilters =
5966
type !== undefined || tasks !== undefined || (search !== undefined && search !== "");
@@ -274,46 +281,33 @@ export class ScheduleListPresenter extends BasePresenter {
274281
skip: (page - 1) * pageSize,
275282
});
276283

277-
const schedules: ScheduleListItem[] = rawSchedules.map((schedule) => {
278-
// Approximate "last run" from the cron's previous slot. Skip inactive
279-
// schedules — the cron's previous slot reflects what *would* have
280-
// fired, but a deactivated schedule didn't actually fire there. Skip
281-
// when the cron's previous slot predates `updatedAt`: any config
282-
// change (cron edited, timezone changed, deactivate/reactivate)
283-
// bumps updatedAt, and a slot from before the most recent change
284-
// didn't fire under the current configuration. cron-parser throws
285-
// on malformed expressions, so degrade to undefined per-row rather
286-
// than failing the whole list. UI is best-effort; the runs page is
287-
// the source of truth.
288-
let lastRun: Date | undefined;
289-
if (schedule.active) {
290-
try {
291-
const cronPrev = previousScheduledTimestamp(
292-
schedule.generatorExpression,
293-
schedule.timezone
294-
);
295-
lastRun = cronPrev.getTime() > schedule.updatedAt.getTime() ? cronPrev : undefined;
296-
} catch {
297-
lastRun = undefined;
298-
}
299-
}
300-
284+
const instances = rawSchedules.map((schedule) => {
301285
const instance = schedule.instances.find(
302286
(instance) => instance.environmentId === environmentId
303287
);
304288
if (!instance) {
305289
throw new Error(`Schedule instance not found for environment: ${environmentId}`);
306290
}
307-
const [nextRun] = calculateNextScheduleRunTimes({
291+
return instance;
292+
});
293+
294+
const timings = resolveScheduleTimings(
295+
rawSchedules.map((schedule, index) => ({
308296
cron: schedule.generatorExpression,
309297
timezone: schedule.timezone,
310298
deduplicationKey: schedule.deduplicationKey,
311299
environmentId,
312-
schedulePhase: instance.schedulePhase,
313-
phaseSecret: env.ENCRYPTION_KEY,
300+
schedulePhase: instances[index].schedulePhase,
314301
windowDurationSeconds: schedule.windowDurationSeconds,
315302
windowPercentage: schedule.windowPercentage,
316-
});
303+
active: schedule.active,
304+
updatedAt: schedule.updatedAt,
305+
})),
306+
{ phaseSecret: env.ENCRYPTION_KEY, includeLastRun }
307+
);
308+
309+
const schedules: ScheduleListItem[] = rawSchedules.map((schedule, index) => {
310+
const { nextRun, nextRunEffectiveAt, lastRun } = timings[index];
317311

318312
return {
319313
id: schedule.id,
@@ -329,8 +323,8 @@ export class ScheduleListPresenter extends BasePresenter {
329323
active: schedule.active,
330324
externalId: schedule.externalId,
331325
lastRun,
332-
nextRun: nextRun.nominalAt,
333-
nextRunEffectiveAt: nextRun.effectiveAt,
326+
nextRun,
327+
nextRunEffectiveAt,
334328
environments: schedule.instances.map((instance) => {
335329
const environment = project.environments.find((env) => env.id === instance.environmentId);
336330
if (!environment) {

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.tasks.scheduled.$taskParam/route.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
205205
tasks: [task.slug],
206206
page: schedulesPage,
207207
pageSize: 25,
208+
includeLastRun: true,
208209
})
209210
.catch(() => null);
210211

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import {
2+
MINIMUM_SCHEDULE_RANGE_MS,
3+
calculateEffectiveScheduleTime,
4+
calculateSchedulePhase,
5+
} from "@internal/schedule-engine";
6+
import { type NormalizedScheduleWindow } from "@trigger.dev/core/v3";
7+
import {
8+
nextScheduledTimestamps,
9+
previousScheduledTimestamp,
10+
} from "./utils/calculateNextSchedule.server";
11+
12+
/**
13+
* Everything a single row needs to have its run times resolved. Deliberately
14+
* free of Prisma types so this stays testable and benchmarkable on its own.
15+
*/
16+
export type ScheduleTimingInput = {
17+
cron: string;
18+
timezone: string | null;
19+
deduplicationKey: string;
20+
environmentId: string;
21+
schedulePhase: number | null;
22+
windowDurationSeconds: number | null;
23+
windowPercentage: number | null;
24+
active: boolean;
25+
updatedAt: Date;
26+
};
27+
28+
export type ScheduleTiming = {
29+
nextRun: Date;
30+
nextRunEffectiveAt: Date;
31+
/** Only ever set when the caller asked for it AND the schedule is active. */
32+
lastRun: Date | undefined;
33+
};
34+
35+
export type ResolveScheduleTimingsOptions = {
36+
phaseSecret: string;
37+
/**
38+
* Walking the cron backwards to approximate "last run" is by far the most
39+
* expensive thing here, and only the dashboard renders it. Callers that
40+
* don't show the column (the public API) leave this off and skip the walk.
41+
*/
42+
includeLastRun: boolean;
43+
/**
44+
* Fixed reference point for the whole batch. Pinning it once is what makes
45+
* the cron walks cacheable across rows, and it stops rows in one response
46+
* disagreeing about "now".
47+
*/
48+
now?: Date;
49+
};
50+
51+
/**
52+
* Resolves run times for a page of schedules.
53+
*
54+
* The cron walk (`cron-parser`) dominates this path: one step costs tens of
55+
* microseconds for a plain UTC expression and milliseconds for a sparse one in
56+
* a named timezone, because the library walks the calendar unit by unit
57+
* through luxon. At 100 rows that is enough to block the event loop for
58+
* seconds.
59+
*
60+
* Two properties keep it cheap:
61+
*
62+
* 1. Nominal run times depend only on (cron, timezone, now). With `now` pinned
63+
* for the batch, rows sharing an expression share an answer, so cost is
64+
* O(distinct crons) rather than O(rows) — projects tend to run the same
65+
* handful of expressions across many schedules.
66+
* 2. Everything that genuinely varies per row (phase, window, effectiveAt) is
67+
* arithmetic over the cached nominal times, not another walk.
68+
* 3. Windowless schedules take one step instead of two. The second step exists
69+
* only to measure the interval to the following occurrence, and the
70+
* interval reaches `calculateEffectiveScheduleTime`'s result solely through
71+
* `min(intervalMs, max(MINIMUM_SCHEDULE_RANGE_MS, windowMs))`. With no
72+
* window `windowMs` is 0, and `CronPattern` rejects expressions with a
73+
* seconds field, so consecutive occurrences are always at least
74+
* `MINIMUM_SCHEDULE_RANGE_MS` apart and that `min` can never bind. Stepping
75+
* a second time would change nothing, and it is the more expensive of the
76+
* two steps because it walks a whole period rather than the remainder of
77+
* the current one.
78+
*
79+
* Caches live for one call only: every entry is valid solely against this
80+
* batch's `now`.
81+
*/
82+
export function resolveScheduleTimings(
83+
inputs: ScheduleTimingInput[],
84+
{ phaseSecret, includeLastRun, now = new Date() }: ResolveScheduleTimingsOptions
85+
): ScheduleTiming[] {
86+
const nominalCache = new Map<string, Date[]>();
87+
const previousCache = new Map<string, Date | undefined>();
88+
89+
return inputs.map((input) => {
90+
const window: NormalizedScheduleWindow | undefined =
91+
input.windowPercentage !== null
92+
? { type: "percentage", percentage: input.windowPercentage }
93+
: input.windowDurationSeconds !== null
94+
? { type: "duration", durationSeconds: input.windowDurationSeconds }
95+
: undefined;
96+
97+
const steps = window ? 2 : 1;
98+
const key = `${cacheKey(input.cron, input.timezone)}\n${steps}`;
99+
100+
let nominalTimes = nominalCache.get(key);
101+
if (!nominalTimes) {
102+
nominalTimes = nextScheduledTimestamps(input.cron, input.timezone, now, steps);
103+
nominalCache.set(key, nominalTimes);
104+
}
105+
106+
const nominalAt = nominalTimes[0];
107+
const nextNominalAt =
108+
nominalTimes[1] ?? new Date(nominalAt.getTime() + MINIMUM_SCHEDULE_RANGE_MS);
109+
110+
const phase =
111+
input.schedulePhase ??
112+
calculateSchedulePhase({
113+
secret: phaseSecret,
114+
environmentId: input.environmentId,
115+
deduplicationKey: input.deduplicationKey,
116+
});
117+
118+
const { effectiveAt } = calculateEffectiveScheduleTime({
119+
nominalAt,
120+
nextNominalAt,
121+
schedulePhase: phase,
122+
window,
123+
});
124+
125+
return {
126+
nextRun: nominalAt,
127+
nextRunEffectiveAt: effectiveAt,
128+
lastRun: includeLastRun ? resolveLastRun(input, now, previousCache) : undefined,
129+
};
130+
});
131+
}
132+
133+
/**
134+
* Approximates "last run" from the cron's previous slot.
135+
*
136+
* Skips inactive schedules — the previous slot reflects what *would* have
137+
* fired. Skips slots that predate `updatedAt`: any config change (cron edited,
138+
* timezone changed, deactivate/reactivate) bumps `updatedAt`, and a slot from
139+
* before the most recent change didn't fire under the current configuration.
140+
*
141+
* `cron-parser` throws on malformed expressions, so this degrades to undefined
142+
* per row rather than failing the whole list. Best-effort by design; the runs
143+
* page is the source of truth.
144+
*/
145+
function resolveLastRun(
146+
input: ScheduleTimingInput,
147+
now: Date,
148+
cache: Map<string, Date | undefined>
149+
): Date | undefined {
150+
if (!input.active) {
151+
return undefined;
152+
}
153+
154+
const key = cacheKey(input.cron, input.timezone);
155+
156+
let previous: Date | undefined;
157+
if (cache.has(key)) {
158+
previous = cache.get(key);
159+
} else {
160+
try {
161+
previous = previousScheduledTimestamp(input.cron, input.timezone, now);
162+
} catch {
163+
previous = undefined;
164+
}
165+
cache.set(key, previous);
166+
}
167+
168+
if (!previous) {
169+
return undefined;
170+
}
171+
172+
return previous.getTime() > input.updatedAt.getTime() ? previous : undefined;
173+
}
174+
175+
/**
176+
* Newline separator: an IANA timezone name cannot contain one, so no
177+
* (cron, timezone) pair can collide with another by straddling the boundary.
178+
*/
179+
function cacheKey(cron: string, timezone: string | null): string {
180+
return `${timezone ?? ""}\n${cron}`;
181+
}

apps/webapp/app/v3/utils/calculateNextSchedule.server.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,26 @@ export function previousScheduledTimestamp(
3636
.toDate();
3737
}
3838

39+
/**
40+
* Steps one parsed expression `count` times, rather than re-parsing and
41+
* re-walking the calendar from scratch for every step.
42+
*/
3943
export function nextScheduledTimestamps(
4044
cron: string,
4145
timezone: string | null,
4246
lastScheduledTimestamp: Date,
4347
count: number = 1
4448
) {
49+
const interval = parseExpression(cron, {
50+
currentDate: lastScheduledTimestamp,
51+
utc: timezone === null,
52+
tz: timezone ?? undefined,
53+
});
54+
4555
const result: Array<Date> = [];
46-
let nextScheduledTimestamp = lastScheduledTimestamp;
4756

4857
for (let i = 0; i < count; i++) {
49-
nextScheduledTimestamp = calculateNextScheduledTimestamp(
50-
cron,
51-
timezone,
52-
nextScheduledTimestamp
53-
);
54-
55-
result.push(nextScheduledTimestamp);
58+
result.push(interval.next().toDate());
5659
}
5760

5861
return result;

0 commit comments

Comments
 (0)