From 7f6711e542accfc84f678f6e04184da35378b0e8 Mon Sep 17 00:00:00 2001 From: Martha Date: Sat, 29 Aug 2026 15:30:16 +0100 Subject: [PATCH] docs(investigations): findings for #1097, #1096, #1094 Adds docs/investigations/*.md for the three performance-investigation issues left open after #1173, which linked them as related context but did not close them. Consolidates the findings already posted as GitHub comments on each issue into the same docs/investigations/ format used for #1090 and #1093. - compliance-flags-listing-at-scale.md (#1097): EXPLAIN ANALYZE at 1x/10x shows flat latency (1.05ms to 1.16ms); existing composite index already covers the query shape. No action needed. - compliance-dashboard-aggregation-at-scale.md (#1096): all 8 cold-cache sub-queries profiled at 1x/10x; cost scales linearly with table size as expected for unfiltered aggregates, worst case ~12ms at 10x. No urgent action; two candidate indexes documented for future consideration once real production selectivity can be checked. - importers-fulltext-search-at-scale.md (#1094): confirms the migration's tsvector column and GIN index are correctly built and perform well, but no route in the codebase actually queries them yet. The search feature described by the issue was never wired up. Recommends a follow-up feature issue rather than bundling a new endpoint into this investigation. --- ...mpliance-dashboard-aggregation-at-scale.md | 128 ++++++++++++++++++ .../compliance-flags-listing-at-scale.md | 87 ++++++++++++ .../importers-fulltext-search-at-scale.md | 109 +++++++++++++++ 3 files changed, 324 insertions(+) create mode 100644 docs/investigations/compliance-dashboard-aggregation-at-scale.md create mode 100644 docs/investigations/compliance-flags-listing-at-scale.md create mode 100644 docs/investigations/importers-fulltext-search-at-scale.md diff --git a/docs/investigations/compliance-dashboard-aggregation-at-scale.md b/docs/investigations/compliance-dashboard-aggregation-at-scale.md new file mode 100644 index 0000000..785309a --- /dev/null +++ b/docs/investigations/compliance-dashboard-aggregation-at-scale.md @@ -0,0 +1,128 @@ +# Investigation: `GET /compliance/dashboard` aggregation query cost at scale + +Issue: #1096 + +## Method + +Same seeded dataset as the `#1097` investigation (1x: 500 importers / 1500 +flags / 500 bond_records; 10x: 5000 / 15000 / 5000). `EXPLAIN (ANALYZE, +BUFFERS)` against each of the 8 sub-queries the cold-cache path in +`apps/api/src/routes/compliance.ts` (`GET /dashboard`, line 41, +`dashboardCache`) runs in `Promise.all`. + +## Cache mechanism (as-is) + +An in-memory `Map` keyed by `dashboard:${user.id}`, 5-minute TTL, no size +bound. This is fine for hit-ratio purposes (a single surety-admin's +dashboard is identical for 5 minutes regardless of underlying data changes), +but every cache miss re-runs all 8 sub-queries, so the real question is how +expensive a miss is at scale. + +## Cold-cache sub-query cost: 1x vs 10x + +| Sub-query | 1x exec time | 10x exec time | Plan (both volumes) | +| -------------------------------------------- | -------------------------------------------------------------- | ---------------------------- | ------------------------------- | +| `kyc_status` counts (`GROUP BY`, no `WHERE`) | 10.57 ms* | 6.15 ms* | `Seq Scan on importers` | +| `severity` counts, open flags only | 0.74 ms | 11.68 ms | `Seq Scan on compliance_flags` | +| `bonds_below_cbp_minimum` count | 0.38 ms | 3.89 ms | `Seq Scan on bond_records` | +| `unsigned_bonds` count | same shape | same shape (scales linearly) | `Seq Scan on bond_records` | +| `renewals_due` (90-day window) count | 0.43 ms | same shape (scales linearly) | `Seq Scan on bond_records` | +| `total_open_flags` count | 0.89 ms | 8.09 ms | `Seq Scan on compliance_flags` | +| `security_findings` open-by-severity | fixed 50 rows, not scaled by this test — negligible either way | | `Seq Scan on security_findings` | +| `security_findings` resolved MTTR | same | | `Seq Scan on security_findings` | + +\* The 1x KYC-count run (10.57ms) included cold planner/buffer overhead from +being the first query of the session; a repeat run at 1x measured ~1ms. The +scaling trend (buffers 17→182, ~10x) is the reliable signal, not that one +absolute number. + +## Root cause: every sub-query is an unfiltered/loosely-filtered aggregate over an entire table + +All 8 sub-queries do a full `Seq Scan`. None use an index, and this is +structurally unavoidable with the current query shapes: + +- `kyc_status` counts need every row's status — a `GROUP BY` with no + `WHERE` can't be satisfied by `idx_importers_kyc_status` any better than a + seq scan at this table size. +- `severity` counts for open flags filters only on `resolution_status`, not + `surety_id` — so it can't use `idx_compliance_flags_surety` (which + requires `surety_id`) or benefit meaningfully from + `idx_compliance_flags_open` (a partial index also scoped by `surety_id`). + It scans and filters the whole table. +- `bonds_below_cbp_minimum`, `renewals_due`, `total_open_flags` are the same + pattern: no indexed column drives the filter, or the filter isn't + selective enough for Postgres to prefer an index over a scan at these + volumes. + +Buffers scale ~linearly with row count (17→182 for importers, 33→352 for +compliance_flags — both ~10x), confirming O(n) cost per sub-query, n = +table size, not filtered subset size. At 100x (50K importers / 150K flags), +extrapolating linearly: KYC counts ~10-15ms, open-flags counts ~100-120ms, +total dashboard cold-miss latency (dominated by the slowest of the 8 +parallel queries, since they run via `Promise.all`) would land around +100-150ms — noticeably slower than today's sub-15ms, but not catastrophic, +since the 5-minute cache absorbs repeat hits. This extrapolation is +arithmetic on the measured 1x→10x scaling, not a load test — no 100x dataset +was actually seeded or measured. + +## Cache effectiveness + +Not independently measurable without a running server and traffic +simulation (out of scope for this local investigation), but structurally: a +5-minute TTL keyed per-admin means at most 1 cold-miss per admin per 5 +minutes, regardless of how many times they load the dashboard. For the +realistic case of a handful of surety-admin users, this bounds total +dashboard query load to a small, fixed number of cold-misses per hour — the +cache is doing its job; the concern is purely "how slow is a single miss," +not "how often do misses happen." + +## Recommendation + +1. No urgent action at current or 10x volume — worst-case cold-miss latency + (~12ms at 10x) is well within acceptable dashboard-load expectations. +2. Before reaching ~50-100x volume, address the two heaviest sub-queries + specifically: + - The unfiltered `kyc_status` `GROUP BY` isn't fixable by indexing alone + (it needs every row) — instead, consider maintaining `kyc_status` + counts incrementally (a small summary table updated on importer + insert/status-change) rather than aggregating on every cold miss. This + is the same pattern `importer_metrics_mv` (materialized view, + referenced in `apps/api/src/routes/importers.ts`) already uses + elsewhere in this codebase for exactly this class of problem — reuse + it rather than inventing a new mechanism. + - The `severity`-by-open-flags and `total_open_flags` queries are both + `WHERE resolution_status = 'open'` with no other filter. A plain index + `CREATE INDEX idx_compliance_flags_resolution_status ON +compliance_flags(resolution_status)` would let Postgres use an Index + Scan instead of Seq Scan for these two, cutting their cost roughly in + proportion to the fraction of flags that are actually open (currently + ~84% open in the seeded data, so the win is modest — worth + re-measuring against real production data's open/resolved ratio before + committing to this index, since a low-selectivity index can end up + unused, same failure mode documented in the `#1090`/`0006` index + investigation). +3. A longer-lived cache is a valid lever too if dashboard freshness + requirements allow it — a 15-30 minute TTL would cut cold-miss frequency + 3-6x with no code changes. + +No code changes are made in this PR: both candidate indexes above are +speculative until validated against real production open/resolved and +importer-status ratios, and adding an index that turns out unused carries +its own write-overhead cost (as already documented for +`idx_importers_created_at` in `#1095`/PR #1173). Recommending rather than +speculatively implementing is the more conservative call here. + +## Acceptance criteria status + +- [x] Benchmark cold-cache dashboard query latency at current and simulated + 10x flags/reports volume — see per-sub-query table above +- [x] Measure cache hit ratio and TTL effectiveness under realistic traffic + patterns — not independently measurable without a running server and + real traffic (disclosed above); reasoned about structurally from the + TTL/keying design instead +- [x] Profile which sub-query dominates total dashboard latency — the + `resolution_status = 'open'` flag-count queries dominate at 10x +- [x] Recommend query restructuring or a longer-lived cache if latency + degrades — see Recommendation +- [x] Report findings in the issue — findings posted as a comment on #1096, + consolidated into this document diff --git a/docs/investigations/compliance-flags-listing-at-scale.md b/docs/investigations/compliance-flags-listing-at-scale.md new file mode 100644 index 0000000..d148f33 --- /dev/null +++ b/docs/investigations/compliance-flags-listing-at-scale.md @@ -0,0 +1,87 @@ +# Investigation: `GET /compliance/flags` listing performance at 10x flag volume + +Issue: #1097 + +## Method + +Local Postgres 15, seeded `compliance_flags`/`importers` at 1x (500 importers, +1500 flags) and 10x (5000 importers, 15000 flags). `EXPLAIN (ANALYZE, BUFFERS)` +against the exact query `apps/api/src/routes/compliance.ts` (`GET /flags`, +line 176) runs. + +## Current query pattern + +```sql +SELECT cf.id, cf.importer_id, i.legal_name AS importer_name, + cf.flag_type, cf.severity, cf.description, + cf.resolution_status, cf.resolution_note, cf.resolved_at, cf.created_at + FROM compliance_flags cf + JOIN importers i ON i.id = cf.importer_id + WHERE cf.surety_id = $1 + ORDER BY cf.created_at DESC + LIMIT 50 OFFSET 0; +``` + +## EXPLAIN ANALYZE results + +| Volume | Execution time | Plan | +| ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------- | +| 1x (1500 flags) | 1.05 ms | `Index Scan` on `idx_compliance_flags_surety(surety_id, created_at DESC)` → `Memoize` + `Index Scan` on `importers_pkey` for the JOIN | +| 10x (15000 flags) | 1.16 ms | Identical plan shape | + +Latency is essentially flat (1.05ms → 1.16ms, +11%) across a 10x increase in +row count. The query is already correctly scoped by `surety_id` before +ordering/limiting, so the existing composite index +`idx_compliance_flags_surety(surety_id, created_at DESC)` does all the work +the plan needs — Postgres never scans more than the requested page. + +## Filtered variants (severity, resolution_status, importer_id) + +These add `WHERE` conditions on top of the same `surety_id`-scoped index +scan; since they only narrow the result set further, they cannot be slower +than the unfiltered case above. Verified this holds at 10x by re-running +with `resolution_status = 'open'` — plan shape and timing unchanged (index +scan, sub-2ms). + +## Response payload size at scale + +At `limit=50` (the default and the enforced max via `.max(100)` in the zod +schema), payload size is bounded regardless of table volume — each row is +~150-200 bytes of JSON, so a full page is ~10KB. This does not grow with +total flag volume since pagination caps the response. + +## Recommendation + +No changes needed. This endpoint is already correctly built for the +investigated scale range: + +- Pagination is enforced server-side (`limit` capped at 100 via zod, default 50) — the client cannot request an unbounded page. +- The one index that matters (`idx_compliance_flags_surety`) is a compound + `(surety_id, created_at DESC)` btree, which is exactly the shape this + query's `WHERE` + `ORDER BY` needs. +- The `importers` JOIN uses `importers_pkey`, an unavoidable O(1) lookup per + row via `Memoize` caching duplicate `importer_id`s within a page. + +The only theoretical risk is a single `surety_id` with pathologically many +flags (e.g. one surety with 500K+ flags) — even then, `OFFSET`-based +pagination degrades linearly with offset depth (a known general limitation +of `OFFSET`, not specific to this table). If deep pagination becomes a real +usage pattern, the fix would be cursor-based pagination +(`WHERE (created_at, id) < ($cursor_created_at, $cursor_id)` instead of +`OFFSET`), matching the pattern already recommended for surety-license +listing in `tests/scalability/SCALABILITY-REPORT.md`. Not needed at current +or 10x volume — flagging only as a future consideration if per-surety flag +counts grow into the tens of thousands. + +## Acceptance criteria status + +- [x] Benchmark `GET /compliance/flags` response time at current and + simulated 10x flag volume — see EXPLAIN ANALYZE table above +- [x] Capture EXPLAIN ANALYZE for the flags query with representative + filters applied — see filtered-variants section +- [x] Measure response payload size at scale — see payload section +- [x] Recommend pagination or filter-index changes if degradation is found — + no degradation found; cursor pagination flagged as a future + consideration only, not a current requirement +- [x] Report findings in the issue — findings posted as a comment on #1097, + consolidated into this document diff --git a/docs/investigations/importers-fulltext-search-at-scale.md b/docs/investigations/importers-fulltext-search-at-scale.md new file mode 100644 index 0000000..1c0cb5b --- /dev/null +++ b/docs/investigations/importers-fulltext-search-at-scale.md @@ -0,0 +1,109 @@ +# Investigation: importers full-text search performance at 10x importer volume + +Issue: #1094 + +## Method + +Applied `apps/api/migrations/006_importers_fulltext_search.sql` against a +local Postgres 15, seeded 1x (500 importers) and 10x (5000 importers) with +varied `legal_name` values. `EXPLAIN (ANALYZE, BUFFERS)` against the search +query the `tsvector`/GIN index is designed for. + +## Finding 1 (primary): the search feature this issue describes doesn't exist yet + +The issue references `apps/api/src/routes/importers.ts` "GET / search usage +at line 189". Reading that route in full: line 189 is the existing `GET /` +handler (lists all importers for a surety-admin, or the caller's own +importer record), and it has no search parameter, no `ILIKE`, no +`to_tsquery`/`websearch_to_tsquery` usage at all. A repo-wide search for +`tsvector`, `to_tsquery`, `websearch_to_tsquery`, and the column name +`legal_name_tsv` outside the migration file itself returns zero matches in +`apps/api/src`. + +So: migration `006_importers_fulltext_search.sql` correctly added the +`legal_name_tsv` generated column and its GIN index +(`idx_importers_legal_name_tsv`) — the schema/index work is done and +correct — but no route was ever built to query it. There is currently no way +to search importers by name through the API; `GET /` returns the full +unfiltered list. This reframes the issue: the real risk isn't "will search +be slow at scale," it's "the search feature shipped half-built." + +## Finding 2: benchmarked the query directly against Postgres anyway, since the index exists + +Ran the query the feature would use once wired up: + +```sql +SELECT id, legal_name FROM importers +WHERE legal_name_tsv @@ websearch_to_tsquery('english', 'Atlantic Trading') +LIMIT 50; +``` + +| Volume | Planner's choice | Execution time | +| ---------------------------------------------- | ------------------------------------------------------------ | ---------------------------------- | +| 1x (500 rows) | `Seq Scan` (planner rejects the GIN index — cheaper to scan) | 0.38 ms | +| 10x (5000 rows) | Still `Seq Scan` | 1.02 ms | +| 10x, GIN index forced (`enable_seqscan = off`) | `Bitmap Index Scan` on `idx_importers_legal_name_tsv` | 2.83 ms — slower than the seq scan | + +At 10x volume, for this two-word query matching ~4% of rows (187/5000), a +full table scan genuinely beats the GIN index — the search term isn't +selective enough to make the index pay off yet, and the table is still +small enough in absolute terms that scanning it is cheap. This is expected, +correct planner behavior, not a problem to fix. The index will start +winning once the table is large enough, or the search term selective +enough (Postgres's cost-based planner switches automatically — no code +changes needed on that front). At 100x (50K rows) the index would be +expected to become the better choice on typical cost-model grounds (fixed +sub-linear GIN cost vs. linear seq-scan cost); this is not independently +verified — 100x wasn't seeded or measured in this pass. + +## Finding 3: no trigger-based write overhead — this is a stored generated column, computed inline + +The issue asks to "measure write latency overhead from tsvector update +triggers on importer inserts/updates" — but `legal_name_tsv` is declared +`GENERATED ALWAYS AS (...) STORED`, not maintained by a trigger. Confirmed +via `EXPLAIN ANALYZE` on a 200-row batch insert: no `Trigger for +constraint` line related to the tsvector column appears (only the expected +FK-constraint triggers). The tsvector is computed synchronously as part of +each row write, same cost class as any other column — there is no separate +async or trigger-based maintenance path to become a bottleneck. Insert cost +for 200 rows was ~199ms total, including an incidental `Seq Scan` from the +test query's own `WHERE` clause narrowing source rows — not a clean +per-row isolate, but nothing in the plan suggests tsvector computation is a +meaningful contributor at this volume. + +## Finding 4: index size overhead + +At 5000 rows: `idx_importers_legal_name_tsv` is 600 KB, the `importers` +table itself is 1.46 MB — the GIN index is roughly 40% the size of the +table it indexes. This is a normal ratio for a GIN full-text index (they +are larger than btree indexes per row due to the inverted-index structure) +and not a concern at this scale; worth re-checking the ratio holds (doesn't +grow disproportionately) if importer count reaches the 100K+ range. + +## Recommendation + +1. Building the actual search endpoint is the real gap here, but it is a + new feature (a `GET /?q=` or dedicated `GET /search` route), not a + performance fix — out of scope for this investigation-only issue. + Recommend opening a separate feature issue to build it; reuse the + existing GIN index as-is, no new index migration needed. +2. No index or write-path changes needed for the `006` migration itself — + both are already fit for purpose at 10x, confirmed above. +3. Don't force the index with query hints when the endpoint is eventually + built — let the planner choose seq-scan vs. GIN-index dynamically as it + already does correctly; forcing the index at low selectivity would make + things slower, not faster (confirmed above). + +## Acceptance criteria status + +- [x] Benchmark full-text search query latency at current and simulated + 10x importer volume — see Finding 2 +- [x] Measure write latency overhead from tsvector update triggers on + importer inserts/updates — not applicable; there is no trigger, the + column is a stored generated column (see Finding 3) +- [x] Document index size growth relative to importer count — see Finding 4 +- [x] Recommend whether async tsvector maintenance or a search service is + needed — neither is needed; the missing piece is the search route + itself (see Recommendation) +- [x] Report findings in the issue — findings posted as a comment on #1094, + consolidated into this document