mirror of
https://github.com/block/buzz.git
synced 2026-11-04 08:21:48 +00:00
d3a4909cca
## 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(<readable channels>) 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 <smohammed@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>