Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/scale-readiness-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ alone is ~3 × 6.75 s of parallel DB CPU.**

### F1 — `match_document_table_facts_text` is slow, but the cause is a generic query plan, not the predicate — CORRECTED 2026-07-08 (CRITICAL, live today)

> **STATUS 2026-07-11: mitigation APPLIED to live.** `plan_cache_mode =
force_custom_plan` set on `match_document_table_facts_text`,
> `match_document_memory_cards_hybrid`, `match_document_index_units_hybrid`,
> and `match_document_embedding_fields_hybrid`; verified in `proconfig` with
> identical row counts and unchanged steady-state timings before/after
> (in-session steady state: ~1132 ms / ~10 ms / ~494 ms / ~13 ms). Codified in
> `supabase/migrations/20260711120000_retrieval_fn_plan_cache_mode.sql`. The
> "full fix" below (plpgsql + dynamic EXECUTE, ~70 ms) remains open and still
> requires the schema.sql body-drift reconciliation first. The
> `statement_timeout` idea from the round-2 perf plan was rejected: a
> function-level SET cannot re-arm a running statement's timer, and a
> role-level timeout would endanger long ingestion RPCs on the same role —
> client-disconnect cancellation is handled app-side via `.abortSignal()`.

**The original diagnosis in this section was wrong**, and the fix it proposed
(rewrite the fuzzy disjunct to the `%` operator) **has already been applied to
live** and did not help. Re-profiling on live (read-only) established the real
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
-- Force custom plans for the non-inlined retrieval RPCs (scale-readiness F1).
--
-- These functions are LANGUAGE sql/plpgsql with SET search_path, which makes
-- them non-inlinable: their bodies are planned with unknown parameters, and the
-- planner cannot estimate trigram/tsv selectivity for an unknown text value.
-- Once a pooled connection caches the resulting generic plan, every call pays
-- near-seq-scan strategies — match_document_table_facts_text measured 5.3 s per
-- call on live pooled connections, ~1.1 s with a custom plan (≈4.8x), and the
-- answer path fans it out up to 3x in parallel per cold request.
--
-- plan_cache_mode = force_custom_plan is results-identical (it changes only
-- which plan is used, never which rows qualify), so no retrieval eval gate
-- applies. Applied to the live Clinical KB Database on 2026-07-11 with
-- before/after verification (identical row counts, steady-state timings
-- unchanged, generic-plan mode now unreachable); this migration codifies the
-- setting for other environments.
--
-- Deliberately NOT included: statement_timeout on these functions. A
-- function-level SET cannot re-arm the timeout for the statement already in
-- flight, and a role-level timeout would endanger the minutes-long ingestion
-- RPCs that run under the same service role. Client-disconnect cancellation of
-- retrieval RPCs is handled app-side via .abortSignal() (see rag.ts withAbort).

alter function public.match_document_table_facts_text(text, integer, uuid[], uuid)
set plan_cache_mode = 'force_custom_plan';

alter function public.match_document_memory_cards_hybrid(vector, text, integer, double precision, uuid[], uuid)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Qualify pgvector signatures in the migration

This file does not set search_path, so in any migration/SQL execution context where extensions is not already on the path, PostgreSQL cannot resolve the unqualified vector type and the migration aborts before applying the plan-cache settings. The repo installs pgvector in the extensions schema and most signature checks use extensions.vector; qualify these three ALTER FUNCTION signatures (or set search_path = public, extensions) to make replay independent of the caller's session state.

Useful? React with 👍 / 👎.

set plan_cache_mode = 'force_custom_plan';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mirror function GUCs in drift state

This migration changes the canonical function definitions via proconfig, but supabase/schema.sql and supabase/drift-manifest.json are not updated in the same change. The drift snapshot hashes pg_get_functiondef(...), and the allowlist currently covers only match_document_table_facts_text, so after this is applied (the doc says it already was on live) npm run check:drift will report unexpected mismatches for the three hybrid RPCs and mask future real drift. Please mirror these SET plan_cache_mode clauses into the schema/manifest path or explicitly account for the new drift.

Useful? React with 👍 / 👎.


alter function public.match_document_index_units_hybrid(vector, text, integer, double precision, uuid[], uuid)
set plan_cache_mode = 'force_custom_plan';

alter function public.match_document_embedding_fields_hybrid(vector, text, integer, double precision, uuid[], uuid)
set plan_cache_mode = 'force_custom_plan';
Loading