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