fix(db): index two FK columns on pki_signer_certificate_issuance_jobs - #7357
fix(db): index two FK columns on pki_signer_certificate_issuance_jobs#7357jaydeep-pipaliya wants to merge 2 commits into
Conversation
The signer-revamp migration (20260528124443) created the issuance-jobs table with three FK columns but only indexed signerId. caId (CASCADE to certificate_authorities) and certificateId (SET NULL to certificates) have no covering index, so a delete on either parent seq-scans this table per row via the RI trigger. Adds a full index on caId (notNullable, every row participates) and a partial WHERE certificateId IS NOT NULL index on certificateId (only set after issuance completes, so most in-flight rows are NULL). Both built CONCURRENTLY so the deploy doesn't take a write-blocking lock, mirroring the pattern from 20260715060344.
📝 Contributor License Agreement requiredBefore this PR can merge, every contributor must sign the Infisical CLA. Still needs to sign: Once everyone has signed, the check updates automatically — no need to close and reopen the PR. |
|
| Filename | Overview |
|---|---|
| backend/src/db/migrations/20260721093822_add-fk-indexes-pki-signer-issuance-jobs.ts | Adds two concurrent FK indexes, but an interrupted build can leave an invalid index that later runs skip. |
Reviews (1): Last reviewed commit: "fix(db): index two FK columns on pki_sig..." | Re-trigger Greptile
| if ((await knex.schema.hasTable(idx.table)) && (await knex.schema.hasColumn(idx.table, idx.column))) { | ||
| const predicate = idx.partial ? `WHERE "${idx.column}" IS NOT NULL` : ""; | ||
| await knex.raw(` | ||
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "${idx.name}" |
There was a problem hiding this comment.
If CREATE INDEX CONCURRENTLY is interrupted by the configured timeout, a deployment cancellation, or a lost connection, PostgreSQL can leave an invalid index with this name. A rerun then skips the build because of IF NOT EXISTS, so the migration completes without a usable FK index and parent deletions continue scanning the issuance-job table. The retry path should detect and replace an index whose indisvalid value is false.
There was a problem hiding this comment.
Good catch — pushed 9f1c443. Added a dropIfInvalid helper that queries pg_index.indisvalid for each target index name and issues DROP INDEX CONCURRENTLY IF EXISTS before the CREATE INDEX CONCURRENTLY IF NOT EXISTS on rerun. Also pushed the same fix to #7294 (aa671ab) since it has the identical pattern and would have the same failure mode.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70591ffa06
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if ((await knex.schema.hasTable(idx.table)) && (await knex.schema.hasColumn(idx.table, idx.column))) { | ||
| const predicate = idx.partial ? `WHERE "${idx.column}" IS NOT NULL` : ""; | ||
| await knex.raw(` | ||
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "${idx.name}" |
There was a problem hiding this comment.
When either concurrent build is canceled by this migration's lock/statement timeout, PostgreSQL can leave an INVALID index with the same name; the docs note failed concurrent builds are ignored for querying and should be dropped/retried, while IF NOT EXISTS only checks that a same-name relation exists (https://www.postgresql.org/docs/current/sql-createindex.html). On a retry this line can skip the invalid index and mark the migration applied, so deletes of CAs/certificates keep seq-scanning the child table; check pg_index.indisvalid and drop/rebuild invalid indexes before using IF NOT EXISTS.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — pushed 9f1c443. Added a dropIfInvalid helper that queries pg_index.indisvalid for each target index name and issues DROP INDEX CONCURRENTLY IF EXISTS before the CREATE INDEX CONCURRENTLY IF NOT EXISTS on rerun. Also pushed the same fix to #7294 (aa671ab) since it has the identical pattern and would have the same failure mode.
An interrupted CREATE INDEX CONCURRENTLY (deploy cancel, statement_timeout, lost connection) leaves the index row in pg_class with indisvalid=false. A rerun with IF NOT EXISTS then no-ops, so the migration succeeds without producing a usable index and parent deletions continue to seq-scan. Check pg_index.indisvalid for each target name before the CREATE and drop the invalid remnant first, so the retry actually rebuilds.
Context
The signer-revamp migration (
20260528124443_signer-revamp.ts) createdpki_signer_certificate_issuance_jobswith three FK columns but only indexedsignerId:caId(uuid notNullable) →certificate_authorities(id)ON DELETE CASCADE— no indexcertificateId(uuid nullable) →certificates(id)ON DELETE SET NULL— no indexPostgres does not auto-index FK columns. Deleting a CA or a certificate fires a per-row RI trigger that seq-scans this table for each deleted parent — cheap today, but grows with issuance-job volume.
caIdisnotNullableand every row participates → full index.certificateIdis populated only after the issuance job completes; most in-flight rows areNULL→ partialWHERE certificateId IS NOT NULLindex (same shape used across the codebase for mostly-NULL FK columns perbackend/CLAUDE.md).Same pattern as the fix in #7294 for the pam-revamp FK columns.
Changes
20260721093822_add-fk-indexes-pki-signer-issuance-jobs.tsadds both indexes usingCREATE INDEX CONCURRENTLY IF NOT EXISTS(so the deploy doesn't take a write-blocking lock) withconfig = { transaction: false }. Down path usesDROP INDEX CONCURRENTLY IF EXISTS.Test plan
npm run migration:latest-devfrombackend/npm run migration:rollback-dev— indexes drop cleanlyIF NOT EXISTSon up,IF EXISTSon down)