Skip to content

Commit 6765f63

Browse files
committed
fix(schedule-engine): seed lastScheduleTime on recovery via cron-prev
Recovery (Redis job missing for an instance) was re-registering with no lastScheduleTime, so the post-recovery first fire fell through the fallback chain to instance.lastScheduledTimestamp — which is frozen at the value last written before this PR stopped writing the column. For schedules that fired heavily after deploy then suffered a Redis loss, that meant payload.lastTimestamp would report a stale pre-deploy timestamp on the first fire post-recovery. Compute lastScheduleTime as the cron expression's previous slot (pure cron math, no DB read — recovery fan-outs must not add load to hot tables). Guarded against the cron-prev predating the instance's createdAt and against cron-parser throwing on malformed expressions. For continuously-running schedules this equals the actual last fire time; for long-paused or recently-edited schedules it's the same approximation the dashboard "Last run" cell accepts. Per Devin and CodeRabbit review on PR #3476.
1 parent 695eab1 commit 6765f63

2 files changed

Lines changed: 57 additions & 4 deletions

File tree

internal-packages/schedule-engine/src/engine/index.ts

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import { Logger } from "@trigger.dev/core/logger";
1111
import { PrismaClient } from "@trigger.dev/database";
1212
import { Worker, type JobHandlerParams } from "@trigger.dev/redis-worker";
1313
import { calculateDistributedExecutionTime } from "./distributedScheduling.js";
14-
import { calculateNextScheduledTimestamp, nextScheduledTimestamps } from "./scheduleCalculation.js";
14+
import {
15+
calculateNextScheduledTimestamp,
16+
nextScheduledTimestamps,
17+
previousScheduledTimestamp,
18+
} from "./scheduleCalculation.js";
1519
import {
1620
RegisterScheduleInstanceParams,
1721
ScheduleEngineOptions,
@@ -699,10 +703,12 @@ export class ScheduleEngine {
699703
select: {
700704
id: true,
701705
generatorExpression: true,
706+
timezone: true,
702707
instances: {
703708
select: {
704709
id: true,
705710
environmentId: true,
711+
createdAt: true,
706712
},
707713
},
708714
},
@@ -777,8 +783,9 @@ export class ScheduleEngine {
777783
instance: {
778784
id: string;
779785
environmentId: string;
786+
createdAt: Date;
780787
};
781-
schedule: { id: string; generatorExpression: string };
788+
schedule: { id: string; generatorExpression: string; timezone: string | null };
782789
}) {
783790
// inspect the schedule worker to see if there is a job for this instance
784791
const job = await this.worker.getJob(`scheduled-task-instance:${instance.id}`);
@@ -793,13 +800,39 @@ export class ScheduleEngine {
793800
return "skipped";
794801
}
795802

803+
// Approximate the previous fire from the cron expression itself rather
804+
// than reading state from the DB. For a continuously-running schedule
805+
// this equals the actual last fire time. For paused-then-resumed
806+
// schedules or recently-edited cron expressions the value will be
807+
// approximate — same trade-off the dashboard "Last run" cell accepts.
808+
// Guarded against the schedule not having existed long enough to have
809+
// fired (cron's previous slot before instance creation), and against
810+
// cron-parser throwing on malformed expressions. Pure cron math, no DB
811+
// read — recovery fan-outs (Redis crash, restart storms) must not add
812+
// load to hot tables.
813+
let lastScheduleTime: Date | undefined;
814+
try {
815+
const cronPrev = previousScheduledTimestamp(
816+
schedule.generatorExpression,
817+
schedule.timezone
818+
);
819+
if (cronPrev.getTime() > instance.createdAt.getTime()) {
820+
lastScheduleTime = cronPrev;
821+
}
822+
} catch {
823+
lastScheduleTime = undefined;
824+
}
825+
796826
this.logger.info("No job found for instance, registering next run", {
797827
instanceId: instance.id,
798828
schedule,
829+
lastScheduleTime: lastScheduleTime?.toISOString(),
799830
});
800831

801-
// If the job does not exist, register the next run
802-
await this.registerNextTaskScheduleInstance({ instanceId: instance.id });
832+
await this.registerNextTaskScheduleInstance({
833+
instanceId: instance.id,
834+
lastScheduleTime,
835+
});
803836

804837
return "recovered";
805838
}

internal-packages/schedule-engine/src/engine/scheduleCalculation.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,26 @@ function calculateNextStep(schedule: string, timezone: string | null, currentDat
2929
.toDate();
3030
}
3131

32+
/**
33+
* Cron's previous slot relative to `fromTimestamp`. For a continuously-
34+
* running schedule this equals the actual last fire time; for paused or
35+
* DST-edge cases it's an approximation. Used only on the recovery path
36+
* where the actual last fire isn't recoverable from in-flight worker state.
37+
*/
38+
export function previousScheduledTimestamp(
39+
cron: string,
40+
timezone: string | null,
41+
fromTimestamp: Date = new Date()
42+
) {
43+
return parseExpression(cron, {
44+
currentDate: fromTimestamp,
45+
utc: timezone === null,
46+
tz: timezone ?? undefined,
47+
})
48+
.prev()
49+
.toDate();
50+
}
51+
3252
export function nextScheduledTimestamps(
3353
cron: string,
3454
timezone: string | null,

0 commit comments

Comments
 (0)