Repository navigation
fix(db): bound every statement and the #p mention scan - #8461
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 review of 9cb6638b1ad9d28f2a0819a6c54f19d2e9ad5ed0 (base dc4cdc4990877d400a435b481a416a73f17247bb): two blocking issues.
Thanks for this. The #p change holds up. Joining on e.created_at = m.event_created_at and repeating since/until on the mention side returns the same rows for since-only, until-only, both, page and COUNT, and the cursor second is still kept whole on m while e does the exact (created_at, id) tie-break. Every production writer copies the signed event second into the mention row. The tighten-only read deadline is correct on both the transaction path and the replica after_connect, and the deletion lifts are transaction-local with no leak into pooled connections. All seven new tests passed by name in the hosted Postgres and unit jobs, and removing the mention bounds or the 20s read deadline makes them fail.
Both blockers are consumers of the new caps that react to 57014 the wrong way.
Blocking: audit entries can now be dropped
The audit pool inherits the writer cap (crates/buzz-relay/src/main.rs:42-51). AuditService::log waits on the community's audit-chain advisory lock (crates/buzz-audit/src/service.rs:58-61). With BUZZ_DB_LOCK_TIMEOUT_MS=0, which is supported, or any lock timeout longer than the statement timeout, an append that waits more than 120s now ends in 57014. The worker only retries 55P03 (is_lock_timeout, crates/buzz-relay/src/state.rs:2420-2451). Every other error is logged and the entry is gone. Before this PR the wait had no cap and the append went through once the lock was released.
Fix: treat a cancelled statement (57014) as a rolled-back, retryable append with the same backoff, and keep admission refusal terminal. Add a regression through the production worker: short statement cap, lock timeout disabled, hold the audit-chain lock past the cap, release it, and assert the original entry is appended. Please don't remove the cap from the audit pool to fix this.
Blocking: a replica read cancelled at 20s now reruns on the writer
Replica sessions now get a 20s statement_timeout (crates/buzz-db/src/runtime/mod.rs:991-999). Six routed reads rerun on the writer after any replica error, including 57014:
- feed mentions, needs-action and activity:
crates/buzz-db/src/store/feed.rs:382-393,467-478,542-552 - thread replies and channel window:
crates/buzz-db/src/store/thread.rs:932-952,1109-1135 - relay membership:
crates/buzz-db/src/store/relay_members.rs:991-994
These run their own SQL rather than the generic event read, so the writer rerun gets the writer's 120s cap per statement, or up to 300s if configured, and no 20s read deadline. Before this PR a slow replica read either finished on the replica or kept running there. The writer never saw it. Now the read is cancelled at 20s and the same query starts on the primary, at the moment we already know it's expensive. ReadSession::query_events (runtime/mod.rs:355-360) and the routed event reads in store/event.rs already return a cancelled statement as-is.
Fix: check is_statement_cancelled() before the fallback in all six branches and return the error, keeping the fallback for other replica failures. Add a routed-path test that forces a replica cancellation and asserts the writer isn't queried. If these reads should also be capped at 20s when they're routed to the writer first, apply the same tighten-only read deadline there.
Optional
deletion_transactions_lift_the_writer_statement_timeout(crates/buzz-db/src/store/deletion.rs:7391-7435) callslift_statement_timeoutdirectly and never callsinventory_schemaorpurge_postgres. Deleting the production call atdeletion.rs:2250leaves the wholebuzz-dbPostgres suite green. The runtime code is right, so this doesn't block. But that lift is what keeps a large-tenant purge from being cancelled and retried forever, so it should have a test that runs both production paths under a short writer cap and fails when either lift is removed. The PR description's test bullet for it should also say it exercises the helper.inventory_schemanow runs every row count in one read transaction (deletion.rs:1237-1255), so one snapshot is held on the writer for the whole inventory instead of one per count. That's worth a sentence in the description for large tenants.- Every generic read on the writer now pays
BEGIN,set_configandROLLBACK, which used to happen only for#e. That's three extra round trips per REQ history read routed to the writer. That's probably fine, but it's not mentioned anywhere. - The first commit's body says the purge raises itself to the 300s ceiling. The second commit replaced that with a lift to
0. If the squash keeps commit bodies, drop that line. - The description says every query is bounded except migrations, schema destruction and deletion. The audit wait and replica fallback above are the cases where that bound changes behavior. Worth a line once they're fixed.
- The description doesn't include
buzz-review-completed.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s GitHub account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/operator behavior) reviews.
Two low-severity P3 findings inline. These are non-blocking review comments; humans decide merge readiness. The mention bounds preserve timestamp/keyset semantics, and the read deadline preserves shorter caps and restores transaction-local settings.
Reviewed head 9cb6638b1ad9d28f2a0819a6c54f19d2e9ad5ed0 against base/merge-base dc4cdc4990877d400a435b481a416a73f17247bb. Source-only: no tests, SQL, app execution, query plans, load or latency measurements. One CI snapshot showed 56 successful and 28 skipped checks; it does not establish runtime correctness. The deletion timeout test exercises the helper directly, so it does not by itself protect the inventory/purge call sites. Public description and discussion inspected; no attached images to inspect.
Minimalism 9/10; elegance 9/10; correctness 8/10 due to the two concrete retry/lock-lifetime issues below. No approval or request-changes verdict is submitted.
|
@wpfleger96 thanks. Both blockers are fixed in
Optional notes:
|
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s GitHub account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/operator recovery) reviews.
No new actionable findings in this follow-up. The per-table inventory transactions resolve the earlier lock-lifetime finding. The revised audit worker retains and retries 57014 failures, the six replica paths propagate cancellation without a writer rerun, and the mention-kind bound matches the production writer. The regression tests now exercise the actual inventory/purge and audit-worker paths. The previously reported ACP timeout-retry debt remains explicitly deferred in the description; this review does not declare it fixed.
Operational tradeoff: SQLSTATE 57014 does not distinguish expensive work from a replica-local lock wait. Under the stated no-rerun policy, even a membership point lookup can therefore fail while the writer could answer. We retained that as policy context, not a new change request.
Reviewed head 03202cab3d4dc2ec4e9de8f79c91677fd8de5753 against base/merge-base dc4cdc4990877d400a435b481a416a73f17247bb, including the endpoint delta from previously reviewed 9cb6638b1ad9d28f2a0819a6c54f19d2e9ad5ed0. Source-only: no tests, SQL, EXPLAIN, application execution, or independent CI validation. Production latency, replica behavior, and historical out-of-band mention-row integrity remain unverified. Public description and discussion inspected; no attached images. Minimalism/elegance/correctness: 9/9/9 for the reviewed follow-up. This is a non-blocking COMMENT, not approval or merge authorization.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review of 03202cab3d4dc2ec4e9de8f79c91677fd8de5753 (base dc4cdc4990877d400a435b481a416a73f17247bb): no blocking issues. Both blockers from the last review are fixed, and so is the deletion test.
- Audit entries on a cancelled statement. The worker now retries
57014with the same backoff as55P03, and admission refusal, pool, constraint and hash errors stay terminal (is_retryable_append_failure,crates/buzz-relay/src/state.rs:2450).audit_worker_retries_statement_timeout_until_original_entry_is_appended_oncegoes through the real worker with a 100ms cap and lock timeout off, holds the audit-chain lock through a retry, and checks for exactly one row. Dropping the57014arm makes it fail with "audit worker never retried". - Replica cancels rerunning on the writer. All six reads now return the cancel and keep the writer fallback for other errors.
replica_statement_cancel_propagates_without_writer_rerunuses separate writer and replica databases, so a writer rerun returns rows and fails the test. Removing the guard inquery_feed_mentions_routed,get_thread_repliesoris_relay_memberfails it with that path named. - Deletion lift. Inventory now lifts and counts each table in its own transaction, so locks and snapshots are released per table.
deletion_inventory_and_purge_outlive_the_writer_statement_timeoutruns the realinventory_schemaandpurge_postgresbehind a heldeventslock past a 500ms cap. Removing either lift fails it with57014.
All new and changed tests passed by name at this head in the hosted unit job and Postgres job.
Optional
- Two replica fallbacks still rerun a cancelled read on the writer:
get_thread_window_with_session(crates/buzz-db/src/store/thread_window.rs:251-260) andReadSession::thread_window_aux(thread_window.rs:448-460, which also switches the session to the writer for good). Both predate this PR and both sides are already capped at 4s bySET LOCAL, so they don't bring back the 20s-to-120s escalation, and the description correctly scopes the fix to the six reads. A follow-up with the same guard and a test would make the rule hold for every routed read. event_mentions.event_kindis nullable (migrations/0001_initial_schema.sql:292), and thekindspushdown is the first reader that depends on it being set. The description already says the production NULL count wasn't checked. Worth runningSELECT count(*) FROM event_mentions WHERE event_kind IS NULLbefore merge.- The first commit's body still says the purge "raises itself to the 300s ceiling". The lift is to
0now, so drop that line if the squash keeps commit bodies. .env.example:73leaves# Generic eventalone on its line.- The description doesn't include
buzz-review-completed.
dc3e2b4 to
e9495a7
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s GitHub account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/operator behavior) reviews.
No new actionable code findings in this follow-up. The two thread-window cancellation guards propagate SQLSTATE 57014 without retrying on the writer. The HTTP bridge ends the request with a retryable 503 before emitting authoritative page bounds; the cancelled auxiliary transaction is dropped when that request ends. The new regression test exercises both production database paths. The earlier four feature patches are preserved across the rebase apart from upstream context; this review concentrates on the added guards, test and integration.
Optional description cleanup: remove the “Startup d_tag backfill” row and its “Identified debt” bullet. Upstream #8468 (c99872f), already in this base, removed that startup step. Those two statements now describe code absent from the reviewed tree. No code change is requested for this note.
The previously reported ACP timeout re-subscribe debt remains explicitly deferred; this review does not declare it fixed. The existing no-rerun policy still treats replica-local cancellations the same as expensive work.
Reviewed head e9495a7c478ad2b81758ca7169842efb616203e8 against base 93114b4279a619613a6df8ef740472216e1d4cfb, comparing the feature series with previously reviewed 03202cab3d4dc2ec4e9de8f79c91677fd8de5753. Source-only: no tests, SQL, apps, latency measurements or independent CI verification. Database cancellation, deployed client retry/focus behavior and production performance remain unverified. Public description/discussion inspected; no attached images. Minimalism/elegance/correctness: 9/9/9 for the follow-up. Non-blocking COMMENT only; no approval or merge authorization.
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review of e9495a7c478ad2b81758ca7169842efb616203e8 (base 93114b4279a619613a6df8ef740472216e1d4cfb): no blocking issues. Since the last review (03202cab), this head adds the thread-window cancel guards and rebases onto current main. The other four commits are patch-identical apart from one line of .env.example context from main.
- Thread-window cancels.
get_thread_window_with_session(crates/buzz-db/src/store/thread_window.rs:256-259) andReadSession::thread_window_aux(thread_window.rs:461) now return57014instead of rerunning on the writer, and the aux path keeps its replica session. Other replica errors still fall back.thread_window_statement_cancel_propagates_without_writer_reruncovers each guard on its own: with only the window guard removed it fails atruntime/tests.rs:3958, and with only the aux guard removed it fails at:3997. Both mutations were run against a disposable Postgres and failed on those assertions. The test also passed 3 of 3 runs alongside the two existing replica-cancel tests, so no flake turned up. - Rebase. Main added no routed replica read in
dc4cdc4990..93114b42. Every route that falls back to the writer still catches57014first, including the relay-membership read that moved (store/relay_members.rs:1229-1244). Main's writer changes (partition catch-all advance with its own 5s cap, the NIP-43 reconcile moved off startup, the NIP-43 list written in the membership transaction) add nothing that needs more than the 120s cap. Main only changed tests indeletion.rs, so inventory and purge still run entirely underlift_statement_timeout.
The full buzz-db Postgres suite passed locally at this head (566/566). Hosted CI is green, including PostgreSQL Tests and Security.
Optional
- The "Startup
d_tagbackfill" row in the long-running writer paths table and the matching "Identified debt" bullet describe code #8468 removed.backfill_d_tagsno longer exists at this head. .env.example:61is a 104-character line after the "0 disables the lock and idle timeouts" edit; the rest of the block wraps at 80.- The first commit's body still says the deletion transaction "raises itself to the 300s ceiling". The lift is to
0, so drop that line if the squash keeps commit bodies. - The description doesn't include
buzz-review-completed.
An agent #p + #h + since REQ ran for 45+ minutes on bb-public: the event_mentions side of the join was unbounded (every mention of the pubkey ever) and nothing cancelled it, because the writer statement_timeout defaulted to 0 and only #e reads had a deadline. - #p reads/counts join event_mentions on the full event key (created_at too) and repeat since/until on m.event_created_at, so the outer scan is a range on idx_event_mentions_pubkey_created. - Every generic event read and COUNT runs under the 20s transaction-local deadline that #e reads already had; read-replica sessions default to 20s. - Writer sessions default to statement_timeout 120s; 0 or values above 300s install 300s, so no writer session is unbounded. Migrations and schema destruction keep their own lifted session. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
…und test Community-deletion inventory counts and the purge transaction run whole-tenant statements that scale with the tenant. Under the always-on writer cap a large tenant's purge would be cancelled (57014, transient), retried from scratch, and eventually blocked. Like migrations and schema destruction these paths are operator-approved and fenced, so they now disable statement_timeout transaction-locally; lock and idle timeouts still bound them. The #p mention-bound test now puts since/until exactly on mentioned rows' seconds and adds a same-second sibling after the cursor id, so an off-by-one on the mention-side bounds fails it. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
…writer With the writer statement cap always on, two paths turned a 57014 into the wrong outcome: - The audit worker retried only lock_timeout (55P03). Under BUZZ_DB_LOCK_TIMEOUT_MS=0 (or >= the statement cap) the advisory-lock wait is cancelled by statement_timeout instead and the entry was dropped. 57014 is now retried with the same backoff; admission refusal stays terminal. - Six replica reads with their own SQL (feed mentions/needs-action/ activity, thread replies, channel window, relay membership) fell back to the writer on any replica error, so a 20s replica cancel re-ran the same expensive read on the writer. They now return the cancel, like the generic routed reads and ReadSession already do. Recovery conflicts still fall back. Community-deletion inventory now counts each table in its own lifted transaction, releasing each table's lock and snapshot as soon as its count finishes. The lift test now drives the real inventory_schema and purge_postgres under a 500ms writer cap behind a held events lock, so removing either lift call fails it. The #p mention join also repeats the kinds filter on event_mentions (every mention insert copies the event kind), letting the planner use idx_event_mentions_pubkey_kind_created. A clamped BUZZ_DB_STATEMENT_TIMEOUT_MS (0 or above the 5-minute maximum) now logs a warning at pool build. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Restore the SHOW statement_timeout = '0' oracle inside a lifted transaction so a bounded lift (e.g. '300000') goes red, and stop the audit retry warning from claiming every 57014 was a lock wait. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
get_thread_window_with_session and ReadSession::thread_window_aux re-ran a cancelled replica statement (57014) on the writer, and the aux path also degraded the session to the writer for the rest of the request. Both now return the cancel, matching the other routed reads; other replica failures still fall back. thread_window_statement_cancel_propagates_without_writer_rerun locks the replica's events table and cancels the parked window read, then times out an aux scan on a proved replica session. The writer copy is unlocked, so dropping either guard makes the test pass a writer result and fail. Also rewrap the .env.example statement_timeout comment. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
e9495a7 to
e004959
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 approving at e0049592. the #p bound and the always-on writer cap both look right to me. a few nonblocking notes:
verify_postgres_logically_deleted(store/deletion.rs) still runs under the 120s writer cap. it runsEXISTS … WHERE community_id = $1on every tenant table right after the purge, when thecommunity_idindex entries are still dead tuples the first scan has to visit, so on the largest tenants it could outrun 120s. retries get through eventually, but it's the same whole-tenant pattern inventory and purge exempt. could it calllift_statement_timeout(&mut tx)too?build_mentions_queryandbuild_needs_action_query(store/feed.rs) still joinevent_mentionson(community_id, event_id)only. they sort bym.event_created_atwith a LIMIT so they can't run away the same way, but each joined row still probes everyeventspartition. addingAND e.created_at = m.event_created_atwould give them the same pruning, one line each.- the replica cancel arms say a cancel "would be just as slow on the writer". that doesn't hold when the replica is waiting on a lock only it holds (a replayed
ACCESS EXCLUSIVEfrom primary DDL, which is also how the tests produce57014). then the user gets an error where the writer would have answered quickly. I'd keep the change since replica reads had no limit before, just worth watching thestatement_cancelledroute reason after rollout. - every REQ history read now holds its writer connection for three extra round trips, so worth keeping an eye on writer pool-acquire wait after deploy.
…eplica-cancel trade-off - verify_postgres_logically_deleted runs one whole-tenant EXISTS per table right after the purge; lift its statement timeout like inventory and purge, and extend the deletion lift test through it. - Feed mentions and needs-action join event_mentions on the event's created_at too, so each probe prunes to one events partition (same invariant as the #p read). - Replace the copied "would be just as slow on the writer" comments with one accurate rationale on DbError::is_statement_cancelled, including the replica-only lock case. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
f43696d to
0fbd059
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s GitHub account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/operator behavior) reviews.
No new actionable findings in this follow-up. The feed timestamp joins preserve results for supported mention writers while supplying the partition key. Logical-deletion verification now receives the same transaction-local timeout exemption as inventory and purge; catalog, lease and fence checks remain, and the regression test exercises the production verification method. The clarified replica-cancellation tradeoff matches the existing no-writer-rerun policy.
The previously reported ACP timeout re-subscribe debt remains explicitly deferred; this review does not declare it fixed. The exemption intentionally leaves statement execution unbounded in these operator-approved deletion transactions; configured lock and idle timeouts address their respective waits, not total execution time.
Reviewed head 0fbd0593b09a95212e93b9988936b7032e5d5fc8 against base d97f6b4c6ed616be0c3f1eb0e13d1eaa9e338a66 (merge-base 93114b4279a619613a6df8ef740472216e1d4cfb), concentrating on the actual endpoint delta from previously reviewed e9495a7c478ad2b81758ca7169842efb616203e8. Source-only: no tests, SQL/EXPLAIN, app execution, CI validation or performance measurements. Partition-pruning plans, production latency/lock behavior, historical out-of-band mention integrity and native error/retry focus remain unverified. Public description and discussion inspected; no attached images. Minimalism/elegance/correctness: 9/9/9 for this follow-up. Non-blocking COMMENT only; no approval or merge authorization.
…pshots * origin/main: fix(db): bound every statement and the #p mention scan (#8461) Signed-off-by: Daniel Rouleau <drouleau@squareup.com>
Problem
An agent
#p+#h+sinceREQ (therequire_mentionsubscription inbuzz-acp) ran for over 45 minutes on a production writer and heldAccessShareon everyeventspartition. There were two causes:#pjoin was unbounded.build_query_events_sqljoinedevent_mentionson(community_id, event_id)only. The planner picked a nested loop whose outer side was every mention of the pubkey, ever, and whose inner side range-scanned the channel index fromsinceon each partition. Cost ≈ total mentions × channel rows sincesince.statement_timeoutdefaulted to0. Only#ereads had a deadline (the 20s cap from 🤖 perf: bound long-thread aux reads and make query deadlines terminal #7854), and read-replica sessions had none at all.pg_stat_statementsshows this shape has a multi-thousand-second max going back months, so it is chronic and was not introduced by a recent change.Change
#pmention bound (store/event.rs,push_mention_join, shared by the page SELECT and COUNT)AND e.created_at = m.event_created_at. Mention rows copy the event'screated_atat insert (migration 0007 already relies on this equality), so the join can pruneeventspartitions by time.since/untilonm.event_created_at, so the outer scan is a range on the existingidx_event_mentions_pubkey_created.untilis a superset bound for the keyset cursor; the exact(created_at, id)cursor test stays one.kindsonm.event_kindwhen the filter has kinds. Every production mention writer binds the event kind (insert_mentionsinstore/runtime/mod.rs, and the replaceable-event path instore/replaceable.rs, back to the first Postgres schema), and no migration inserts mention rows, so this drops no rows written by the relay. Rows imported out of band with a NULLevent_kindwould no longer match a kinds-filtered#pread; a read-only production count on 2026-10-20 found 0 such rows on both production reader databases. The pushdown makesidx_event_mentions_pubkey_kind_createdavailable to the planner for therequire_mentionkind-9 shape; its use has not been observed (see below). An E2E fixture with a hand-set NULLevent_kindrow confirmed the caveat: base returns 8 events, head 7.store/feed.rs,build_mentions_query/build_needs_action_query) joinevent_mentionsthe same way, so each joined row probes oneeventspartition instead of all of them. TheirORDER BY m.event_created_at ... LIMITalready kept them from running away; this removes the per-row cross-partition probe.9cb6638b1, before thekindspushdown; 60,000 historical mentions + 2,500 recent decoys, stock planner,EXPLAIN (ANALYZE, BUFFERS)of the captured production SQL): the mention side is a bounded range onidx_event_mentions_pubkey_created(13 rows), then a hash join with a scan of the recenteventspartition. Base scanned 2,509 recent events and probedevent_mentions_pkey2,509 times. Head: 1.1 ms / 319 shared hits; base: 8.5 ms / 10,049 shared hits. Not measured against the production replica. A freshEXPLAIN (ANALYZE, BUFFERS)replay ata7065c367, after the pushdown, ran on a smaller fixture where the planner chose a sequential mention scan, so it neither confirms nor replaces these numbers.Reads: 20s deadline on every generic event read
query_events_onandcount_events_onalways run under the transaction-local 20s deadline that#ereads used (fetch_with_read_deadline, formerlyfetch_with_e_tag_deadline). It only tightens: shorter operator or caller caps are kept.after_connect, using the same tighten-only SQL.set_config+ ROLLBACK (three extra round trips; before, only#ereads did). In the local E2E the end-to-end history read was still faster at head (median 4.1 ms vs 10.2 ms at base, 9 samples each, warm cache), so the round trips are dominated by the bounded plan; this does not isolate them or establish production p99. The round trips also lengthen how long each such read holds its writer connection; that hold time has not been measured against production pool sizing.ACCESS EXCLUSIVEfrom primary DDL), when the writer would have answered quickly; that miss is the accepted price of keeping runaway reads off the primary. The rationale now lives once onDbError::is_statement_cancelledinstead of in a dozen copied comments. The generic routed reads andReadSessionalready did this; the eight routed reads with their own SQL (feed mentions, needs-action and activity; thread replies; channel window; relay membership; thread window and its auxiliary scan) now do too. A cancelled auxiliary scan also keeps its replica session instead of degrading it to the writer for the rest of the request. Other replica failures (e.g. hot-standby recovery conflicts) still fall back to the writer.SET LOCAL(artifact 2–5s, thread window 4s, personal reads, usage 5–15s, serving lease 1.5s) are unchanged.Writes: every writer session is bounded
statement_timeoutchanges from0to 120s (DEFAULT_STATEMENT_TIMEOUT_MS).BUZZ_DB_STATEMENT_TIMEOUT_MSstill overrides it.0or any value above 300s installs 300s (MAX_STATEMENT_TIMEOUT_MS), so a session can no longer disable it, and a clamped value logs awarn!when the pool is built. Only the operator-approved, fenced destruction paths below lift it, per transaction.Long-running writer paths, audited
with_exclusive_schema_destruction_lock)statement_timeoutbuzz-admin)inventory_schemawhole-tenant counts,purge_postgreswhole-communityDELETEper table in one tx,verify_postgres_logically_deletedwhole-tenantEXISTSper table right after the purge)statement_timeout = 0) transaction-locally bylift_statement_timeout, like migrations; lock and idle timeouts still bound them. Inventory counts each table in its own lifted transaction, so each table's lock and snapshot are released as soon as its count finishes, as the autocommit counts did. A cancelled purge would retry from scratch and eventually block, so a cap here would turn large-tenant deletion into an operator overridebuzz-audit, audit pool inherits the writer cap)BUZZ_DB_LOCK_TIMEOUT_MS=0(or at least the statement cap),statement_timeout(57014) cancels the wait instead oflock_timeout(55P03). The relay's audit worker now retries both with the same backoff instead of dropping the entry on 57014; admission refusal stays terminalOn HTTP routes a statement timeout already maps to the distinct retryable 503 (
api/mod.rs,statement_timeout_maps_to_distinct_503). A cancelled REQ fails its history read instead of hanging.Tests
p_tag_query_bounds_mention_side_by_time(unit): page and COUNT SQL carry thecreated_atjoin and thesince/until/kindsmention bounds, and the bounds are absent when the filter has none.p_tag_read_with_mention_bounds_keeps_results(Postgres): through the production mention insert, unbounded,since, window and keyset-cursor reads return exactly the mentioned rows, and COUNT matches. Every bound sits exactly on a mentioned row's second and the cursor page includes a same-second sibling after the cursor id, so an off-by-one on either mention-side bound fails it.plain_read_is_cancelled_at_deadline_when_timeout_disabled/count_events_cancels_at_read_deadline_when_timeout_disabled(Postgres): the production#p+#h+sinceshape and a plain COUNT are cancelled at 20s with the session timeout disabled. Previously these covered#eonly.writer_statement_timeout_is_always_bounded(unit) andsession_statement_timeouts_bound_writer_and_reader_sessions(Postgres): throughDb::new, writer sessions install2min/5min(for0and for 600s) /45s, and replica sessions install20s.deletion_whole_tenant_stages_outlive_the_writer_statement_timeout(Postgres): on a scratch desired-state database with a 500ms writer cap, the realinventory_schema,purge_postgresandverify_postgres_logically_deletedeach wait behind a heldACCESS EXCLUSIVElock oneventsfor over 1s and still complete, a plain statement is cancelled (57014), and pooled sessions still read500msafterwards. A lifted transaction readsstatement_timeout = 0, so a bounded lift (e.g.'300000') fails it, as does removing any of the threelift_statement_timeoutcalls.mentions_query_is_tenant_scoped_and_joins_mentions_by_composite_key/needs_action_query_is_tenant_scoped_and_joins_mentions_by_composite_key(unit): both feed queries carry thee.created_at = m.event_created_atjoin.replica_statement_cancel_propagates_without_writer_rerun(Postgres): with the replica locked past its cap, each of the six routed reads above returns 57014 instead of the writer's copy. Removing the cancel arm in any of them fails it.thread_window_statement_cancel_propagates_without_writer_rerun(Postgres): a thread-window read parked behind a replica lock and cancelled (57014; the window's own 1slock_timeoutwould otherwise fire first) returns the cancel instead of the unlocked writer's window, and an auxiliary scan timed out on a replica session returns 57014 and leaves the session on the replica. Removing either guard fails it.audit_worker_retries_statement_timeout_until_original_entry_is_appended_once(Postgres,buzz-relay): the production audit worker, withlock_timeoutdisabled and a 100ms statement cap, keeps the entry through repeated 57014 cancels while the chain lock is held and appends it exactly once after release. Treating only 55P03 as retryable fails it.5minfor a disabled statement timeout.After deploy, watch
buzz_db_route_decision{decision="replica",reason="statement_cancelled"}: replica reads that now fail instead of falling back to the writer, including the replica-only lock case above.buzz_db_pool_acquire_wait_secondsandbuzz_db_pool_waiterson the writer pool: every generic REQ history read and COUNT now holds its connection for three extra round trips.Database checklist (
docs/database.md§ Review checklist)This PR changes schema-dependent
buzz-dbqueries (the#pjoin and itsm.event_kindpushdown). It does not change the schema.event_kind: a kinds-filtered#pread no longer matches them (see thekindsbullet under Change). Relay writers always bindevent_kind, so only rows imported out of band are affected. A read-only production count on 2026-10-20 found 0 such rows on both production reader databases.idx_event_mentions_pubkey_createdandidx_event_mentions_pubkey_kind_created.statement_timeoutper transaction; migrations and schema destruction are unchanged (see "Long-running writer paths, audited"). No index builds and no operator prebuild step.What got simpler
#e-only branch inquery_events_onandcount_events_on.#pjoin SQL that was duplicated between SELECT and COUNT now lives in onepush_mention_join.Identified debt (not in this PR)
buzz-acpre-sends the same filter after a timeoutCLOSED(no backoff or narrowing). With the#pbound therequire_mentionread should finish well inside 20s, so this is a follow-up, not a blocker.