diff --git a/apps/api/migrations/007_hot_path_indexes.sql b/apps/api/migrations/007_hot_path_indexes.sql index 7403393..065cd25 100644 --- a/apps/api/migrations/007_hot_path_indexes.sql +++ b/apps/api/migrations/007_hot_path_indexes.sql @@ -1,15 +1,13 @@ -- Migration 007: Missing indexes on hot-path queries in routes/importers.ts (issue #257) -- Up -- --- Analysis identified five index gaps causing sequential scans on the most +-- Analysis identified index gaps causing sequential scans on the most -- frequently hit query paths at production data volumes: -- -- Query Missing index -- ───────────────────────────────── ────────────────────────────────────────── -- importers WHERE user_id = $1 importers.user_id -- importers ORDER BY created_at DESC importers.created_at DESC --- contract_events WHERE importer_id contract_events(importer_id, created_at, id) --- + cursor keyset pagination -- bonds WHERE importer_id ORDER BY bonds(importer_id, created_at DESC) -- created_at DESC -- tariff_uploads WHERE importer_id tariff_uploads(importer_id, created_at DESC) @@ -17,6 +15,31 @@ -- -- All indexes use CONCURRENTLY to avoid table-level locks in production. -- See docs/query-analysis.md for full EXPLAIN ANALYZE output and cost table. +-- +-- #1095 correction: this file originally also declared +-- CREATE INDEX CONCURRENTLY idx_contract_events_importer_created_at +-- ON contract_events(importer_id, created_at DESC, id DESC); +-- contract_events has been a partitioned table (PARTITION BY RANGE +-- (created_at)) since migration 0002_partition_contract_events.ts, and +-- PostgreSQL does not support CREATE INDEX CONCURRENTLY directly on a +-- partitioned table — every run of this file failed on that one statement. +-- Because `psql -f` does not abort on a single statement error, this went +-- unnoticed: the other four indexes below were created successfully and +-- the failure was silent unless someone was watching the output. +-- +-- The intended target query, GET /:id/events (cursor pagination in +-- routes/importers.ts), orders by `id DESC` alone — it never sorts by +-- created_at. That's already fully covered by +-- idx_contract_events_importer_id_pagination(importer_id, id DESC), which +-- 0002 declared on the contract_events parent (so it auto-propagates to +-- every partition, current and future). The two remaining contract_events +-- queries that do filter by created_at (routes/importers.ts GET +-- /admin/events, routes/regulatory.ts's claims-filed query) have no +-- importer_id predicate, so they're served by the BRIN index +-- idx_contract_events_created_at_brin, also from 0002. +-- So the statement is dropped outright rather than reworked into a +-- per-partition CONCURRENTLY + ATTACH PARTITION sequence — there is no +-- query in this codebase it would actually serve. -- importers.user_id: supports both the "does this user already have an importer?" -- existence check (POST /) and the per-user importer list (GET / non-admin path). @@ -28,14 +51,6 @@ CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_importers_user_id CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_importers_created_at ON importers(created_at DESC); --- contract_events(importer_id, created_at DESC, id DESC): covers both the initial --- event-history fetch and the cursor-keyset continuation clause --- `(created_at, id) < ($2::timestamptz, $3::uuid)` used in GET /:id/events. --- The compound index allows the planner to satisfy ORDER BY created_at DESC, id DESC --- via an Index Scan rather than a sequential scan + sort. -CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_contract_events_importer_created_at - ON contract_events(importer_id, created_at DESC, id DESC); - -- bonds(importer_id, created_at DESC): supports GET /:id/bonds which fetches the -- full bond history for an importer ordered by created_at DESC. CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_bonds_importer_created_at diff --git a/apps/api/src/migrations/0005_scalability_indexes.ts b/apps/api/src/migrations/0005_scalability_indexes.ts index d0d5930..8fbb736 100644 --- a/apps/api/src/migrations/0005_scalability_indexes.ts +++ b/apps/api/src/migrations/0005_scalability_indexes.ts @@ -7,6 +7,10 @@ import type { PoolClient } from 'pg'; +// CREATE INDEX CONCURRENTLY is rejected by Postgres inside any transaction +// block — see runner.ts's MigrationModule.nonTransactional doc comment. +export const nonTransactional = true; + export const up = async (client: PoolClient): Promise => { // 1. Composite index for surety license listing pagination // Supports: ORDER BY created_at DESC, id DESC with cursor-based pagination diff --git a/apps/api/src/migrations/runner.ts b/apps/api/src/migrations/runner.ts index c59813c..b426a44 100644 --- a/apps/api/src/migrations/runner.ts +++ b/apps/api/src/migrations/runner.ts @@ -1,11 +1,28 @@ import fs from 'fs/promises'; import path from 'path'; import { fileURLToPath, pathToFileURL } from 'url'; +import type { PoolClient } from 'pg'; import { pool } from '../db.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); +interface MigrationModule { + up: (client: PoolClient) => Promise; + down: (client: PoolClient) => Promise; + // Set by a migration that runs CREATE/DROP INDEX CONCURRENTLY (or any + // other statement Postgres refuses to run inside a transaction block — + // see PreventInTransactionBlock in Postgres's own source). Such a + // migration is run on its own connection with no surrounding BEGIN/COMMIT, + // so it does not get the same all-or-nothing rollback guarantee the rest + // of the chain gets: if it fails partway through, whatever it already + // created stays behind and must be cleaned up manually before retrying. + // CONCURRENTLY is designed to leave an INVALID index behind on failure + // rather than silently rolling back, precisely so it never takes a table + // lock — so this is Postgres's own tradeoff, not one this runner adds. + nonTransactional?: boolean; +} + export async function runMigrations(action: 'up' | 'rollback' = 'up'): Promise { const client = await pool.connect(); try { @@ -55,27 +72,48 @@ export async function runMigrations(action: 'up' | 'rollback' = 'up'): Promise