perf: force custom plans for the non-inlined retrieval RPCs (round 2, phase 6)#484
Conversation
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 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Updates to Preview Branch (claude/perf-r2-plan-cache-migration) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f36914c04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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'; |
There was a problem hiding this comment.
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_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) |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Summary
Phase 6 of the round-2 performance initiative (stacked on #482). Implements scale-readiness F1 — the single largest measured cold-answer latency win of the round.
The retrieval RPCs are
LANGUAGE sql/plpgsqlwithSET search_path, making them non-inlinable: their bodies get planned with unknown parameters, and once a pooled PostgREST connection caches the generic plan, every call pays near-seq-scan strategies.match_document_table_facts_textmeasured 5.3 s per call in that mode (vs ~1.1 s custom-plan), and the answer path fans it out up to 3× in parallel per cold request.plan_cache_mode = force_custom_planapplied to 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 — so no eval gate applies (per the F1 write-up indocs/scale-readiness-review.md).Live application (done, user-approved)
Applied to
Clinical KB Database(sjrfecxgysukkwxsowpy) on 2026-07-11 via the Supabase connector:proconfigon all four functionsThe migration codifies the setting for other environments;
ALTER … SETis idempotent, and it is body-agnostic so it does not collide with the known schema.sql↔live body drift on this RPC.Deliberately not included
statement_timeouton retrieval functions (was in the round-2 plan): a function-levelSETcannot re-arm the timer for the statement already in flight, 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()(perf: remove serial dead weight from the answer hot path (round 2, phase 3) #480). Rationale recorded in the migration and in the F1 status note.EXECUTE, recovers to ~70 ms): still blocked on the schema.sql body-drift reconciliation; remains open in the doc.Also updated
docs/scale-readiness-review.md: F1 status block marking the mitigation applied + measured verification🤖 Generated with Claude Code