fix: Optimize job query with created_at and status indexes #267
Conversation
WalkthroughThe pull request introduces modifications across multiple files in the web service and database layers. The changes primarily focus on refactoring job-related data fetching, counting, and rendering logic. Key updates include replacing job trigger queries with count-based queries, updating index structures in the database, and streamlining data handling in components like Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/JobTable.tsx (2)
76-85: Consider adding aria-label for accessibility.The total count badge should have an aria-label for screen readers.
<Badge variant="outline" + aria-label={`Total jobs: ${total ?? 0}`} className="rounded-full border-neutral-800 text-inherit" >
Line range hint
107-122: Consider adding virtual scrolling for large datasets.The table might benefit from virtual scrolling if the dataset grows large, as rendering 100 rows at once could impact performance.
Consider using a virtualized table library like
react-virtualorreact-windowif performance becomes an issue with large datasets.apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/TableDeployments.tsx (1)
112-118: LGTM! Clean simplification of the null handling logic.The change improves type safety by ensuring
releaseJobTriggersis always an array. Consider using a const assertion for the empty array to make it even more type-safe.- : []; + : [] as const;packages/db/src/schema/job.ts (1)
99-102: LGTM! Well-chosen indexes for query optimization.The addition of indexes on
createdAtandstatuscolumns is a good optimization for:
- Sorting operations using
created_at(commonly used in ORDER BY clauses)- Filtering operations using
status(commonly used in WHERE clauses)Consider monitoring the index usage patterns using PostgreSQL's
pg_stat_user_indexesview to ensure these indexes are being utilized effectively.packages/api/src/router/job.ts (1)
Line range hint
189-241: Consider extracting common query structure.The new count procedure duplicates much of the join and where clause logic from the list procedure. Consider extracting this into a shared base query function to improve maintainability.
+const baseJobQuery = (tx: Tx, workspaceId: string, filter?: JobCondition) => + tx + .from(schema.releaseJobTrigger) + .innerJoin(schema.job, eq(schema.releaseJobTrigger.jobId, schema.job.id)) + .innerJoin(schema.resource, eq(schema.releaseJobTrigger.resourceId, schema.resource.id)) + .innerJoin(schema.release, eq(schema.releaseJobTrigger.releaseId, schema.release.id)) + .innerJoin(schema.deployment, eq(schema.release.deploymentId, schema.deployment.id)) + .innerJoin(schema.environment, eq(schema.releaseJobTrigger.environmentId, schema.environment.id)) + .innerJoin(schema.system, eq(schema.environment.systemId, schema.system.id)) + .where( + and( + eq(schema.system.workspaceId, workspaceId), + isNull(schema.resource.deletedAt), + schema.releaseJobMatchesCondition(tx, filter), + ), + ); count: protectedProcedure .input( z.object({ workspaceId: z.string().uuid(), filter: jobCondition.optional(), }), ) .meta({ authorizationCheck: ({ canUser, input }) => canUser .perform(Permission.JobList) .on({ type: "workspace", id: input.workspaceId }), }) .query(({ ctx, input }) => - ctx.db + baseJobQuery(ctx.db, input.workspaceId, input.filter) .select({ count: countDistinct(schema.releaseJobTrigger.id), }) - .from(schema.releaseJobTrigger) - .innerJoin(...) // Remove duplicated joins - .where(...) // Remove duplicated where clause .then(takeFirst) .then((t) => t.count), ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/JobTable.tsx(5 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/page.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/TableDeployments.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/ReleaseSequencingNode.tsx(1 hunks)packages/api/src/router/job.ts(4 hunks)packages/db/drizzle/0046_little_warstar.sql(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/schema/job.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/page.tsx (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/db/src/schema/job.ts (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/TableDeployments.tsx (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/ReleaseSequencingNode.tsx (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/JobTable.tsx (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/api/src/router/job.ts (1)
Pattern **/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
🔇 Additional comments (7)
packages/db/drizzle/0046_little_warstar.sql (1)
1-2: LGTM! Verify index usage in production queries.
The indexes are well-structured and align with the optimization objective. B-tree is appropriate for both created_at (range queries) and status (equality queries) columns.
Let's verify if these indexes will be used by the queries:
✅ Verification successful
Let me generate additional scripts to find queries that use these indexes:
Based on the search results, I can now generate the final response:
✅ Indexes are well-justified by query patterns
The created indexes will be used by several queries across the codebase:
-
Job status index (
job_status_idx):- Used in policy checks:
eq(schema.job.status, JobStatus.Pending) - Used in job filtering:
notInArray(job.status, [...]) - Used in job querying:
eq(SCHEMA.job.status, JobStatus.Pending) - Used in job cancellation:
eq(schema.job.status, JobStatus.Pending)
- Used in policy checks:
-
Job created_at index (
job_created_at_idx):- Used in ordering:
orderBy(desc(schema.job.createdAt)) - Used in date range queries:
lt(job.createdAt, date),gt(job.createdAt, date) - Used in job analytics:
date_trunc('day', schema.releaseJobTrigger.createdAt)
- Used in ordering:
The B-tree indexes will effectively support these equality, range, and ordering operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find queries that might benefit from these indexes
# Search for job table queries in the codebase
rg -A 5 'FROM.*job.*WHERE.*created_at|status'
# Search for job-related Prisma/Drizzle queries
rg -A 5 'job.*findMany.*where.*createdAt|status'
Length of output: 65878
Script:
#!/bin/bash
# Search for job table queries in SQL files and ORM code
# Look for queries filtering by created_at or status
# Search in SQL files
rg -t sql "job.*where.*created_at|status"
# Search in TypeScript/JavaScript files for Drizzle/Prisma queries
rg -t ts -t js "from\(.*job.*\).*where.*created_at|status|createdAt"
# Search specifically for job table related code
rg -t ts -t js "table\(.*job.*\)|job.*findMany"
Length of output: 71076
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/page.tsx (1)
22-22: LGTM! Query optimization looks good.
Good optimization by:
- Setting limit to 1 since we only need to check existence
- Using length check which is appropriate for the limited result set
Also applies to: 25-25
apps/webservice/src/app/[workspaceSlug]/(app)/(job)/jobs/JobTable.tsx (2)
39-48: LGTM! Efficient implementation of count queries.
Good separation of total counts:
- One for overall total (unfiltered)
- One for filtered total
Both using appropriate refetch intervals and placeholder data for optimal UX.
Line range hint 90-94: LGTM! Good replacement of lodash with native Array.from.
The change from lodash's range to native Array.from improves bundle size and maintainability.
packages/db/drizzle/meta/_journal.json (1)
326-332: LGTM! Migration entry follows the established pattern.
The new migration entry maintains consistency with the existing journal structure and includes all required fields.
packages/api/src/router/job.ts (2)
Line range hint 139-188: LGTM! Query optimization aligns with new index.
The changes effectively utilize the new created_at index by:
- Ordering by
job.createdAtinstead ofreleaseJobTrigger.createdAt - Simplifying the query structure while maintaining functionality
Line range hint 27-38: Verify index usage across all job queries.
The new indexes should improve performance for multiple queries in this file. Let's verify their usage.
✅ Verification successful
Indexes are correctly defined and utilized across job queries
The verification confirms that the job table has appropriate indexes that are being utilized:
-
job_status_idxonstatuscolumn is used in multiple queries:- Policy checks and filters using
job.status - Job queue management
- Release sequencing and dependency checks
- Policy checks and filters using
-
job_created_at_idxoncreatedAtcolumn is used in:- Ordering jobs by creation time in release dependency checks
- Resource-specific job ranking queries
- Job listing and filtering operations
The indexes are properly defined in packages/db/src/schema/job.ts and are actively used across the codebase in various query patterns, which will help optimize query performance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all queries that could benefit from the new indexes
# Find queries using job.status in WHERE clauses
echo "Queries potentially using status index:"
rg -A 2 "job\.status.*WHERE" --type ts
# Find queries using job.createdAt in ORDER BY
echo "Queries potentially using createdAt index:"
rg -A 2 "job\.createdAt.*ORDER" --type ts
Length of output: 268
Script:
#!/bin/bash
# Let's try a broader search for job table usage patterns
# Search for job table queries
echo "Queries using job table:"
rg "from.*job|schema\.job" -B 2 -A 2 --type ts
# Search specifically for WHERE clauses with job table
echo -e "\nWHERE clauses with job table:"
rg "where.*job|schema\.job.*where" -B 2 -A 2 --type ts --ignore-case
# Search for ORDER BY clauses with job table
echo -e "\nORDER BY clauses with job table:"
rg "order by.*job|schema\.job.*order" -B 2 -A 2 --type ts --ignore-case
Length of output: 74246
Script:
#!/bin/bash
# Let's search for job table indexes and their usage
# Find index definitions for job table
echo "Job table index definitions:"
rg "createIndex.*job" --type ts
# Search for specific job status and createdAt usage in queries
echo -e "\nQueries using job status or createdAt:"
rg "job.*status|job.*createdAt|schema\.job\.(status|createdAt)" -B 2 -A 2 --type ts
# Look for ORDER BY clauses specifically with job.createdAt
echo -e "\nORDER BY with job.createdAt:"
rg "ORDER BY.*job\.createdAt|ORDER BY.*schema\.job\.createdAt" -B 2 -A 2 --type ts --ignore-case
Length of output: 49673
| pendingJobsQ.data != null && pendingJobsQ.data.length > 0; | ||
| const isSeparateReleaseInProgress = | ||
| inProgressJobsQ.data != null && inProgressJobsQ.data.total > 0; | ||
| inProgressJobsQ.data != null && inProgressJobsQ.data.length > 0; |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent array length checks found in job and release queries
The codebase shows mixed usage of .total and .length for checking array sizes. This inconsistency should be addressed as part of the job query optimization efforts.
Key locations requiring attention:
useReleaseChannel.ts: Usesdata?.totalDeploymentPageContent.tsx: Usesdata?.totalReleaseSequencingNode.tsx: Contains both patterns - the updated.lengthcheck for jobs and.totalfor releasesJobHistoryChart.tsx: Usesdata?.totalSearchDialog.tsx: Usesdata?.totalAppSidebarPopoverResources.tsx: Uses bothdata?.totalanddata?.items.lengthResourcePageContent.tsx: Usesdata?.total
🔗 Analysis chain
LGTM! Consistent array length checks.
The change from .total to .length aligns with the job query optimization efforts. The null checks are properly maintained.
Let's verify that this pattern is consistently applied across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining uses of .total in job-related queries
rg -A 2 'data\?\.total' --type ts
Length of output: 4174
Summary by CodeRabbit
New Features
Bug Fixes
Chores