From 5f36914c0440f1dba6044c8d9ed6c9dc069e66d0 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 11:55:09 +0800 Subject: [PATCH] perf: force custom plans for the non-inlined retrieval RPCs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 phase 6 (scale-readiness F1). These LANGUAGE sql/plpgsql functions carry SET search_path, so they are non-inlinable and their bodies get planned with unknown parameters; a pooled connection that caches the generic plan pays near-seq-scan strategies on every call (match_document_table_facts_text measured 5.3 s per call live, ~1.1 s with a custom plan, fanned out up to 3x in parallel per cold answer). plan_cache_mode = force_custom_plan on the four profiled victims: table-facts text, memory-cards hybrid, index-units hybrid, embedding-fields hybrid. Results-identical by construction (only the plan changes, never which rows qualify) — no eval gate. Applied to the live Clinical KB Database (sjrfecxgysukkwxsowpy) 2026-07-11 with before/after verification: settings visible in proconfig, identical row counts, steady-state timings unchanged (~1132/10/494/13 ms), and the cached generic-plan mode now unreachable. This migration codifies the setting for other environments; ALTER ... SET is idempotent. statement_timeout was deliberately dropped from this phase: a function-level SET cannot re-arm the running statement's timer, and a role-level timeout would endanger the minutes-long ingestion RPCs on the same service role. Client-disconnect cancellation is handled app-side via .abortSignal() (#480). Co-Authored-By: Claude Fable 5 --- docs/scale-readiness-review.md | 14 ++++++++ ...711120000_retrieval_fn_plan_cache_mode.sql | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 supabase/migrations/20260711120000_retrieval_fn_plan_cache_mode.sql diff --git a/docs/scale-readiness-review.md b/docs/scale-readiness-review.md index 1beff4033..f787ac39c 100644 --- a/docs/scale-readiness-review.md +++ b/docs/scale-readiness-review.md @@ -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 diff --git a/supabase/migrations/20260711120000_retrieval_fn_plan_cache_mode.sql b/supabase/migrations/20260711120000_retrieval_fn_plan_cache_mode.sql new file mode 100644 index 000000000..2d3183db0 --- /dev/null +++ b/supabase/migrations/20260711120000_retrieval_fn_plan_cache_mode.sql @@ -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) + set plan_cache_mode = 'force_custom_plan'; + +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';