From d3a4909ccad7c75a8c13ca91ccdaffd2c83024df Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Tue, 3 Nov 2026 13:25:27 -0500 Subject: [PATCH] perf(search): rank only the newest prefix matches and plan with the real tsquery (#8976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Message search is one of the slowest relay queries. Short typeahead prefixes make it worst: `a:*` matches most of a community. **Why it was slow:** the tsquery was built inside a `CROSS JOIN LATERAL` subquery, so the planner saw an InitPlan and couldn't read the `search_tsv` statistics. It fell back to the default 0.5% selectivity for `@@`: for `a:*` on 1M rows it estimated 4,497 rows when 517,065 matched. Every search therefore took every GIN match, ran `ts_rank_cd` on each one, and top-N sorted them before `LIMIT 20`. This PR: 1. **Binds the real tsquery.** `push_tsquery` is unchanged. A separate `SELECT (…)::text` round trip resolves the tsquery, and the search binds it as `$q::tsquery`. An empty tsquery returns no hits without running the search. 2. **Plans each search for its own tsquery.** The search runs with `.persistent(false)`. A cached prepared statement can switch to a generic plan after five runs, and a generic plan can't see the tsquery. Forcing a generic plan lost the gain in testing. 3. **Ranks only the newest 1,000 prefix matches.** Prefix-mode relevance search ranks the newest `PREFIX_RANK_WINDOW` (1,000) matches. The inner `ORDER BY created_at DESC, id DESC LIMIT 1000` matches each partition's primary key read backwards, so for common prefixes Postgres streams the newest rows and stops at 1,000. For rarer terms it keeps the GIN bitmap plan, now with an accurate estimate. Full-text search and profile (kind:0) typeahead still rank every match. ### Behavior change: every human message search (product decision needed) Prefix mode isn't limited to keystroke typeahead. buzz-app (`useSearchMessages.ts`) and legacy desktop (`desktop/src-tauri/src/commands/messages.rs:184`) send `search_mode: "prefix"` for **all** message search. Full-text search runs only on the WS REQ path, which agents and the CLI use. So for people: - Results are the best-ranked of the newest 1,000 matches in the search's scope, not of all matches. If more than 1,000 newer messages also match, an older, more relevant message drops out of ⌘K results. The scope includes `#channel`, `from:` and date bounds, so a narrower search has the same limit within it. People have no full-text fallback. - The bridge honors `page`, and buzz-app loads later pages at 20 per page. Every page ranks the same window, so paging stays consistent, but results end after 1,000 matches (page 50). Full-text search (agents/CLI) and profile (kind:0) typeahead still rank every match. ### Perf risk: matches the backward scan has to walk far to find The planner picks the backward scan when it expects 1,000 matches near the top of the community, and it estimates that by treating each filter as independent. Two known shapes break that assumption. **Terms common in old messages but absent from recent ones.** The planner assumes matches are spread evenly over time. For a term that was common long ago but doesn't appear recently (an old project name, a former teammate), it can pick the backward primary-key scan and walk every newer row before reaching the first match. Reviewers measured this on 1.2M synthetic rows: about 2.05–2.1 s, against 0.7–1.8 s for the old query shape and 1.2 s with the GIN plan forced. The 20 s statement timeout and the #8954 cancel still bound it. The worst case is close to the old `a:*` worst case, but it now scales with community size, not match count. The planned EXPLAIN on production-shaped data will include such a term. **Terms common in channels the searcher can't read.** Human search always adds `channel_id = ANY() OR channel_id IS NULL` (`build_search_channel_scope_filter`). A term that is common in private channels or other people's DMs can get a high match estimate, take the backward scan, and walk the community's rows to find a few readable matches, where the old shape used the GIN bitmap. This hasn't been reproduced yet; the production-shaped EXPLAIN will include such a term next to the old-only one. The same timeout and cancel bound it. ### Cost - One extra round trip per search to resolve the tsquery (about 0.03 ms of execution). It runs on the same pool checkout as the search (`cancel::fetch_all` takes the caller's connection), so a busy pool queues each search once. An IMMUTABLE SQL function could remove the round trip, but that needs a migration, so it's not done here. ### Related issue - Follows #8954 (cancel abandoned searches, bound the search pool) and #8964 (closed in favor of #8954). - Builds on #8947 (3-character server-side prefix minimum), merged from main. The resolve step passes its minimum. When the trailing prefix is too short, the resolved tsquery is NULL, and the search returns no hits without running the second query. - Client-side pairing: block/buzz-app#2210 (3-character last-word minimum for ⌘K message search). ## Testing **EXPLAIN ANALYZE on a synthetic 1M-row `events` table** (monthly partitions, GIN on `search_tsv`, kinds 9/40002/40008, custom plans, warm cache): | Prefix | Matches | Before | After | Plan after | |---|---|---|---|---| | `a` | 517k | 322 ms | **0.7 ms** | backward primary-key scan per partition, stops at 1,000 | | `th` | 137k | 127 ms | **1.2 ms** | backward primary-key scan, stops at 1,000 | | `a` (community with 10% of rows) | — | ~100 ms | **1–2 ms** | backward primary-key scan | | `deploy` | 8.2k | 15 ms | 5–10 ms | GIN bitmap, accurate estimate | | `d278` | 500 | 1.1 ms | 0.5 ms | GIN bitmap | | rare word | 0 | 0.07 ms | 0.03 ms | GIN bitmap | **Postgres integration tests:** - New test `prefix_relevance_ranks_only_the_newest_matches`. An old, highly relevant message drops out of prefix results behind 1,000 newer matches but still ranks first in full-text search. Prefix pages end at page 50. An old, best-ranked profile still tops profile typeahead behind 1,000 newer profile matches. Removing the window fails the test, and so does applying it to profiles. - `abandoned_search_cancels_its_statement`: the `pg_stat_activity` matcher now looks for `ts_rank_cd(`, because the old `search_query.query` text no longer exists. - Locally, each test ran against a fresh database because `cargo-nextest` isn't installed, so the hosted lane hasn't run yet. 26 of 27 pass. `artifact_search_uses_only_live_current_head` also fails on unmodified main in that local setup, so the failure isn't from this change. **Checks:** `cargo fmt --check` and `cargo clippy -p buzz-search --all-targets -D warnings` are clean. **Not yet done:** - hosted CI; - a manual check against a local relay with ⌘K typeahead; - EXPLAIN against production-shaped data, including a term common only in old messages and a term common only in channels the searcher can't read. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Salman Mohammed Co-authored-by: Claude Opus 5.5 --- ARCHITECTURE.md | 15 ++- crates/buzz-search/src/cancel.rs | 8 +- crates/buzz-search/src/query.rs | 83 ++++++++++++---- .../tests/postgres_fts_integration.rs | 97 ++++++++++++++++++- 4 files changed, 177 insertions(+), 26 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 23961d940c..b7e5df8d71 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -582,12 +582,16 @@ re-authorizes every candidate hit before returning it. **Key behaviors:** - `SearchService::new(pool)` wraps a `PgPool`; `search(&SearchQuery)` runs a parameterized FTS query against the `events.search_tsv` GIN index and returns - `SearchResult` (candidate `SearchHit`s). + `SearchResult` (candidate `SearchHit`s). Prefix-mode relevance (outside + profile search) ranks only the newest 1000 matches, which Postgres may read + from the primary key backward instead of the GIN index. - `ChannelScope` makes the channel constraint explicit (`Any` / `ChannelLessOnly` / `Channels` / `ChannelsOrChannelLess`), closing the ambiguity the old `Option> + bool` matrix could not express. -- Every query carries `community_id`; the FTS predicate is BitmapAnd-ed with - the community-leading btree filters so a query never crosses tenants. +- Every query carries `community_id`, and every access path leads with it (the + FTS predicate BitmapAnd-ed with community-leading btree filters, or the + `(community_id, created_at, id)` primary key read backward), so a query never + crosses tenants. - Permission filtering is **caller's responsibility** — `buzz-search` returns candidate hits; the relay re-authorizes each one (channel membership, `#p`, owner gates) before delivering it. @@ -959,7 +963,10 @@ kinds via `CASE WHEN kind IN (1059, 30300, 30622) THEN NULL`, so those rows are storage-level unsearchable (a `NULL` tsvector never matches `@@`). A GIN index (`idx_events_search_tsv`) backs the `@@` probe; in multi-community mode the community-leading btree filters BitmapAnd with the GIN probe so every query is -fenced to its `community_id`. +fenced to its `community_id`. Prefix-mode relevance ranks only the newest 1000 +matches; for a common prefix Postgres reads the `(community_id, created_at, id)` +primary key backward and stops there, still fenced by its leading +`community_id`. --- diff --git a/crates/buzz-search/src/cancel.rs b/crates/buzz-search/src/cancel.rs index 1ca4bf2090..9549d39ba2 100644 --- a/crates/buzz-search/src/cancel.rs +++ b/crates/buzz-search/src/cancel.rs @@ -57,13 +57,15 @@ pub const CANCEL_SQL: &str = concat!( " = $2" ); -/// Run `query` on a connection from `pool`, cancelling its statement on the -/// server if the returned future is dropped before it completes. +/// Run `query` on `conn`, a connection from `pool`, cancelling its statement +/// on the server if the returned future is dropped before it completes. +/// Callers pass the connection so earlier steps of the same search reuse one +/// pool checkout. pub(crate) async fn fetch_all( pool: &PgPool, + mut conn: PoolConnection, query: Query<'_, Postgres, PgArguments>, ) -> Result, sqlx::Error> { - let mut conn = pool.acquire().await?; let backend: (i32, i64) = sqlx::query_as(BACKEND_SQL).fetch_one(&mut *conn).await?; let mut armed = CancelOnDrop { pool: pool.clone(), diff --git a/crates/buzz-search/src/query.rs b/crates/buzz-search/src/query.rs index facd9a7f37..07ecf29277 100644 --- a/crates/buzz-search/src/query.rs +++ b/crates/buzz-search/src/query.rs @@ -62,17 +62,20 @@ pub enum SearchMode { /// Prefix-match the trailing normalized query token (`pro` matches `project`). /// /// Intended for bounded typeahead surfaces such as the desktop topbar. The - /// relay still refetches and re-authorizes every hit; this mode changes only - /// the candidate tsquery, not the access boundary. A trailing token that - /// yields a lexeme shorter than three characters returns no hits unless the - /// query is kind:0-only (see `PREFIX_MIN_CHARS`). + /// relay still refetches and re-authorizes every hit; this mode changes the + /// candidate tsquery and, for relevance outside profile search, ranks only + /// the newest 1000 matches. It does not change the access boundary. A + /// trailing token that yields a lexeme shorter than three characters + /// returns no hits unless the query is kind:0-only (see `PREFIX_MIN_CHARS`). Prefix, } /// Result ordering for a search. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SearchSort { - /// `ts_rank_cd` relevance, newest first among equal ranks. + /// `ts_rank_cd` relevance, newest first among equal ranks. In + /// [`SearchMode::Prefix`] outside profile search, only the newest 1000 + /// matches are ranked, so pages end with them. Relevance, /// Newest first, ignoring relevance; ties ordered by event id. Newest, @@ -151,6 +154,13 @@ const SEARCH_TEXT_MAX_CHARS: usize = 4096; /// bridge uses page 1), but clamp here too so a future caller cannot accidentally /// wire untrusted input into a multi-trillion-row OFFSET. const PAGE_MAX: u32 = 1000; +/// How many of the newest matches a prefix-mode relevance search ranks. +/// The GIN index returns matches in no useful order, so ranking every match +/// before `LIMIT` costs time proportional to the match count, and a short +/// typeahead prefix matches most of a community. Reading candidates newest +/// first lets Postgres stop at this many instead. Every page ranks the same +/// window, so pages stay consistent and end with it. +const PREFIX_RANK_WINDOW: i64 = 1000; /// Shortest lexeme [`SearchMode::Prefix`] gives `:*` for anything but a /// kind:0-only (profile) query. If the trailing token yields a shorter one, /// the tsquery is NULL and the search returns no hits, whatever completed @@ -236,15 +246,18 @@ fn normalized_search_text(q: &str) -> Option { /// Execute a community-scoped FTS query. /// -/// SQL shape (always): +/// The mode-specific tsquery is resolved first and bound as a value, so the +/// planner estimates its selectivity instead of assuming a rare match. The +/// search itself is a one-shot statement, planned for that value every time. +/// +/// SQL shape: /// ```sql /// SELECT id, kind, pubkey, channel_id, EXTRACT(EPOCH FROM created_at)::bigint AS created_at_s, -/// ts_rank_cd(search_tsv, query) AS rank -/// FROM events, -/// AS query +/// ts_rank_cd(search_tsv, $q) AS rank +/// FROM events -- prefix relevance: the newest PREFIX_RANK_WINDOW matches /// WHERE community_id = $ctx /// AND deleted_at IS NULL -/// AND search_tsv @@ query +/// AND search_tsv @@ $q /// [+ channel scope, kinds, authors, since, until] /// ORDER BY rank DESC, created_at DESC, id -- SearchSort::Newest: created_at DESC, id /// LIMIT $per_page OFFSET (($page - 1) * $per_page) @@ -279,18 +292,42 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result = QueryBuilder::new("SELECT ("); + push_tsquery(&mut resolve, query.mode, &search_text, min_prefix_chars); + resolve.push(")::text"); + let tsquery: Option = resolve.build_query_scalar().fetch_one(&mut *conn).await?; + let Some(tsquery) = tsquery.filter(|tsquery| !tsquery.is_empty()) else { + // A too-short trailing prefix (NULL) or nothing searchable after + // normalization (empty) matches nothing. + return Ok(SearchResult { + hits: Vec::new(), + page, + }); + }; let mut qb: QueryBuilder = QueryBuilder::new( "SELECT id, kind, pubkey, channel_id, \ EXTRACT(EPOCH FROM created_at)::bigint AS created_at_s, \ - ts_rank_cd(search_tsv, search_query.query) AS rank \ - FROM events CROSS JOIN LATERAL (SELECT ", + ts_rank_cd(search_tsv, ", ); - let min_prefix_chars = if profile_only { 0 } else { PREFIX_MIN_CHARS }; - push_tsquery(&mut qb, query.mode, &search_text, min_prefix_chars); - qb.push(" AS query) AS search_query WHERE community_id = "); + qb.push_bind(&tsquery); + qb.push("::tsquery) AS rank FROM "); + if rank_window.is_some() { + qb.push("(SELECT id, kind, pubkey, channel_id, created_at, search_tsv FROM "); + } + qb.push("events WHERE community_id = "); qb.push_bind(*query.community.as_uuid()); - qb.push(" AND deleted_at IS NULL AND search_tsv @@ search_query.query"); + qb.push(" AND deleted_at IS NULL AND search_tsv @@ "); + qb.push_bind(&tsquery); + qb.push("::tsquery"); // Search is a current-state surface; old artifact snapshots cannot match. qb.push(" AND (events.kind <> 45010 OR EXISTS (SELECT 1 FROM artifact_heads ah WHERE ah.community_id=events.community_id AND ah.event_id=events.id AND NOT ah.deleted))"); @@ -346,6 +383,14 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result Result = row.try_get("id")?; diff --git a/crates/buzz-search/tests/postgres_fts_integration.rs b/crates/buzz-search/tests/postgres_fts_integration.rs index d79e240764..b68cf1a41f 100644 --- a/crates/buzz-search/tests/postgres_fts_integration.rs +++ b/crates/buzz-search/tests/postgres_fts_integration.rs @@ -745,6 +745,101 @@ async fn prefix_mode_preserves_storage_level_privacy_exclusions() { teardown(pool, &schema).await; } +/// Prefix relevance (typeahead) ranks only the newest 1000 matches, so an +/// older, better-ranked message drops out, and its pages end with that +/// window; full-text and profile relevance still rank every match. +/// Mutation: rank every prefix match → the old message tops the first page; +/// window profiles too → the old profile drops off the first page. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn prefix_relevance_ranks_only_the_newest_matches() { + let (pool, schema) = setup().await; + let c = mk_community(&pool, "rank-window.example").await; + let (old, old_profile) = (rand_bytes32(), rand_bytes32()); + for (id, kind) in [(old, 9), (old_profile, 0)] { + insert_event( + &pool, + c, + id, + rand_bytes32(), + kind, + "planning planning planning planning", + None, + 1_700_000_000, + ) + .await; + // 1000 newer, weaker matches fill the window. + sqlx::query( + "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig) \ + SELECT $1, sha256((g || '/' || $2)::bytea), sha256((g || '/' || $2)::bytea), \ + to_timestamp(1700000001 + g), $2, '[]'::jsonb, 'planning notes ' || g, 'signature' \ + FROM generate_series(1, 1000) g", + ) + .bind(c.as_uuid()) + .bind(kind) + .execute(&pool) + .await + .expect("insert newer matches"); + } + + let svc = SearchService::new(pool.clone()); + let query = |q: &str, mode, page| SearchQuery { + community: c, + q: q.into(), + channel_scope: ChannelScope::Any, + kinds: Some(vec![9]), + authors: None, + since: None, + until: None, + page, + per_page: 20, + mode, + sort: buzz_search::SearchSort::Relevance, + }; + let ids = |r: buzz_search::SearchResult| r.hits.iter().map(|h| h.event_id).collect::>(); + + let typeahead = ids(svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 1)) + .await + .expect("prefix")); + assert_eq!(typeahead.len(), 20); + assert!(!typeahead.contains(&old), "outside the newest-1000 window"); + + let full_text = ids(svc + .search(&query("planning", buzz_search::SearchMode::FullText, 1)) + .await + .expect("full text")); + assert_eq!(full_text.first(), Some(&old), "full text ranks every match"); + + // Every page ranks the same window, so prefix pages end with it. + let last = ids(svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 50)) + .await + .expect("page 50")); + assert_eq!(last.len(), 20); + assert!(!last.contains(&old)); + let past = svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 51)) + .await + .expect("page 51"); + assert!(past.hits.is_empty(), "no page past the window"); + + let profiles = ids(svc + .search(&SearchQuery { + kinds: Some(vec![0]), + ..query("plan", buzz_search::SearchMode::Prefix, 1) + }) + .await + .expect("profile prefix")); + assert_eq!( + profiles.first(), + Some(&old_profile), + "profile typeahead ranks every match" + ); + + teardown(pool, &schema).await; +} + #[tokio::test] #[ignore = "requires Postgres"] async fn channel_scope_restricts_results() { @@ -1990,7 +2085,7 @@ async fn waiting_searches(pool: &PgPool) -> i64 { sqlx::query_scalar( "SELECT count(*) FROM pg_stat_activity WHERE pid <> pg_backend_pid() \ AND datname = current_database() \ - AND wait_event_type = 'Lock' AND query LIKE '%search_query.query%'", + AND wait_event_type = 'Lock' AND query LIKE '%ts_rank_cd(%'", ) .fetch_one(pool) .await