Repository navigation
perf(db): split newest-first events reads at a month floor so Postgres prunes partitions - #9091
Conversation
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated multi-lane review at head 5bf0de03. Verdict: changes requested. The floor arithmetic, ordering, tie handling, has_more probe, literal safety, eligibility exclusions, and older-pass error propagation all check out. Two independent code-review lanes converged on one blocker: the two passes don't read the same snapshot.
1. IMPORTANT (correctness): the recent and older passes read different snapshots, so an atomic replacement between them drops a live row
query_events_on runs each pass through its own fetch_with_read_deadline call (crates/buzz-db/src/store/event.rs:578-582). Each call opens and rolls back its own transaction (event.rs:974-1004), or on a ReadDeadline::Session autocommit connection runs as its own statement. Under READ COMMITTED, each pass therefore sees its own snapshot. get_channel_window_on has the same structure, with two plain fetch_all calls (crates/buzz-db/src/store/thread.rs:669-687). The base branch runs one SELECT, which can't see a half-applied replacement.
Concrete failure: a {kinds: [45010], limit: 100} read (artifacts are not NIP-01 replaceable, so floor_for splits it) finds no current artifact head at or above the floor. Before the older pass runs, an edit commits. It replaces the artifact_heads row for an old revision with a new revision dated now (crates/buzz-db/src/store/artifact.rs:196-199, one transaction). The older pass now drops the old revision through the generic-read artifact_heads predicate (event.rs:694-698), and the new revision falls outside the older pass's range. The page comes back short or empty, and looks exhausted, even though a matching live head existed the whole time. Mixed-kind reads that include replaceable or addressable kinds have the same gap: the old version sits below the floor, and its replacement lands above it between the passes. The "no row is read twice and the page is exact" guarantee in recent_pass.rs:17-19 holds only within a single snapshot.
Corrective action: when the read isn't already inside a transaction with a fixed snapshot, run both passes in one REPEATABLE READ, READ ONLY transaction (with the read deadline applied once to that transaction). Otherwise, don't split. Apply the same fix to the channel-window path, and add a Postgres test that commits a head replacement between the two passes.
2. MINOR: a page that doesn't fill above the floor gets twice the read deadline
Each pass is bounded by its own READ_STATEMENT_TIMEOUT, so a split read can take up to twice the deadline the unsplit read had. Running both passes in a single transaction under one deadline, as in finding 1, fixes this too.
|
🤖 Thanks, both findings were right. Fixed in 31c3999. 1. Separate snapshots. A split read now runs both passes in one New Postgres tests: 2. Double deadline. Each statement after the first now runs under whatever is left of Mutants in the gate: opening the split read |
…es partitions A newest-first page read with no created_at lower bound plans and locks every monthly partition of `events` and every partition index, about 200 relations, which overflows the backend's fast-path lock slots and contends on LWLock:LockManager. query_events_on and get_channel_window_on now split eligible reads at the start of the previous UTC month (store/recent_pass.rs). The recent pass reads created_at >= floor as a SQL literal, so Postgres prunes at plan time; when it returns a full page that page is the answer. Otherwise the older pass reads created_at < floor for just the missing rows. Both passes run in one REPEATABLE READ, READ ONLY transaction (ReadTransaction), so they read one snapshot, and each later pass runs under what is left of the 20 s read deadline. Inside a transaction the passes take a savepoint; the callers that read inside one are routed replica reads, already REPEATABLE READ. Lookups (ids, #e including the thread-reply route, #d), OFFSET pages, fetch-everything reads, reads whose own bounds already prune, and reads of only replaceable or addressable kinds are not split. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
31c3999 to
f65a8b6
Compare
The fixture writes events partitions directly, which the guarded-write source scan rejects in test_support.rs. Standalone *_tests.rs modules are outside the scan. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
The split channel window opened its ReadTransaction with no deadline, so each pass ran under the session's own statement_timeout: two full budgets on a reader, two writer budgets on the writer. Windows now take a ReadDeadline like every other event read (Session on a routed reader, PerRead on the writer), and their passes share it. The deadline is no longer optional on ReadTransaction, and fetch_with_read_deadline takes a built query so the unsplit window uses it too. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
A second case of the window deadline test keeps the newest partition locked for good, so the window is cancelled in its recent pass. It fails if the writer entry hands the window the Session deadline, which leaves the first pass under the writer session's longer timeout. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Approved at 8f62e008. Two independent source reviews found no issues. Pages stay exact: the two reads cover ranges that don't overlap, split on the leading sort key, and the older read fetches only the missing rows. The channel window keeps its extra row for has_more. Both reads share one REPEATABLE READ, READ ONLY snapshot and one 20 s read deadline, which fixes both findings from the earlier review. If the older read fails, the read returns an error rather than a partial page. The month floor goes into the SQL as a fixed-format UTC timestamp, with no caller input in it. Offset reads, id/tag lookups, max_limit reads and reads limited to replaceable kinds still run as one query. Channel windows on the primary database now time out after 20 s instead of 2 minutes. That matches every other event read.
…p-cache * origin/main: perf(db): split newest-first events reads at a month floor so Postgres prunes partitions (#9091) fix(relay): bound mesh dials and keep failing pods out of the mesh (#8752) Signed-off-by: Ravneet Arora <rarora@squareup.com> Co-authored-by: maincourse <37ce551b493d4a998caf030929c47bce0b6a0e43f1178b3a63c4151d7c16b76e@buzz.block.builderlab.xyz>
## Summary Explicit multi-channel `#h` reads whose channels × page limit fit a budget of 8,192 now read each channel through the channel index and keep the newest `n` rows overall. Before, Postgres was left to plan `channel_id = ANY($set) ORDER BY created_at DESC LIMIT n` from a row estimate. Stacked on #9091 (both change `build_query_events_sql`). It retargets to `main` when that merges. ## Problem The desktop app's cold unread repair reads every joined channel in chunks, with `kinds [9, 40002, 40008]`, `include_aux`, `limit 500` and no `since`. It never fills its page. For that shape, Postgres's custom plans (the first executions of each prepared statement on a connection) overestimate how many rows a quiet channel set matches. They walk the partition's primary key backward and test every community row against the set. The inflated cost also turns on JIT. On a load-test database with about 940k events in one partition (`EXPLAIN ANALYZE`, custom plan): | Channels in the set | Plan | Rows returned | Time | |---|---|---|---| | 3 | channel index | 4 | 0.5 ms | | 6 | channel index, plus JIT | 254 | 54 ms (1.2 ms scan) | | 15 | pkey walk, 940k rows filtered | 427 | 546 ms | | 48 | pkey walk, 299k rows filtered | 863 | 197 ms | | 128 | channel index | 500 | 5 ms | Generic plans pick the channel index, but every connection runs custom plans first. A client-side `since` would not avoid the walk: it still walks the whole window. It would also change unread semantics, and it does nothing for app versions already shipped. ## Change `build_query_events_sql` handles plain explicit `#h` reads like this: ```sql SELECT e.* FROM (SELECT DISTINCT unnest($set::uuid[]) AS ch) c CROSS JOIN LATERAL ( SELECT … FROM events WHERE community_id = $1 … AND channel_id = c.ch … ORDER BY created_at DESC, id ASC LIMIT $n ) e ORDER BY e.created_at DESC, e.id ASC LIMIT $n OFFSET 0 ``` - **Same page as the set read:** every row of the overall top `n` is also in its own channel's top `n`. - **Bounded candidates:** at most distinct channels × `n` rows reach the outer sort, whatever the statistics say. How many index entries each channel scans still depends on the other predicates, such as `kinds`. - **Same SQL for any number of channels:** the channel set stays one array bind. - **Repeated ids:** `DISTINCT` reads a repeated id once. - **Per pass:** #9091's recent and older passes each use this shape. These keep today's SQL: scopes that include global events (`channel_id IS NULL OR ANY`), `#p` mention joins, e-tag fences, offset pages, the COUNT, and sets whose channels × page limit exceed the budget. ### Candidate budget A per-channel read fetches up to `n` rows from every channel, where the set read stops once the page fills. So busy channel sets cost more. On a synthetic community (920k events; 128 busy channels with 1,000 recent messages and 1,000 reactions each, 400 more busy channels, 128 quiet channels with 3 messages each), the repair shape (`kinds [9, 40002, 40008]`, recent pass), JIT off: | Set × limit | Set read, custom plan | Set read, generic plan | Per-channel | |---|---|---|---| | 128 busy × 500 | 2.3 ms | 238 ms | 114 ms | | 20 busy + 108 quiet × 500 | 19 ms | 33 ms | 18 ms | | 128 quiet × 500 | 473 ms | 0.7 ms | 1.3 ms | | 15 busy × 500 | 22 ms | 23 ms | 13 ms | | 15 quiet × 500 | 0.2 ms | 0.2 ms | 0.3 ms | | 128 busy × 100 | 0.7 ms | | 17 ms | | 128 quiet × 100 | 141 ms | | 1.2 ms | With JIT on (the default), the per-channel custom plan for 128 channels × 500 crossed the JIT thresholds: about 0.9 s for the busy set and 0.75 s for the quiet one, nearly all compile time. At 15 × 500 it stayed under them. So the per-channel read runs only while channels × page limit ≤ 8,192 (`PER_CHANNEL_CANDIDATE_BUDGET`, the clamped page limit). That bounds it at about 8k candidate rows, where it was still faster than the set read here. Larger sets keep the set read, as on `main`. Either way there are two SQL texts, so plan caching holds. The desktop repair's full 128-channel chunks at `limit 500` (64,000) are over the budget and keep the set read. Only its last, partial chunk (16 channels or fewer) and other small explicit sets read per channel. The load-rig numbers below predate the budget and don't show its effect. On the same data, the per-channel form took 0.5 ms (6 channels), 0.9 ms (15) and 4.2 ms (128). Load rig, cold desktop reconnect scenario (every client runs the unread repair), #9091 alone vs this PR on top: | | Pool default: #9091 | Pool default: + this PR | Pool 200: #9091 | Pool 200: + this PR | |---|---|---|---|---| | Repair read mean | 302 ms | 41 ms | 1,990 ms | 93 ms | | Repair read total DB time | 1,547 s | 245 s | 9,622 s | 561 s | | Repair reads served | 5,121 | 5,915 | 4,834 | 6,011 | | Client op failures | 23,475 | 10,966 | 28,797 | 10,829 | | Unread-load errors | 1,018 | 113 | 1,434 | 3 | | Relay pool-timeout log lines | 8,762 | 824 | 13,860 | 409 | The rig stays overloaded in every arm. Most remaining failures are the per-connection concurrent-request limit and connect/auth under load, not this read. ## Tests - `per_channel_reads_match_the_set_read` (Postgres) covers: - every limit from 1 to total+1, including a page held entirely by one channel - newer rows of another kind or another channel taking no slot - a repeated channel id and an id tie across channels - keyset paging across the tie, and `since` - rows below the recent-pass floor - a prepared-statement check that the read ran per channel and no `ANY` read ran - `per_channel_scope_eligibility` covers which reads keep the current SQL, including the budget edge (16 × 512 in, 17 × 512 and 128 × 500 out) and the clamped page limit. - `channel_scope_sql_does_not_depend_on_channel_count` now also asserts the per-channel shape for explicit `#h` pages. ## What got simpler Not much. This adds one SQL shape to the events query builder, behind one eligibility function and one budget. In exchange, the planner's row estimate no longer decides whether a small multi-channel read scans a whole partition. The channel predicate is decided in one place: `push_event_filters` no longer pushes the channel scope, and each caller (the page read and the thread route) pushes its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Newest-first
eventspage reads are now split at the start of the previous UTC month, so Postgres can prune the older partitions whenever the newest ones fill the page. Results are unchanged.Why.
eventsis range-partitioned by month, and every partition carries the full index set. A page read with nocreated_atlower bound can't prune, so Postgres plans and locks every partition and every partition index: about 200 relations per read with 19 partitions. A backend has 16 fast-path lock slots, and every lock past those goes through the shared lock table. On a production-shaped load rig at pool 200, active backends spent 36% of their time waiting onLWLock:LockManagerand 1.6% on disk.What changes.
query_events_on(REQ history, bridge/query, mentions, rosters, notices) andget_channel_window_on(channel windows) now read in up to two passes on the same connection, through one helper (store/recent_pass.rs):created_at >= <floor>, where the floor is the start of the previous UTC month. When it returns a full page, that page is the answer, because every row at or above the floor sorts before every row below it.created_at < <floor>, limited to the rows still missing, appended after the recent rows. The ranges don't overlap, so no row is read twice and the page is exact. For channel windows the limit includes the +1 probe row, sohas_morestays exact.One snapshot, one deadline. The two passes run in one
REPEATABLE READ, READ ONLYtransaction (ReadTransaction), so both read the same snapshot. UnderREAD COMMITTED, a replacement committing between the passes (a replaceable event, an artifact head) could move the live row from below the floor to above it, and it would land in neither pass. Inside a transaction the passes run in a savepoint instead; the only callers that read inside a transaction are routed replica reads, which are alreadyREPEATABLE READ. Each pass after the first runs under whatever is left of the 20 s read deadline, so a split read never gets more time than an unsplit one. Single-statement reads (COUNT, unsplit pages) keep their old path, now through the same type. Channel windows now take the read deadline too. On a routed reader that is the reader's session deadline. On the writer they used to run under the writer's own session timeout (2 minutes), and now get the 20 s read deadline, like every other event read there.The floor is a SQL literal, not a bind parameter, so Postgres prunes when it plans the statement, cached generic plans included. It changes once a month, so the statement cache holds one statement per pass and shape.
#preads put the same bound onevent_mentions.event_created_at.These reads are not split:
sinceat or after the floor) or fall entirely before the floor (untilbefore it).OFFSETreads, since the offset spans both sides of the floor.ids,#e,#d, artifactdtags.max_limit, the COUNT fallback).No protocol, schema, index or client change.
Not covered: COUNT and the per-row aux
#ereads, neither of which has a page to fill. The recent pass also has no upper bound, because rows dated in the future must stay visible. So partitions ahead of the current month (the manager keeps 6) and the catch-all are still planned.Evidence
A production-shaped load rig (6,600 desktop clients connecting cold at 100/s, against a 19-partition
eventstable), comparingmainwith this branch. Each row is one arm, so expect run-to-run noise of about ±10%.mainmainmain(repeat)Pages that fill above the floor stop planning old partitions, and that halves the locks taken outside the fast path.
LWLock:LockManagerstays at about 41% of active backend samples at pool 200, though. The dominant remaining cost there is the cold unread repair: a multi-channel read with nosinceand a page that never fills. That read needs its own fix (a lookback bound or a better plan) and is out of scope here.An earlier version re-ran the whole read after a short recent pass. That doubled the work on non-filling reads: errors went from 4.75% to 11.88% at pool 200. This version reads only the missing older rows.
Tests
recent_pass::tests: floor arithmetic (including the year wrap), the predicate literals, an eligibility table, and an in-memory exactness test that drives the passes at every limit and pins which passes run with which limits.e_tag_filter_shape_tests::split_passes_bound_created_at_with_a_literal/recent_pass_skips_lookups: SQL shape (both thee.andm.sides for#p, no bound on unsplit reads or COUNT) and the exclusions, including offset.postgres_tests::split_page_reads_match_the_unsplit_read(buzz-db event): rows on both sides of the floor. A full recent page is answered alone; short pages at limits 4, 5 and 6 add exactly the missing older rows; offset reads across the floor are not split; a cursor below the floor works. It readspg_prepared_statementsto show which passes ran. With another session locking a partition only the older pass plans (and a shortlock_timeout), a full recent page still answers and a short one returnsErrrather than a partial page.postgres_tests::channel_window_split_matches_the_unsplit_read(buzz-db thread): the same checks for channel windows, includinghas_morewhen the recent pass has no probe row, and anErrwhen the older pass fails.split_read_passes_share_one_snapshot(event, both deadline modes) andchannel_window_passes_share_one_snapshot(thread): another session holds a lock that only the older pass needs, so the read stops between its passes. A replacement then commits that deletes a row below the floor and makes its replacement live above it. The page must still be the first pass's snapshot: the replaced row is present and its replacement is absent.read_statements_share_one_deadline: per-read, session and inside-a-transaction reads. The second statement runs under less than the full deadline, and finishing the read restores the connection's own timeout.channel_window_passes_share_one_deadline(thread): through the production writer entry. The recent pass waits 8 s on a lock and the older pass is blocked for good. The window must be cancelled within the one shared deadline, not a full deadline after its older pass starts.channel_scoped_mention_reads_share_one_prepared_statementnow expects one recent and one older statement, shared by every channel count.Mutation-checked. Each of these mutants fails a new Postgres test: dropping the older pass, giving the older pass the full limit, removing the split, splitting offset reads, dropping the window's probe row from the split limit, removing the window's split, swallowing an older-pass error in either path, opening the split read
READ COMMITTED(events and windows), opening it with a plainBEGIN, giving every pass the full deadline, and running a window with no read deadline.What got simpler
Nothing got simpler overall. This adds one small module (
store/recent_pass.rs) shared by both read paths. The single-statement deadline helper is now a thin wrapper overReadTransaction, so the begin, savepoint and tighten logic lives in one place. The alternative, asincein every client plus a new index per read shape, means more code in more places and does nothing for app versions already shipped.🤖 Generated with Claude Code