🤖 perf: bound long-thread aux reads and make query deadlines terminal

[wpfleger96]

Long threads (over ~100 replies) time out in desktop. The aux hop that loads reactions and other e-tag references builds its filter as one tags @> … term per id, joined with OR. Past about 100 ids the planner switches to walking each partition's primary key backward, expecting to stop early. When few rows actually match, it scans the whole community. With a dirty GIN pending list, a 188-reply thread never finishes.

What changes

• Set-based e-tag reads (buzz-db). The aux e-tag query now puts the id set behind a MATERIALIZED CTE and matches with ANY. Each e-tag read also runs under a transaction-local statement_timeout of 20 s.
• Terminal timeout contract (buzz-relay). When a read hits the deadline, the client gets a terminal timeout (CLOSED for REQ, the timeout error for COUNT and search), not a retryable failure.
• Subscription lifecycle (buzz-relay). The per-connection subscription map lock is now the lifecycle lock. Claims, replacement, client CLOSE, owner-checked timeout and search cleanup, revoke, and connection teardown all change the map, the registry, topic refcounts and terminal frames while holding it. As a result, a timed-out subscription's CLOSED goes out before any replacement with the same id can claim it, and claims made after teardown are refused.
• Dropped terminal frame disconnects (buzz-relay). If the CLOSED for a retired subscription (owner-checked timeout or channel eviction) can't be queued, the relay cancels the connection instead of leaving the client holding a subscription that will never receive fan-out. A client-initiated CLOSE ack is exempt.
• COUNT under the same deadline (buzz-db). The fast COUNT path runs e-tag counts under the same transaction-local 20 s cap as SELECT, so WS COUNT and HTTP /count produce the terminal timeout too.
• Replica reads surface the deadline (buzz-db). When a replica-routed read is cancelled by statement_timeout (SQLSTATE 57014), the error is returned instead of re-running the query on the writer, which would run the same slow statement. Other replica errors still fall back to the writer. PLANS/REPLICA_FULL_READ_ROUTING_DESIGN.md records the exception.
• Deadline handling (desktop). Only the relay's query timed out counts as a query deadline. A client-side request timed out stays retryable. For one-shot reads a deadline is terminal: the global query retry (budget 1), thread-replies retry and reaction hydration don't re-send the same slow query. Reaction ids held back by a deadline are released when the channel is opened again, so reopening retries them while same-channel re-renders don't. Live subscriptions, in both the renderer and the native archive-sync socket, keep treating a deadline CLOSED as transient: they back off and resubscribe with the same filter instead of being dropped.
• Deadline handling (mobile). Mobile recognizes the deadline on both transports: HTTP 503 {"error":"query timed out"} and the WS CLOSED reason.
  • The root ProviderScope retry policy returns no retry for deadline errors and keeps Riverpod's default backoff for everything else.
  • Background thread-summary recounts don't retry a deadline.
  • Forum posts polling pauses while the last load settled on a deadline; reopening the forum retries.
  • A channel's newest-window load surfaces a deadline instead of falling back to legacy WS history, and the channel list's batched queries return "unavailable" on a deadline instead of fanning out to per-filter fallbacks. An unavailable batch is not treated as empty. Known last-message timestamps and unread state are kept, and unread catch-up moves lastMessageAt forward (never backward) the same way live events do, so a cold start with no known timestamp still gets one.
  • A live subscription that gets a deadline CLOSED backs off and resubscribes with the same filter. History requests still fail on it.
  • Settled load failures show a Retry control, matching desktop's thread-replies error card: a shared LoadErrorView on the channel timeline, forum thread and forum posts list, and an icon button beside the thread replies summary. While the thread query has no value, the thread page shows the route snapshot plus live, cached and optimistic replies, with the loading or error status still visible.
  • Other re-sends (reconnect, foreground resume, live events, the unread backstop, scroll) are not suppressed; each is bounded by the relay's 20 s statement deadline.
• Long-thread seeder. just seed-long-thread [replies] posts one thread (default 187 replies) with some reactions to the local dev relay, for reproducing long threads in the apps. It uses a throwaway key, refuses non-loopback relays and ignores ambient BUZZ_* credentials. Writes are paced to fit the stock relay rate limit.

Lifecycle concurrency. The lock serializes one connection's subscription changes. Other connections can still be delayed by pressure on the shared pubsub command queue, and by serial eviction loops waiting behind a blocked connection. This PR does not claim strict isolation between connections.

Recovery vs. load. Clients stop automatic retries of the same one-shot read, but they don't remember deadlines across other triggers: reconnect, resume, live events and the unread backstop can each send the read again later, bounded by the 20 s statement deadline. A live subscription that hits the deadline resubscribes with its original filter under the existing 1–30 s backoff, and against a filter that keeps timing out it retries at the 30 s cap indefinitely, so the worst case is one bounded query per subscription every 30 s. since is deliberately not advanced to now on resubscribe, because that would skip events missed while the subscription was closed. Unchanged from main: when mobile's channel history sync fails with cached messages available, the cached messages are shown without the error or Retry control.

Benchmark

Local reproduction on an isolated bench stack (not committed):

• PG16, migrations applied, one 188-reply thread plus 5M background events in the same community.
• Each request replays the desktop thread-replies filter through the relay, with include_aux on. Times are total ms for 3 runs, with a 60 s client cap.

GIN pending-list state is controlled with VACUUM (clean) or by refilling the pending list (dirty), and recorded per partition with pgstatginindex. Dirty runs held 402 pending pages on every partition's tags index at the start and end of each matrix. Clean runs held 0–1 pages (relay startup writes).

build	GIN pending state	limit 10	50	100	200 (188 ids)
main	clean	26 / 16 / 13	56 / 51 / 52	92 / 93 / 95	662 / 639 / 641
main	dirty	315 / 298 / 327	22141 / 15888 / 16769	28992 / 27819 / 28080	timeout ×3 (>60 s)
this PR	clean	35 / 21 / 14	57 / 49 / 48	82 / 80 / 75	153 / 150 / 135
this PR	dirty	290 / 254 / 251	1257 / 1204 / 1206	2432 / 2378 / 2382	4333 / 4706 / 5104

Baseline origin/main b37e47721; this PR at 5f2650f5f, whose e-tag read builders are identical to the current head. At head 1ad81d868 against origin/main 15d44dcce, limit 200 measured 145–179 ms clean (main 683–715) and 4.41–4.51 s dirty (main timeout ×3).

Every completed cell returns the same signed event set: 209 events, 187 replies at limit 200. With include_aux off, every cell is ≤66 ms. The thread-window loader (select_aux) is unchanged from main, and its timings and captured plans match main's.

Actual aux-hop statements captured from each relay binary with auto_explain (analyze + buffers), execution ms:

build	clean, 100 ids	clean, 188 ids	dirty, 100 ids	dirty, 188 ids
main (N-way OR)	40	298	28785	62892
this PR (MATERIALIZED fence + ANY)	23	45	1992	3539

What the fence changes

• On main, the planner combines ORDER BY created_at … LIMIT with an overestimated match count and walks each partition's primary key backward, expecting to stop early. With few real matches it scans the whole community: the dirty 188-id statement touches ~4.9M shared buffers on main vs ~0.6M with the fence.
• The fence prevents that early-stop plan. Across the measured cases (1–188 ids, both pending states), the planner used the GIN tags index. The fence doesn't force GIN in general; a broad enough filter could still get a different plan.

Tradeoff: the fence materializes every match before sorting. In this bench small limits stayed within run-to-run noise of main. A single hot id (5,000 reactions, limit 100) measured 30–34 ms unfenced vs 35 ms fenced.

Deadline: each e-tag read (SELECT and COUNT) runs under a transaction-local statement_timeout of 20 s. It only tightens: a shorter operator timeout (BUZZ_DB_STATEMENT_TIMEOUT_MS) is kept. It's a per-statement limit, not a per-request one, so a request that runs several pages or hops can take longer than 20 s in total. The benchmark does not force the deadline.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested. Three P2 findings below prevent the promised terminal/bounded-read behavior. Merge criteria: stop mobile provider-level replay of canonical deadline errors, make failed terminal delivery disconnect rather than silently abandon a live subscription, and apply the e-tag statement cap to the fast COUNT path. Add production-owner regressions for those cases.

Reviewed HEAD 22dd186d935f853d287254b46cf4202d2b48c8a7 against BASE d01e5f82058463709a22e93bb4cd795da5f53e10, source-only. Existing Unit, Mobile, Desktop Core and PostgreSQL CI passed on merge 0ae3181a9540e1cc4becbc30cafdaaa06a7a1bde of that exact pair; no tests/builds/SQL were executed or CI rerun. The security-review check was cancelled. Earlier benchmark results do not establish performance for this final head.

Scope notes, not additional blockers: successful stale same-ID output and FTS-stage cancellation behavior are inherited; cross-connection isolation and a total-request deadline are not promised. Old clients need upgrading to consume the new terminal reason.

addressed

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no remaining blockers in this re-review. Head 115b1a933ddb9280dc55900207772be5518943e1, pinned base 8f6b66f9ff6b2e42cd713506805763269375b473.

All three previously reported blockers are addressed: mobile's production root retry policy suppresses canonical deadlines; failed required CLOSED delivery cancels the connection; and fast e-tag COUNT uses SELECT's transaction-local min(operator timeout, 20 seconds) cap. The changed mobile request owners preserve terminal state across automatic lifecycle triggers and expose explicit retry paths.

Validation was source-only, including independent DB, relay-lifecycle and mobile review plus integration checks. Tests were inspected, not executed; runtime performance and live recovery were not reproduced. This is a COMMENT review, not approval.

Nonblocking follow-up: add cold-start coverage for unavailable latest-message timestamps followed by successful unread catch-up, and an assertion covering the production root retry wiring. The cold-start seed/mark-read behavior is inherited when the previous fallback also failed; it is not a verified new blocker, and automatic fallback must not be restored.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No new actionable defects found in this bounded follow-up. Reviewed head 538e73e92d8f8c56ac809de2602a8412778865fc against base 676e8c43825fa478850a3a320b1367d3288cce1a, carrying forward the previous clear review.

• Reviewed the new thread-window aux fence/shared predicate, explicit mobile Retry across channel/thread/forum surfaces, local seeder, and necessary merge integration. Prior terminal-deadline fixes remain intact; the declared materialization tradeoff is unchanged.
• Source-only review with independent mobile review on the pinned Blox host. No PR code, tests, seeder, or benchmark executed. The new Retry widget tests stub production providers; provider binding and optimistic-reply preservation were source-traced, not integration-tested here. Existing PostgreSQL, Mobile and relay-integration CI passed on synthetic merge 9ff975d04006bd701a85f4963d3cde2914776447 of this exact head/base.
• CI remains a merge gate: desktop smoke shard 4 repeatedly failed scroll-history.spec.ts:2205 (oldest index stayed 550). This mock-bridge test does not exercise the new SQL/mobile paths; no causal regression was established in this follow-up. Resolve or explicitly disposition that failure before merging. This comment is not an approval or a green-CI claim.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no new actionable blockers in this follow-up. HEAD 719935392da73f8861efdd74a86940ad3f6d16ee, BASE 6410e685a80d42db0645fadc9bbe710559ef911e. Carries forward the previous clear review, with fresh review of the six client follow-ups and base-merge integration.

• Finite reads still stop on the canonical relay deadline without immediate provider retry or equivalent fallback. Client-side timeouts remain retryable. Persistent subscriptions instead recover with existing 1–30s backoff; finite ownership is handled first, and cancellation/replacement fences remain intact. Desktop reaction reopen recovery binds the production hook. The base merge preserves NIP-FI/NIP-98 admission, DB-timeout responses, and both test selections.
• Non-blocking contract/documentation follow-up: update the PR body, which still describes the removed mobile registry. Stateless guards permit bounded later refreshes from live events, reconnect/resume and the unread backstop. Live recovery reuses the original filter, not universally a fresh since: now filter; it can retry persistently at the backoff cap. Correct that comment and document the recovery/load tradeoff. Do not blindly advance since to now, which could skip missed events. Also retain the existing mobile cached-refresh visibility gap as a follow-up: _init can return AsyncData(cached) after failed history sync, hiding the error/Retry. That fallback is inherited from base, not a new defect requiring the removed registry.
• Source-only review on the pinned Blox host, including independent renderer, mobile-provider and native/session lanes. Existing CI reports success for this exact head/base, including Mobile, PostgreSQL, relay integration and all desktop smoke shards. The separate Codex Security Review was cancelled. No PR code, tests, builds or benchmarks executed; no CI reruns. Runtime recovery and performance were not independently reproduced. No accidental media/generated artifacts found in the cumulative diff.

This is a COMMENT review, not approval.