Repository navigation
perf(api): index the module join tables - #366
cavidelizade wants to merge 3 commits into
Conversation
module_issues, module_members and module_links had no indexes, so every lookup by module_id or issue_id was a sequential scan, while the cycle join tables already have the equivalent indexes. Add them in migration 000014. This covers the index half of Devlaner#346. The N+1 association loading it also mentions is a larger refactor and is left for a follow-up. Refs Devlaner#346 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Walkthrough
Merge Risk:
|
000014 and 000015 are now taken on main (Slack integration, cancelled-state re-fix); a duplicate version stops golang-migrate from loading migrations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔇 Additional comments: 2
apps/api/migrations/000016_module_join_indexes.up.sql-5-5: 1
5-5:Major · Stability & Availability · Heavy lift
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Avoid blocking writes during the startup migration.
If these join tables contain substantial production data, each standard
CREATE INDEXblocks writes to its table until the build finishes. The four builds can therefore interrupt module-association updates during rollout. UseCREATE INDEX CONCURRENTLYin separate migration files: golang-migrate notes that multiple statements in one PostgreSQL execution run in a transaction, where concurrent index creation is not allowed. (postgresql.org)As per coding guidelines, migrations are “Auto-applied at startup.”
Sources: Coding guidelines, Linters/SAST tools
apps/api/migrations/000016_module_join_indexes.down.sql-1-1: 1
1-1:Major · Stability & Availability · Heavy lift
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Keep live rollbacks from blocking table access.
If this migration is rolled back while the application serves traffic, each standard
DROP INDEXtakes anACCESS EXCLUSIVElock on its table. A drop that waits behind another transaction can block subsequent reads and writes. Use separateDROP INDEX CONCURRENTLYmigrations if live rollback is required, or require a maintenance window for this rollback. (postgresql.org)Source: Linters/SAST tools

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1b0d3810-5fc1-499e-9608-de1a61694470
Files selected for processing: 2-
apps/api/migrations/000016_module_join_indexes.down.sqlapps/api/migrations/000016_module_join_indexes.up.sql
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
-
Summary
The module join tables (
module_issues,module_members,module_links) had no indexes, so every lookup bymodule_idorissue_idwas a sequential scan. The cycle join tables already have the equivalent indexes.Linked issues
Refs #346 (the index half)
Type of change
perf:)Surface
apps/api/)apps/api/migrations/)What changed
Migration
000014addsidx_module_issues_module,idx_module_issues_issue,idx_module_members_moduleandidx_module_links_module, usingCREATE INDEX IF NOT EXISTSso it's safe to re-run.Scope note
Issue #346 also flags N+1 association loading in the issue list/detail paths. That's a larger refactor (batching assignees/labels/cycles/modules), so I've left it for a separate PR and kept this one to the indexes.
Migration ordering
I have another open PR (#350) that adds
000013. This one is000014, so it should merge after #350 to keep the sequence clean. Happy to renumber if you'd prefer.Test plan
go build,go vet,go test ./...green; migration applies as plainCREATE INDEX IF NOT EXISTSAI assistance
Claude Code (Opus 4.8), commits carry aCo-Authored-By:trailerSummary by CodeRabbit