Fix for Schedules list page slow loading #1992
Conversation
Getting BackgroundWorkerTask was very slow (Prisma was getting every single one…)
|
WalkthroughThis set of changes refactors the schedule listing and editing functionality to operate strictly within a single environment context, replacing previous support for multi-environment filtering. The Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant Loader as Loader/Route
participant Presenter as ScheduleListPresenter
participant DB as Database
UI->>Loader: Request schedules for environment
Loader->>Presenter: call({ projectId, environmentId, ... })
Presenter->>DB: Validate environmentId exists
alt Environment not found
Presenter-->>Loader: Throw ServiceValidationError
else Environment found
Presenter->>DB: Fetch latest BackgroundWorker for environmentId
alt No worker found
Presenter-->>Loader: Return empty schedule list
else Worker found
Presenter->>DB: Query schedules/tasks for environmentId and worker
Presenter-->>Loader: Return schedules/tasks
end
end
Loader-->>UI: Render schedules/tasks
sequenceDiagram
participant UI as User Interface
participant Loader as Loader/Route
participant Presenter as EditSchedulePresenter
participant DB as Database
UI->>Loader: Request schedule edit page with environmentSlug
Loader->>Presenter: call({ userId, projectSlug, environmentSlug, friendlyId })
Presenter->>DB: Find environment by slug
alt Environment not found
Presenter-->>Loader: Throw ServiceValidationError (404)
else Environment found
Presenter->>DB: Fetch latest BackgroundWorker for environment
alt No worker found
Presenter-->>Loader: Return empty task list
else Worker found
Presenter->>DB: Query scheduled tasks filtered by worker, project, environment, trigger source
Presenter-->>Loader: Return sorted task slugs
end
end
Loader-->>UI: Render edit schedule page with tasks
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.new/route.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (8)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (1)
243-245:⚠️ Potential issue
pagemay beundefined→ Prisma will receiveNaN.If the caller omits the
pageparameter,(page - 1)evaluates toNaN, which Prisma rejects.- skip: (page - 1) * pageSize, + skip: ((page ?? 1) - 1) * pageSize,Or give
pagea default in the function signature:page = 1.
🧹 Nitpick comments (2)
apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (2)
124-131: Query only the required column to save memory & I/O.You only use
sluglater, butfindManycurrently retrieves full rows. Slim the query:-const possibleTasks = await this._replica.backgroundWorkerTask.findMany({ - where: { - workerId: latestWorker.id, - projectId: project.id, - runtimeEnvironmentId: environmentId, - triggerSource: "SCHEDULED", - }, -}); +const possibleTasks = await this._replica.backgroundWorkerTask.findMany({ + where: { + workerId: latestWorker.id, + projectId: project.id, + runtimeEnvironmentId: environmentId, + triggerSource: "SCHEDULED", + }, + select: { slug: true }, +});This reduces the amount of data shipped from the database.
276-279: Guard against 0 total pages.
Math.ceil(0 / pageSize)returns0, which may break pagination components expecting a minimum of1.- totalPages: Math.ceil(totalCount / pageSize), + totalPages: Math.max(1, Math.ceil(totalCount / pageSize)),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/webapp/app/components/runs/v3/ScheduleFilters.tsx(1 hunks)apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts(6 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules/route.tsx(1 hunks)apps/webapp/app/routes/api.v1.schedules.ts(1 hunks)internal-packages/database/prisma/migrations/20250429100819_background_worker_index_environment_id_and_created_at/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
internal-packages/database/prisma/schema.prisma (1)
1630-1631: Adding index for efficient background worker retrieval by environment.This index optimizes querying for the latest background worker in a given environment by indexing
runtimeEnvironmentIdandcreatedAt(in descending order). This is a good performance improvement that addresses the slow loading issue on the Schedules list page.internal-packages/database/prisma/migrations/20250429100819_background_worker_index_environment_id_and_created_at/migration.sql (1)
1-2: Excellent use of concurrent index creation.Using
CREATE INDEX CONCURRENTLYis the correct approach here as it prevents table locking during index creation, which is critical for production databases. The index will help optimize background worker queries as per the PR objective.apps/webapp/app/components/runs/v3/ScheduleFilters.tsx (1)
43-45: LGTM: Adapted filter destructuring for single environment model.This change correctly removes
environmentsfrom the destructuring pattern, aligning with the refactoring from multi-environment filtering to single environment context.apps/webapp/app/routes/api.v1.schedules.ts (1)
103-103: LGTM: Switched to single environment ID parameter.This update properly converts from the previous
environmentsarray parameter to a singleenvironmentIdparameter, which matches the schema changes and will utilize the new database index for better performance.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules/route.tsx (1)
97-103: Validate thatfilterscannot silently overwriteenvironmentId.
environmentIdis passed first and then the spread operator injects the parsedfilters.
If, in the future,ScheduleListFiltersre-gains a property calledenvironmentId, it would overwrite the explicitly-supplied value and introduce hard-to-trace bugs.Consider making the property name in the filters object more explicit (e.g.
runtimeEnvironmentIdFilter) or adding a TypeScript utility type that prevents such collisions.apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (3)
90-94: 👍 Good defensive check for environment ownership.Adding an explicit validation and surfacing a
ServiceValidationErroravoids leaking data from other environments.
100-121: Early return may hide existing schedules if no worker is found.If an environment has schedules configured but, for whatever reason, the latest worker record has been purged or never created, users will see an empty list even though schedules exist.
Please confirm this is the intended UX, or consider falling back to:
- Querying tasks without the
workerIdfilter, or- Returning schedules but an empty
possibleTasksarray.If needed I can supply a verification script that checks for environments with schedules but without a
BackgroundWorker.
286-289: The returnedfiltersobject dropped thetypefield.Down-stream UI or API consumers that rely on echoing the active filter set will now lose the
typecontext. Re-add it unless you have confirmed nothing reads it.- filters: { - tasks, - search, - }, + filters: { + tasks, + search, + type, + },
The Schedules list page was slow to load because we were getting the possible
BackgroundWorkerTask.This had become very slow because
distinctPrisma wasn't use the Postgres DISTINCT feature – it was loading all of them into memory… this could be millions in some cases if you had a lot of versions.Summary by CodeRabbit