Skip to content

Commit 3fef4ac

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp): allow schedule replacements at the quota limit
Mono-RevId: 90559be02c928bd675173ec4a5ff2e8cb3319a7e
1 parent b32c1d1 commit 3fef4ac

4 files changed

Lines changed: 486 additions & 154 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Allow deployments to replace declarative schedules at the schedule limit when the resulting set stays within quota.

‎apps/webapp/app/v3/services/checkSchedule.server.ts‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,14 @@ type Schedule = {
1717
};
1818

1919
export class CheckScheduleService extends BaseService {
20-
public async call(projectId: string, schedule: Schedule, environmentIds: string[]) {
20+
public async call(
21+
projectId: string,
22+
schedule: Schedule,
23+
environmentIds: string[],
24+
quotaExclusions?: { environmentId: string; scheduleIds: string[] },
25+
pendingCreations = 0,
26+
quotaLimit?: number
27+
) {
2128
//validate the cron expression
2229
try {
2330
CronPattern.parse(schedule.cron);
@@ -115,15 +122,18 @@ export class CheckScheduleService extends BaseService {
115122

116123
//if creating a schedule, check they're under the limits
117124
if (!schedule.friendlyId) {
118-
const limit = await getLimit(project.organizationId, "schedules", 100_000_000);
125+
const limit =
126+
quotaLimit ?? (await getLimit(project.organizationId, "schedules", 100_000_000));
119127
const schedulesCount = await CheckScheduleService.getUsedSchedulesCount({
120128
prisma: this._prisma,
121129
projectId,
130+
quotaExclusions,
122131
});
123132

124-
if (schedulesCount >= limit) {
133+
const projectedCount = schedulesCount + pendingCreations;
134+
if (projectedCount >= limit) {
125135
throw new ServiceValidationError(
126-
`You have created ${schedulesCount}/${limit} schedules so you'll need to increase your limits or delete some schedules.`
136+
`You have created ${projectedCount}/${limit} schedules so you'll need to increase your limits or delete some schedules.`
127137
);
128138
}
129139
}
@@ -132,14 +142,24 @@ export class CheckScheduleService extends BaseService {
132142
static async getUsedSchedulesCount({
133143
prisma,
134144
projectId,
145+
quotaExclusions,
135146
}: {
136147
prisma: PrismaClientOrTransaction;
137148
projectId: string;
149+
quotaExclusions?: { environmentId: string; scheduleIds: string[] };
138150
}) {
139151
return await prisma.taskScheduleInstance.count({
140152
where: {
141153
projectId,
142154
active: true,
155+
...(quotaExclusions?.scheduleIds.length
156+
? {
157+
NOT: {
158+
environmentId: quotaExclusions.environmentId,
159+
taskScheduleId: { in: boundedIn(quotaExclusions.scheduleIds) },
160+
},
161+
}
162+
: {}),
143163
environment: {
144164
projectId,
145165
type: {

‎apps/webapp/app/v3/services/createBackgroundWorker.server.test.ts‎

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,268 @@ describe("syncDeclarativeSchedules registration", () => {
499499
});
500500
});
501501

502+
describe("syncDeclarativeSchedules quota", () => {
503+
containerTest(
504+
"replaces a schedule at the limit without deleting it before validation",
505+
async ({ prisma, redisOptions }) => {
506+
const { project, prodEnv } = await seedProjectWithEnvs(prisma);
507+
const old = await makeDeclarativeSchedule(prisma, project.id, [prodEnv.id], "old-task");
508+
await seedScheduledTask(prisma, project.id, prodEnv.id);
509+
const engine = createTestScheduleEngine(prisma, redisOptions);
510+
511+
try {
512+
await expect(
513+
syncDeclarativeSchedules(
514+
declarativeTasks({ cron: "not-a-cron", timezone: "UTC" }),
515+
noWorker,
516+
asEnv(prodEnv),
517+
prisma,
518+
engine,
519+
undefined,
520+
1
521+
)
522+
).rejects.toThrow("Invalid cron expression");
523+
expect(await prisma.taskSchedule.findUnique({ where: { id: old.id } })).not.toBeNull();
524+
525+
await syncDeclarativeSchedules(
526+
declarativeTasks({ cron: "0 * * * *", timezone: "UTC" }),
527+
noWorker,
528+
asEnv(prodEnv),
529+
prisma,
530+
engine,
531+
undefined,
532+
1
533+
);
534+
535+
expect(await prisma.taskSchedule.findUnique({ where: { id: old.id } })).toBeNull();
536+
expect(
537+
await prisma.taskSchedule.findFirst({
538+
where: { projectId: project.id, taskIdentifier: "my-task" },
539+
})
540+
).not.toBeNull();
541+
expect(await prisma.taskScheduleInstance.count({ where: { projectId: project.id } })).toBe(
542+
1
543+
);
544+
const created = await prisma.taskSchedule.findFirstOrThrow({
545+
where: { projectId: project.id, taskIdentifier: "my-task" },
546+
include: { instances: true },
547+
});
548+
expect(await engine.getJob(scheduleJobId(created.instances[0].id))).toBeDefined();
549+
} finally {
550+
await engine.quit();
551+
}
552+
}
553+
);
554+
555+
containerTest(
556+
"rejects a later invalid schedule before writing an earlier replacement",
557+
async ({ prisma }) => {
558+
const { project, prodEnv } = await seedProjectWithEnvs(prisma);
559+
const old = await makeDeclarativeSchedule(prisma, project.id, [prodEnv.id], "old-task");
560+
await seedScheduledTask(prisma, project.id, prodEnv.id);
561+
const worker = await prisma.backgroundWorker.findFirstOrThrow({
562+
where: { projectId: project.id },
563+
});
564+
await prisma.backgroundWorkerTask.create({
565+
data: {
566+
friendlyId: `task_other_${prodEnv.id}`,
567+
slug: "other-task",
568+
filePath: "src/trigger/other-task.ts",
569+
workerId: worker.id,
570+
projectId: project.id,
571+
runtimeEnvironmentId: prodEnv.id,
572+
triggerSource: "SCHEDULED",
573+
},
574+
});
575+
const tasks = [
576+
{ id: "my-task", schedule: { cron: "0 * * * *", timezone: "UTC" } },
577+
{ id: "other-task", schedule: { cron: "0 * * * *", timezone: "Not/A_Timezone" } },
578+
] as TasksArg;
579+
580+
await expect(
581+
syncDeclarativeSchedules(tasks, noWorker, asEnv(prodEnv), prisma, undefined, undefined, 1)
582+
).rejects.toThrow("Invalid IANA timezone");
583+
expect(await prisma.taskSchedule.findUnique({ where: { id: old.id } })).not.toBeNull();
584+
expect(await prisma.taskSchedule.count({ where: { projectId: project.id } })).toBe(1);
585+
586+
tasks[1].schedule.timezone = "UTC";
587+
await expect(
588+
syncDeclarativeSchedules(tasks, noWorker, asEnv(prodEnv), prisma, undefined, undefined, 1)
589+
).rejects.toThrow("You have created 1/1 schedules");
590+
expect(await prisma.taskSchedule.count({ where: { projectId: project.id } })).toBe(1);
591+
}
592+
);
593+
594+
containerTest(
595+
"does not count development schedules toward projected quota",
596+
async ({ prisma, redisOptions }) => {
597+
const { project, devEnv } = await seedProjectWithEnvs(prisma);
598+
await seedScheduledTask(prisma, project.id, devEnv.id);
599+
const worker = await prisma.backgroundWorker.findFirstOrThrow({
600+
where: { projectId: project.id },
601+
});
602+
await prisma.backgroundWorkerTask.create({
603+
data: {
604+
friendlyId: `task_other_${devEnv.id}`,
605+
slug: "other-task",
606+
filePath: "src/trigger/other-task.ts",
607+
workerId: worker.id,
608+
projectId: project.id,
609+
runtimeEnvironmentId: devEnv.id,
610+
triggerSource: "SCHEDULED",
611+
},
612+
});
613+
const engine = createTestScheduleEngine(prisma, redisOptions);
614+
try {
615+
await syncDeclarativeSchedules(
616+
[
617+
{ id: "my-task", schedule: { cron: "0 * * * *", timezone: "UTC" } },
618+
{ id: "other-task", schedule: { cron: "0 * * * *", timezone: "UTC" } },
619+
] as TasksArg,
620+
noWorker,
621+
asEnv(devEnv),
622+
prisma,
623+
engine,
624+
undefined,
625+
1
626+
);
627+
expect(await prisma.taskScheduleInstance.count({ where: { projectId: project.id } })).toBe(
628+
2
629+
);
630+
} finally {
631+
await engine.quit();
632+
}
633+
}
634+
);
635+
636+
containerTest(
637+
"serializes concurrent replacements at the limit",
638+
async ({ prisma, redisOptions }) => {
639+
const { project, prodEnv } = await seedProjectWithEnvs(prisma);
640+
await makeDeclarativeSchedule(prisma, project.id, [prodEnv.id], "old-task");
641+
await seedScheduledTask(prisma, project.id, prodEnv.id);
642+
const worker = await prisma.backgroundWorker.findFirstOrThrow({
643+
where: { projectId: project.id },
644+
});
645+
await prisma.backgroundWorkerTask.create({
646+
data: {
647+
friendlyId: `task_other_${prodEnv.id}`,
648+
slug: "other-task",
649+
filePath: "src/trigger/other-task.ts",
650+
workerId: worker.id,
651+
projectId: project.id,
652+
runtimeEnvironmentId: prodEnv.id,
653+
triggerSource: "SCHEDULED",
654+
},
655+
});
656+
const engine = createTestScheduleEngine(prisma, redisOptions);
657+
try {
658+
const results = await Promise.allSettled(
659+
["my-task", "other-task"].map((id) =>
660+
syncDeclarativeSchedules(
661+
[{ id, schedule: { cron: "0 * * * *", timezone: "UTC" } }] as TasksArg,
662+
noWorker,
663+
asEnv(prodEnv),
664+
prisma,
665+
engine,
666+
undefined,
667+
1
668+
)
669+
)
670+
);
671+
expect(results.some((result) => result.status === "fulfilled")).toBe(true);
672+
expect(await prisma.taskScheduleInstance.count({ where: { projectId: project.id } })).toBe(
673+
1
674+
);
675+
expect(
676+
await prisma.taskSchedule.findFirstOrThrow({ where: { projectId: project.id } })
677+
).toMatchObject({
678+
type: "DECLARATIVE",
679+
});
680+
} finally {
681+
await engine.quit();
682+
}
683+
}
684+
);
685+
686+
containerTest(
687+
"concurrent replacements with spare quota leave only the last declaration",
688+
async ({ prisma, redisOptions }) => {
689+
const { project, prodEnv } = await seedProjectWithEnvs(prisma);
690+
await makeDeclarativeSchedule(prisma, project.id, [prodEnv.id], "old-task");
691+
await seedScheduledTask(prisma, project.id, prodEnv.id);
692+
const worker = await prisma.backgroundWorker.findFirstOrThrow({
693+
where: { projectId: project.id },
694+
});
695+
await prisma.backgroundWorkerTask.create({
696+
data: {
697+
friendlyId: `task_other_${prodEnv.id}`,
698+
slug: "other-task",
699+
filePath: "src/trigger/other-task.ts",
700+
workerId: worker.id,
701+
projectId: project.id,
702+
runtimeEnvironmentId: prodEnv.id,
703+
triggerSource: "SCHEDULED",
704+
},
705+
});
706+
const engine = createTestScheduleEngine(prisma, redisOptions);
707+
try {
708+
await Promise.all(
709+
["my-task", "other-task"].map((id) =>
710+
syncDeclarativeSchedules(
711+
[{ id, schedule: { cron: "0 * * * *", timezone: "UTC" } }] as TasksArg,
712+
noWorker,
713+
asEnv(prodEnv),
714+
prisma,
715+
engine,
716+
undefined,
717+
2
718+
)
719+
)
720+
);
721+
const schedules = await prisma.taskSchedule.findMany({
722+
where: { projectId: project.id },
723+
select: { taskIdentifier: true },
724+
});
725+
expect(schedules).toHaveLength(1);
726+
expect(["my-task", "other-task"]).toContain(schedules[0].taskIdentifier);
727+
} finally {
728+
await engine.quit();
729+
}
730+
}
731+
);
732+
733+
containerTest("does not borrow quota from another environment's instance", async ({ prisma }) => {
734+
const { project, prodEnv, devEnv } = await seedProjectWithEnvs(prisma);
735+
await prisma.runtimeEnvironment.update({
736+
where: { id: devEnv.id },
737+
data: { type: "PRODUCTION" },
738+
});
739+
const old = await makeDeclarativeSchedule(
740+
prisma,
741+
project.id,
742+
[prodEnv.id, devEnv.id],
743+
"old-task"
744+
);
745+
await seedScheduledTask(prisma, project.id, prodEnv.id);
746+
747+
await expect(
748+
syncDeclarativeSchedules(
749+
declarativeTasks({ cron: "0 * * * *", timezone: "UTC" }),
750+
noWorker,
751+
asEnv(prodEnv),
752+
prisma,
753+
undefined,
754+
undefined,
755+
1
756+
)
757+
).rejects.toThrow("You have created 1/1 schedules");
758+
759+
expect(await prisma.taskSchedule.findUnique({ where: { id: old.id } })).not.toBeNull();
760+
expect(await prisma.taskScheduleInstance.count({ where: { projectId: project.id } })).toBe(2);
761+
});
762+
});
763+
502764
describe("syncDeclarativeSchedules deletion path", () => {
503765
containerTest(
504766
"does not issue any instance delete when the env owns no instance of the missing schedules",

0 commit comments

Comments
 (0)