perf(webapp): Add BatchTaskRun index to speed up the batch list dashboard page#2499
perf(webapp): Add BatchTaskRun index to speed up the batch list dashboard page#2499matt-aitken merged 1 commit intomainfrom
Conversation
|
WalkthroughAdds a new PostgreSQL migration to create an idempotent concurrent index on BatchTaskRun over columns runtimeEnvironmentId and id (DESC), named BatchTaskRun_runtimeEnvironmentId_id_idx. Updates Prisma schema: removes the existing index on dependentTaskAttemptId and introduces a composite index on [runtimeEnvironmentId, id(sort: Desc)] with a comment indicating it supports the batch list dashboard page. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
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. Comment |
![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
🧹 Nitpick comments (2)
internal-packages/database/prisma/schema.prisma (2)
1581-1583: Pin Prisma index name to the manual SQL index to prevent drift.Add a map so Prisma keeps using
BatchTaskRun_runtimeEnvironmentId_id_idxand never tries to recreate/rename it.- @@index([runtimeEnvironmentId, id(sort: Desc)]) + @@index([runtimeEnvironmentId, id(sort: Desc)], map: "BatchTaskRun_runtimeEnvironmentId_id_idx")
1581-1583: Clarify intent re: dependentTaskAttemptId index.AI summary says it was removed, but it still exists in the model. Keep it if queries rely on it; otherwise consider dropping to save write cost.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal-packages/database/prisma/migrations/20250912155941_add_batch_task_run_dashboard_index/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
internal-packages/database/prisma/migrations/20250912155941_add_batch_task_run_dashboard_index/migration.sql (1)
1-2: LGTM — single CONCURRENTLY index will run non-transactionally.migration file contains only the single CREATE INDEX CONCURRENTLY statement (internal-packages/database/prisma/migrations/20250912155941_add_batch_task_run_dashboard_index/migration.sql); Prisma Migrate does not wrap PostgreSQL migrations in a transaction by default, so a single-statement CONCURRENTLY index is applied outside a transaction. (prisma.io)
Caveat: Prisma has historically wrapped/mishandled multi-statement migrations in some versions (causing CREATE INDEX CONCURRENTLY to fail) — keep this migration single-statement (no extra SET/BEGIN/COMMIT) and ensure CI uses the pinned prisma version in package.json (devDependency prisma: 6.14.0). (github.com)
internal-packages/database/prisma/schema.prisma (1)
1581-1583: Cannot verify — confirm dashboard query uses WHERE runtimeEnvironmentId = ? and ORDER BY id DESC; run EXPLAIN on the exact query.Repository search returned no matches for prisma.batchTaskRun.findMany (the provided ripgrep commands produced no output), so I couldn't locate the batch list query to confirm ordering/filtering. If the UI orders by createdAt or filters by status, the current index won’t be used; provide the exact query or EXPLAIN output.
2 participants
No description provided.