Skip to content

Commit aca234d

Browse files
authored
perf(webapp): bound checkSchedule environment load to the requested ids (#4598)
## What `CheckScheduleService.call` loaded **every** environment of a project (`{ id, type, archivedAt }`, no filter) and then immediately narrowed to just the requested `environmentIds` via `resolveProjectScopedEnvironments`. It only ever uses the requested envs (to reject foreign env ids and reject archived branches). On a preview-heavy project that meant loading hundreds of archived branch rows to validate one, on a path called in a per-scheduled-task loop on the deploy path (`createBackgroundWorker` -> `syncDeclarativeSchedules`) and from `upsertTaskSchedule`. The query is index-backed and individually fast (rows_read/returned = 1 per predicate), so this is about result-set width / egress and wasted work at scale (~580k calls/24h observed via Insights), not a slow plan. ## Change Bound the `environments` relation load to `boundedIn(environmentIds)`: ```ts environments: { where: { id: { in: boundedIn(environmentIds) } }, select: { id: true, type: true, archivedAt: true }, } ``` Returns `<=` the number of requested envs (usually 1) instead of the whole project. Both existing behaviors are preserved: - **Foreign-id rejection**: the relation is still scoped to the project, so a requested id belonging to another project never comes back and `resolveProjectScopedEnvironments` reports it as `foreign` (a missing requested id is already treated as foreign). - **Archived-branch rejection**: a requested id that is an archived branch still comes back with `archivedAt` set, so the downstream `Can't add or edit a schedule for an archived branch` check still fires. `archivedAt` is kept in the select deliberately, so this bounds by id rather than filtering archived rows out. ## Evidence (isolated stack, seeded 1 prod env + 40 archived branch envs) Local `EXPLAIN (ANALYZE)` of the exact environments sub-select: | | rows returned | buffers | |---|---|---| | before (unbounded) | **41** | shared hit=12 | | after (`id IN (requested)`) | **1** (`Rows Removed by Filter: 40`) | shared hit=4 | Same `RuntimeEnvironment_projectId_idx`, no plan change. Rows to the client drop to `len(environmentIds)`, which is the point. **Unit (vitest, testcontainers, real Postgres):** `apps/webapp/test/checkSchedule.test.ts` extended to prove, on real rows, that the bounded load returns only the requested env (1 of 10), still reports a foreign id as foreign, and still surfaces an archived branch when it is the requested one. 5/5 pass. **Full e2e (both execution modes, real stack):** a purpose-built project with two declarative `schedules.task`s. - `trigger dev`: dev worker created, both schedules synced through the edited `checkSchedule` loop, no errors. - `trigger deploy` (managed deployment): PRODUCTION worker registered, both schedules synced against the **prod** environment through the same loop, prod + dev schedule instances active, no errors. `typecheck --filter webapp` clean. ## Rollout / rollback Straight deploy, no flag, no migration. Rollback is revert-only (read-path narrowing, no data change). Old and in-flight rows read correctly under both the old and new code. ## Out of scope The two lower-priority sibling reads in the ticket (the Query/metrics env id->slug map and the env-var repository fan-out) are left for follow-ups; they need caching / per-method scoping rather than this single bound.
1 parent c6ef5f3 commit aca234d

3 files changed

Lines changed: 105 additions & 6 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: improvement
4+
---
5+
6+
Validating a schedule when deploying or updating a schedule now does less work on projects with many preview branches, so those operations stay fast as branches accumulate.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { getLimit } from "~/services/platform.v3.server";
66
import { getTimezones } from "~/utils/timezones.server";
77
import { env } from "~/env.server";
88
import type { ScheduleWindow } from "@trigger.dev/core/v3";
9-
import { type PrismaClientOrTransaction } from "@trigger.dev/database";
9+
import { boundedIn, type PrismaClientOrTransaction } from "@trigger.dev/database";
1010
import { validateScheduleWindowSyntax } from "../scheduleWindow.server";
1111

1212
type Schedule = {
@@ -81,6 +81,9 @@ export class CheckScheduleService extends BaseService {
8181
select: {
8282
organizationId: true,
8383
environments: {
84+
where: {
85+
id: { in: boundedIn(environmentIds) },
86+
},
8487
select: {
8588
id: true,
8689
type: true,

apps/webapp/test/checkSchedule.test.ts

Lines changed: 95 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
import { containerTest } from "@internal/testcontainers";
2-
import type { PrismaClient } from "@trigger.dev/database";
2+
import { boundedIn, type PrismaClient } from "@trigger.dev/database";
33
import { describe, expect, vi } from "vitest";
44
import { resolveProjectScopedEnvironments } from "~/v3/services/resolveProjectScopedEnvironments";
55

66
vi.setConfig({ testTimeout: 60_000 });
77

8-
// Exercises the environment-scoping primitive CheckScheduleService relies on
9-
// (`resolveProjectScopedEnvironments`) with real RuntimeEnvironment rows,
10-
// imported directly to avoid `~/db.server` and its eager global-prisma connect.
11-
128
async function seedProjectWithEnv(prisma: PrismaClient, slugBase: string) {
139
const slug = `${slugBase}_${Math.random().toString(36).slice(2, 10)}`;
1410
const organization = await prisma.organization.create({ data: { title: slug, slug } });
@@ -29,10 +25,47 @@ async function seedProjectWithEnv(prisma: PrismaClient, slugBase: string) {
2925
return { organization, project, environment };
3026
}
3127

28+
async function seedBranchEnv(
29+
prisma: PrismaClient,
30+
project: { id: string; organizationId: string },
31+
slugBase: string,
32+
{ archived }: { archived: boolean }
33+
) {
34+
const slug = `${slugBase}_${Math.random().toString(36).slice(2, 10)}`;
35+
return prisma.runtimeEnvironment.create({
36+
data: {
37+
slug: `${slug}-branch`,
38+
type: "PREVIEW",
39+
branchName: slug,
40+
projectId: project.id,
41+
organizationId: project.organizationId,
42+
apiKey: `tr_preview_${slug}`,
43+
pkApiKey: `pk_preview_${slug}`,
44+
shortcode: Math.random().toString(36).slice(2, 10),
45+
archivedAt: archived ? new Date() : null,
46+
},
47+
});
48+
}
49+
3250
function projectEnvironments(prisma: PrismaClient, projectId: string) {
3351
return prisma.runtimeEnvironment.findMany({ where: { projectId }, select: { id: true } });
3452
}
3553

54+
function loadScopedEnvironments(prisma: PrismaClient, projectId: string, environmentIds: string[]) {
55+
return prisma.project
56+
.findFirst({
57+
where: { id: projectId },
58+
select: {
59+
organizationId: true,
60+
environments: {
61+
where: { id: { in: boundedIn(environmentIds) } },
62+
select: { id: true, type: true, archivedAt: true },
63+
},
64+
},
65+
})
66+
.then((project) => project?.environments ?? []);
67+
}
68+
3669
describe("resolveProjectScopedEnvironments (schedule env scoping)", () => {
3770
containerTest("rejects an environment id that belongs to another project", async ({ prisma }) => {
3871
const a = await seedProjectWithEnv(prisma, "orga");
@@ -58,3 +91,60 @@ describe("resolveProjectScopedEnvironments (schedule env scoping)", () => {
5891
expect(result.kind).toBe("ok");
5992
});
6093
});
94+
95+
describe("CheckScheduleService bounded environments load", () => {
96+
containerTest(
97+
"loads only the requested environments, not every project environment",
98+
async ({ prisma }) => {
99+
const a = await seedProjectWithEnv(prisma, "orga");
100+
for (let i = 0; i < 8; i++) {
101+
await seedBranchEnv(prisma, a.project, `branch${i}`, { archived: true });
102+
}
103+
await seedBranchEnv(prisma, a.project, "active", { archived: false });
104+
105+
const all = await projectEnvironments(prisma, a.project.id);
106+
expect(all.length).toBe(10);
107+
108+
const scoped = await loadScopedEnvironments(prisma, a.project.id, [a.environment.id]);
109+
expect(scoped.length).toBe(1);
110+
expect(scoped[0]?.id).toBe(a.environment.id);
111+
112+
const result = resolveProjectScopedEnvironments([a.environment.id], scoped);
113+
expect(result.kind).toBe("ok");
114+
}
115+
);
116+
117+
containerTest(
118+
"still rejects a foreign environment id when the load is bounded",
119+
async ({ prisma }) => {
120+
const a = await seedProjectWithEnv(prisma, "orga");
121+
const b = await seedProjectWithEnv(prisma, "orgb");
122+
await seedBranchEnv(prisma, a.project, "branch", { archived: true });
123+
124+
const scoped = await loadScopedEnvironments(prisma, a.project.id, [
125+
a.environment.id,
126+
b.environment.id,
127+
]);
128+
expect(scoped.length).toBe(1);
129+
130+
const result = resolveProjectScopedEnvironments([a.environment.id, b.environment.id], scoped);
131+
expect(result.kind).toBe("foreign");
132+
expect(result).toMatchObject({ foreignEnvironmentId: b.environment.id });
133+
}
134+
);
135+
136+
containerTest(
137+
"still surfaces an archived branch env when it is the requested one",
138+
async ({ prisma }) => {
139+
const a = await seedProjectWithEnv(prisma, "orga");
140+
const archivedBranch = await seedBranchEnv(prisma, a.project, "branch", { archived: true });
141+
142+
const scoped = await loadScopedEnvironments(prisma, a.project.id, [archivedBranch.id]);
143+
expect(scoped.length).toBe(1);
144+
145+
const result = resolveProjectScopedEnvironments([archivedBranch.id], scoped);
146+
expect(result.kind).toBe("ok");
147+
expect(result.kind === "ok" && result.environments.some((env) => env.archivedAt)).toBe(true);
148+
}
149+
);
150+
});

0 commit comments

Comments
 (0)