docs: performance investigations for importers pagination, oracle feed contention, metrics refresh, and GIN index

apps/api/src/migrations/0006_importers_and_events_perf_indexes.ts

@@ -0,0 +1,48 @@
+import { PoolClient } from 'pg';
+
+// #1090 — index that a prior investigation (#257) intended to add but which
+// never actually reached this migration runner.
+//
+// #257's closing PR added `idx_importers_created_at` only to the legacy,
+// no-longer-executed `apps/api/migrations/007_hot_path_indexes.sql`. That
+// file is read by nothing — `npm run migrate` / `db:migrate` only runs
+// `src/migrations/000N_*.ts` via `src/migrations/runner.ts` — and the index
+// doesn't exist in `0001_initial_schema.ts` or any other file here. Verified
+// by grepping every `src/migrations/*.ts` file and `src/db.ts` for the index
+// name: zero hits before this migration. So every database this pipeline has
+// ever actually provisioned has been missing it.
+//
+// idx_importers_created_at supports `GET /importers` for surety_admin
+// (`ORDER BY i.created_at DESC` with no WHERE clause,
+// routes/importers.ts:189-197). Without it this is a sequential scan + sort
+// that grows with the full importers table on every admin list load (#1090).
+// See docs/investigations/importers-list-pagination-at-scale.md.
+//
+// (#1093 also investigated the similarly-unwired `idx_contract_events_raw_gin`
+// from #238/005_contract_events_jsonb_gin.sql, and deliberately does NOT
+// re-add it here — see docs/investigations/contract-events-gin-index-at-scale.md
+// for why: nothing in this codebase ever writes or queries
+// contract_events.raw, so the index would add write overhead with no read
+// benefit.)
+//
+// Not using CONCURRENTLY: `src/migrations/runner.ts` wraps every migration's
+// `up()` in a single `BEGIN`/`COMMIT`, and `CREATE INDEX CONCURRENTLY` cannot
+// run inside a transaction block — Postgres rejects it outright.
+// (`0005_scalability_indexes.ts` already uses CONCURRENTLY and would hit
+// this same error if actually run through this runner — a pre-existing
+// issue in that migration, unrelated to #1090, flagged separately rather
+// than fixed here since changing runner.ts's transaction handling affects
+// every migration, not just this one.) A plain CREATE INDEX briefly locks
+// writers on `importers`, matching the precedent already set by
+// 0002_partition_contract_events.ts's own non-concurrent
+// `CREATE INDEX ... ON contract_events(...)`.
+
+export async function up(client: PoolClient): Promise<void> {
+  await client.query(`
+    CREATE INDEX IF NOT EXISTS idx_importers_created_at ON importers(created_at DESC);
+  `);
+}
+
+export async function down(client: PoolClient): Promise<void> {
+  await client.query(`DROP INDEX IF EXISTS idx_importers_created_at;`);
+}

apps/api/src/routes/importers.ts

@@ -717,8 +717,20 @@ importersRouter.post('/:id/upload-tariff-csv', async (req: Request, res: Respons
     // serving a stale cache entry for up to 30s.
     await invalidateOnChainAccount(importer.id);
 
-    // Refresh the importer_metrics materialized view
-    await refreshImporterMetricsView();
+    // #1091: refresh cost scales with total importer/bond/event volume system-wide
+    // (the view aggregates across ALL importers), not just this one upload, so as
+    // that volume grows this REFRESH gets slower and is more likely to time out or
+    // error under load. Isolated in its own try/catch, matching the alert-evaluation
+    // pattern just above — the tariff upload and on-chain collateral update have
+    // already succeeded by this point, so a slow/failed metrics refresh must not
+    // turn into a 500 for an otherwise-successful request. The periodic refresh job
+    // (see refreshImporterMetricsView's doc comment) remains as a backstop if this
+    // on-demand refresh fails.
+    try {
+      await refreshImporterMetricsView();
+    } catch (err) {
+      console.error('[importers] importer_metrics refresh failed:', err);
+    }
 
     res.json({
       annualDutyTotal,

docs/investigations/contract-events-gin-index-at-scale.md

@@ -0,0 +1,103 @@
+# Investigation: `contract_events` JSONB GIN index at scale
+
+Issue: #1093
+
+## Summary
+
+The issue's premise is that `idx_contract_events_raw_gin` (GIN index on
+`contract_events.raw`, originally `apps/api/migrations/005_contract_events_jsonb_gin.sql`,
+issue #238) "is used by admin event and importer event queries," and asks
+whether its scan performance and size hold up at 10x event volume.
+
+Two things turned up during this investigation that the issue itself
+doesn't anticipate, both more fundamental than the scan-performance
+question it asks:
+
+## 1. The index was never actually applied to any real database
+
+Same situation as #1090's `idx_importers_created_at` (see
+`docs/investigations/importers-list-pagination-at-scale.md`): #238's
+closing work added `idx_contract_events_raw_gin` only to
+`apps/api/migrations/005_contract_events_jsonb_gin.sql`, a file nothing
+executes. `npm run migrate` / `db:migrate` only runs
+`src/migrations/000N_*.ts` via `src/migrations/runner.ts`. Grepped every
+file in `src/migrations/` and `src/db.ts` for `raw_gin`/`USING GIN`/`GIN`:
+zero hits anywhere. So every database this pipeline has ever actually
+provisioned has never had this index — every query touching
+`contract_events.raw`, if any existed, would be doing a sequential scan
+across every partition today.
+
+## 2. Nothing in the codebase writes or reads `contract_events.raw`
+
+This is the more important finding. `raw JSONB` is declared on the table
+(`0001_initial_schema.ts:46`, re-declared identically for the partitioned
+table in `0002_partition_contract_events.ts:69,229`), but:
+
+- **Every `INSERT INTO contract_events` in application code omits `raw`
+  entirely** — checked all four insert call sites:
+  `apps/api/src/queue.ts:163-165`, `apps/api/src/routes/importers.ts:166-169`
+  and `:697-699`, `apps/api/src/routes/admin.ts:525-528`. None of their
+  column lists include `raw`, so every row any of these paths create has
+  `raw IS NULL`. (The only places `raw` appears at all are the
+  copy-through-partition-cutover `INSERT ... SELECT ... raw ...` statements
+  in `0002_partition_contract_events.ts:126-129,237-240`, which move
+  whatever's already in the column — always NULL — rather than populate
+  it.)
+- **No `SELECT` anywhere filters, projects, or otherwise reads `raw`** —
+  checked every route file and service under `apps/api/src`. The two
+  endpoints the issue names as the JSONB-index consumers don't touch it:
+  `GET /admin/events` (`importers.ts:231-292`) selects
+  `id, importer_id, kind, amount, tx_hash, created_at, ledger_sequence,
+event_index` and filters only on `created_at`;
+  `GET /:id/events` (`importers.ts:434-475`) selects
+  `id, kind, amount, tx_hash, created_at` and filters on `importer_id` and
+  a keyset `id` cursor. Neither uses a JSONB containment operator (`@>`,
+  `?`, `->`, `->>`) against `raw`, or against anything.
+
+In other words: `contract_events.raw` is a column that's declared but
+functionally dead in the current codebase — always NULL, never read. A GIN
+index on an always-NULL column indexes nothing useful; it would still pay
+its full write cost (index maintenance on every insert into the busiest
+table in the schema, plus its own storage and VACUUM overhead) for zero
+query benefit, since there's no query for it to accelerate.
+
+## Recommendation
+
+**Do not add `idx_contract_events_raw_gin`.** Doing so — even though it's
+literally what #238/the legacy migration file describe, and even though
+re-adding it might look like "finishing" that prior work — would be a pure
+regression: write overhead on `contract_events` (already flagged as the
+largest, partitioned-for-scale table in the schema, per
+`0002_partition_contract_events.ts`'s own header comment) with no
+corresponding read speedup, because nothing queries the column it would
+index.
+
+If a future feature actually starts writing structured event payloads into
+`raw` and querying them with JSONB operators, add the GIN index at that
+point, in the same change that introduces the read query — not
+speculatively ahead of it. Until then, this issue's AC question ("does GIN
+index scan performance hold up at 10x volume") doesn't apply: there is no
+GIN-indexed scan happening, at any volume, because there's no index and no
+query that would use one.
+
+## Reproducing / measuring
+
+No live Postgres instance was available in this environment to run the
+JSONB containment query benchmarks, index-size, or vacuum/reindex-duration
+measurements this issue's AC asks for. That's moot here regardless: those
+measurements are about a query pattern (JSONB containment against `raw`)
+that doesn't exist anywhere in this codebase to benchmark.
+
+## Acceptance criteria status
+
+- [ ] Benchmark JSONB containment query latency at current volume and
+      simulated 10x volume — not applicable; no code path runs a JSONB
+      containment query against `raw`
+- [ ] Measure GIN index size and vacuum/reindex duration at scale — not
+      applicable; the index doesn't exist in any real database this
+      pipeline provisions, and shouldn't be added (see Recommendation)
+- [ ] Compare planner choice between GIN index scan and sequential scan
+      across volumes — not applicable, same reason
+- [x] Recommend index tuning or partitioning if latency degrades — the
+      recommendation is not to add the index at all; see above
+- [x] Report findings in the issue — this document

docs/investigations/importer-metrics-mv-refresh-cost.md

@@ -0,0 +1,121 @@
+# Investigation: `importer_metrics` materialized view refresh cost at scale
+
+Issue: #1091
+
+## Summary
+
+`/admin/importers/metrics` (`apps/api/src/routes/importers.ts:346-360`) and
+`/importers/:id/metrics` (`importers.ts:367-387`) both read from the
+`importer_metrics` materialized view (defined in
+`apps/api/src/migrations/0001_initial_schema.ts:467-497`, re-created
+identically post-partitioning in
+`0002_partition_contract_events.ts:183-213`). This is a different view from
+the similarly-named `importer_metrics_mv` singleton dashboard-stats view
+(#251) — the two are refreshed by two different functions in `db.ts` and
+this issue is about `importer_metrics` specifically, the per-importer one.
+
+## Is `REFRESH ... CONCURRENTLY` used?
+
+Yes. `refreshImporterMetricsView()` (`apps/api/src/db.ts:953-959`) runs
+`REFRESH MATERIALIZED VIEW CONCURRENTLY importer_metrics`, and the view has
+the required unique index for that
+(`idx_importer_metrics_importer_id`, `0001_initial_schema.ts:496-497`). So
+concurrent _readers_ are never blocked by a refresh — that AC question has
+a definite, code-verified answer.
+
+## What CONCURRENTLY doesn't fix: refresh cost scales with total system volume
+
+The view definition (`0001_initial_schema.ts:467-491`) aggregates across
+**every** importer, with a `LEFT JOIN contract_events ce ON ce.importer_id =
+i.id` and a `SUM(...) FILTER` over that importer's full event history, for
+every row. A `REFRESH` — concurrent or not — has to recompute the entire
+view, i.e. do that full join/aggregate over all importers × all bonds × all
+contract_events, every time it runs.
+
+`refreshImporterMetricsView()` is called from exactly one place:
+`POST /importers/:id/upload-tariff-csv`
+(`importers.ts`, previously line 721, now inside the try/catch at
+~line 730), synchronously awaited on every single tariff-CSV upload from
+any importer. That means:
+
+- The cost of one importer's upload scales with **total** bond/collateral
+  volume across the whole system, not their own data — exactly what this
+  issue's AC asks about ("as underlying bond/collateral row counts grow").
+- At 10x volume, this refresh takes proportionally longer, all of it spent
+  synchronously inside the HTTP request for an action (uploading a tariff
+  CSV) that has nothing to do with the other importers whose data is being
+  re-aggregated.
+- Before this investigation's fix, that `await` had no `try`/`catch`
+  around it: a slow refresh that happened to time out or fail (lock wait,
+  connection pool exhaustion, whatever) would throw past the point where
+  the on-chain collateral update and `tariff_uploads` row had **already
+  succeeded**, turning a successful upload into a client-visible `500`.
+  Every other non-critical side effect in this same handler (friendbot
+  funding in `POST /`, `evaluateTariffAlerts` two lines above) is
+  deliberately wrapped for exactly this reason; the refresh call was the
+  one exception.
+
+## Fix applied
+
+`importers.ts`: wrapped `await refreshImporterMetricsView()` in its own
+`try`/`catch`, matching the adjacent `evaluateTariffAlerts` pattern and its
+documented rationale. This doesn't change the refresh's cost or timing (it
+is still awaited synchronously, so the staleness-window guarantee in
+`refreshImporterMetricsView`'s doc comment — refresh completes before the
+response is sent — is preserved), it only stops a slow/failed refresh from
+turning an otherwise-successful upload into a `500`.
+
+## What wasn't changed, and why
+
+Removing the `await` (fire-and-forget) or dropping the on-demand call
+entirely in favor of only the documented periodic-refresh cadence would
+better address the _latency_ half of this issue (an upload response no
+longer waits on a system-wide aggregate), but both are staleness/product
+tradeoffs already made deliberately, per the doc comment on
+`refreshImporterMetricsView`:
+
+> Near-zero latency for tariff upload mutations since refresh is triggered
+> immediately. Up to 5 minutes latency... for on-chain events if relying
+> on periodic refresh.
+
+Changing that tradeoff isn't a "fix a bug" change, it's a product decision
+about acceptable staleness — out of scope for a minimum fix here, and left
+as the concrete recommendation below instead.
+
+## Recommendation
+
+If refresh latency becomes a measured problem at real 10x volume:
+incremental/partial refresh isn't available for materialized views in
+Postgres (a `REFRESH` always recomputes the full definition), so the
+options are (a) make the on-demand call fire-and-forget instead of awaited,
+accepting a small staleness window on the upload response itself, or (b)
+drop the on-demand refresh and rely solely on the periodic job, accepting
+the documented up-to-5-minute staleness for tariff-upload-triggered changes
+too. Either requires deciding how stale `/importers/:id/metrics` is allowed
+to be immediately after an upload — a product call, not a technical one.
+
+## Reproducing / measuring
+
+No live Postgres instance was available in this environment to benchmark
+actual `REFRESH` duration at simulated 10x bond/collateral volume or to
+observe lock behavior against concurrent readers directly. The
+`CONCURRENTLY`-avoids-reader-blocking conclusion is verified by reading the
+migration and `db.ts` (not by observing it); the refresh-cost-scales-with-
+total-volume conclusion follows from the view's own `LEFT JOIN`/aggregate
+definition, not from a captured timing number.
+
+## Acceptance criteria status
+
+- [ ] Benchmark refresh duration at current volume and at simulated 10x
+      volume — not possible without a live DB in this environment
+- [ ] Measure lock contention against concurrent reads during refresh —
+      same limitation; `CONCURRENTLY` is confirmed in use by code
+      inspection, which by definition avoids exclusive-locking readers
+- [x] Document whether CONCURRENTLY refresh is used and its impact — yes,
+      confirmed (`db.ts:953-959`); it protects concurrent _readers_, not
+      the refresh-triggering request's own latency or error handling
+- [x] Recommend refresh cadence or incremental-refresh alternative if
+      needed — see Recommendation above; incremental refresh isn't
+      available for materialized views in Postgres, so the real choice is
+      fire-and-forget vs. periodic-only, both staleness tradeoffs
+- [x] Report findings in the issue — this document