Skip to content

Unblock the server event loop: timeline build coalescing, deferred pruning, websocket backpressure - #2387

Closed
vburojevic wants to merge 5 commits into
get-bb:mainfrom
vburojevic:bb/mobile-perf2/server-event-loop
Closed

Unblock the server event loop: timeline build coalescing, deferred pruning, websocket backpressure#2387
vburojevic wants to merge 5 commits into
get-bb:mainfrom
vburojevic:bb/mobile-perf2/server-event-loop

Conversation

@vburojevic

Copy link
Copy Markdown
Contributor

What was wrong

The server's event loop showed p95 578ms / max 1.18s, and every stall delays every phone RPC and websocket flush (better-sqlite3 is synchronous; plugins run in-process). Four verified mechanisms: (1) timeline rebuilds didn't coalesce across clients — the response cache is cold on every appended event and refuses >200-row (streaming) windows, so desktop+browser+phone+plugin windows each triggered an independent synchronous 130–260ms rebuild that serialized on the loop; (2) maybePruneActiveThreadEventHistory ran inline in POST /internal/session/events every ~30s per active thread, unattributable to the stall monitor, with two DELETEs walking delta rows through correlated subqueries even when nothing was prunable; (3) the app websocket broadcast paths looped bare socket.send — no bufferedAmount check, no queue cap, no try/catch — so a suspended phone grew unbounded buffers and one throwing socket truncated fan-outs; (4) bb.realtime.publish triple-serialized with no rate/size bound, bb.log did mkdir+stat+append per line on the loop, and thread lists ran unbounded queries.

What changed

  • Timeline cache: a per-params-key last-build slot shares one build across same-key requests inside a 250ms window regardless of row count, and floors rebuilds of over-LRU-cap windows (the streaming shape) to one per 250ms; LRU-cacheable windows keep eager rebuilds and status flips are never floored. The route records latest-rows under the response's own maxSeq so floored responses stay delta-coherent.
  • The active-thread prune moved into the existing post-response follow-up batch, wrapped in runEventLoopWorkSync for stall attribution, with a LIMIT-1 covering-index probe per prune class so the no-op case skips the correlated-subquery DELETEs.
  • The terminal socket queue/drain/drop machinery is generalized into per-lane sendWithBackpressure; all five app notify paths route through a 1 MiB high-water / 4 MiB budget lane with try/caught sends and drop-and-close (1013) for wedged or throwing sockets (clients reconnect and catch up via the watermark). Terminal lane semantics unchanged.
  • bb.realtime.publish serializes exactly once, rejects payloads over 64KB, and drops publishes over a per-plugin 10/s (burst 20) token bucket with a coalesced warning. bb.log batches lines into one async append per 200ms/8KB with a cached rotation counter; the plugin handle's dispose hooks flush the writer.
  • Thread lists default to a 200-row cap in both list functions; GET /threads reports truncation via an x-bb-thread-list-has-more header. Known deliberate behavior change: bb thread list and unpaged SDK lists now return at most 200 rows; in-repo programmatic consumers already pass explicit limits.

No daemon-facing payload shapes changed (no HOST_DAEMON_PROTOCOL_VERSION bump); the plugin-signal wire frame is byte-compatible.

How you verified

New tests proven fail-before/pass-after per step: one build shared across concurrent same-key requests (builder spy); floored request returns the prior window, never floored below the row cap or across params keys; prune runs post-response with identical timeline rows read between append and deferred prune; queue-then-drain above high water, drop-at-cap while siblings receive, throwing-socket isolation; publish rate-limit and 64KB rejection through a real installed plugin; log batching with no sync writes on the call path; db default-cap (201→200) and has-more header tests. Full @bb/server (2,033 tests after rebase) and @bb/db (423) suites green via turbo; typecheck clean. Rebased onto current origin/main (clean; verified by merge-tree). Remaining operational verification: the 24h bb perf issues event-loop-lag soak post-deploy.

AGENT GENERATED

vburojevic and others added 5 commits August 25, 2026 09:12
The timeline response cache is keyed by maxSeq, so every appended event
makes it cold, and it deliberately refuses to store windows over 200 rows
(the streaming case). With several clients watching one streaming thread,
each self-paced refetch triggered its own synchronous 130-260ms rebuild,
and the rebuilds serialized on the event loop under every other request.

Add a per-params-key last-build slot to the timeline cache: inside a
250ms share window, requests at the same key share the one completed
build whatever its row count, and requests at a newer maxSeq for an
over-cap window get the prior window back (a rebuild floor). The floor
deliberately skips LRU-cacheable windows - those already coalesce at the
same key via the LRU, and their rebuilds stay eager - and never crosses
params keys, so a status flip (interrupt, completion) is never floored.

The route now records the latest-rows snapshot under the response's own
maxSeq rather than the route-computed one, so a floored response still
names exactly the revision its rows reflect and delta bookkeeping stays
coherent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
maybePruneActiveThreadEventHistory ran synchronously inside
POST /internal/session/events - the hottest handler - every ~30s per
active thread, and it was not wrapped in event-loop-work, so the stall
monitor could not name it when it exceeded a second. Two of its DELETEs
also walk their candidate rows with correlated subqueries even when the
thread has nothing prunable, which is the common case.

Move the prune into the existing deferEventFollowUpBatch as an
"active-thread-event-prune" follow-up, wrapped in runEventLoopWorkSync
so stalls are attributable to `event-prune <threadId>`. Pruning is
output-preserving and never lowers maxSeq, so deferral only changes when
redundant rows disappear, never what any read returns - the updated
ingest test pins both the post-response ordering and that a timeline
read between append and deferred prune projects identical rows.

Cheapen the no-op case with a LIMIT 1 necessary-condition probe per
walking prune class (usage CTEs, resolved item deltas, background-task
progress): one covering (thread_id, type, sequence) index seek decides
whether the DELETE can possibly match before it walks anything. The
plain range-delete class needs no probe - it already is one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chinery

The app broadcast paths (notifyClientsByKeySet, notifyThreadListOnlySockets,
notifyThreadOpen, notifyThreadPaneAction, notifyPluginSignal) looped bare
socket.send: no bufferedAmount check, no queue cap, and no try/catch, so a
suspended phone's socket buffer grew without bound into server RSS and one
throwing socket truncated delivery to every subscriber after it.

Generalize the terminal queue/drain/drop machinery into per-lane
sendWithBackpressure: send below the socket high water, queue and
poll-drain above it, and drop-and-close the socket (1013) when a send
throws or the queue budget overflows. The terminal lane keeps its
1 MiB / 32 MiB limits and drop semantics verbatim; the new app lane uses
1 MiB / 4 MiB - app messages are small invalidation hints, and a socket
that cannot flush them is wedged. Dropping an app socket unregisters it
first so the rest of the fan-out and all later ones skip it; clients
already reconnect and catch up via the watermark. Broadcast counts now
report sockets that accepted (sent or queued) the signal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bb.realtime.publish serialized every payload three times on the event
loop (stringify to check serializability, parse back, zod-parse the
envelope, stringify the frame), had no size bound, and no rate bound -
a runaway in-process publisher multiplied by every connected client.
bb.log did mkdirSync + statSync + appendFileSync per line on the loop.

publish now serializes the payload exactly once; that string is the
size-cap measurement (64KB, rejected with a clear error) and the exact
bytes on the wire - the hub validates the envelope fields through the
strict outgoing schema and embeds the payload verbatim instead of
re-parsing it. A per-plugin token bucket (10/s, burst 20) drops signals
over the limit - they are ephemeral by contract - warning once per 10s
window with the plugin id.

bb.log now buffers lines and flushes them as one async appendFile per
200ms window or 8KB, creates the log directory once per writer, and
tracks rotation from a cached size counter seeded by a single stat. The
tail path flushes before reading so it stays read-your-writes, and the
plugin API handle flushes and drops the writer in its dispose hooks so
a pending flush timer cannot race a plugin data-dir removal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /threads and the projects-with-threads bootstrap ran unbounded list
queries: the route accepted an optional limit nobody passed, and the
per-projects variant had no limit parameter at all, so every sidebar
bootstrap and list read scaled with a long-lived install's full thread
history.

listThreadsWithPendingInteractionState now defaults its limit to
DEFAULT_THREAD_LIST_LIMIT (200) and the ForProjects variant gains the
same defaulted limit across all listed projects; both order by the
active-sidebar order, so the cap keeps the most relevant rows. The
GET /threads route fetches one probe row past the effective limit and
reports truncation in an x-bb-thread-list-has-more response header -
the response body stays a bare array for compatibility - so clients can
page with limit/offset once they hit the cap.

Known behavior change, deliberate: a `bb thread list` or unpaged SDK
list call now returns at most 200 rows (the two in-repo programmatic
consumers already pass explicit limits and page).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SawyerHood

Copy link
Copy Markdown
Collaborator

Closing this PR for rework after an adversarial review of the #2385#2393 set. Method: two independent multi-agent review passes over a worktree at this PR head; every finding went to three independent refuters (code trace, a throwaway experiment against the real code, and an impact judge), and only findings that survived at least two of three are listed. The branch stays as-is; please reopen this PR or open a new one when the blocking items are addressed.

Blocking

  1. Sidebar bootstrap is silently capped at 200 live threads across all projectspackages/db/src/data/threads.ts:1277. listThreadsWithPendingInteractionStateForProjects now applies DEFAULT_THREAD_LIST_LIMIT to a query ordered by projectId first, and its only production caller (buildProjectsWithThreadsResponseFromRows, behind GET /projects/sidebar-bootstrap and GET /projects?include=threads) passes no limit. With project A (201 active threads) and project B (5), the bootstrap returns 200 rows for A and 0 for B; B renders empty in the desktop and mobile sidebars, and useChildThreads loses children the same way. Reproduced with the real query on in-memory SQLite (3 projects × 150 threads → 150 / 50 / 0). The body's claim that in-repo programmatic consumers already pass explicit limits is false for this call site. Fix: no silent default inside the db layer; make limit explicit and fill the 200 default once in GET /threads (AGENTS.md: fill defaults once at the boundary); the sidebar passes what it needs, or caps per project with an explicit hasMore.
  2. The timeline rebuild floor serves a stale window with no catch-upapps/server/src/services/threads/timeline-cache.ts:122. For >200-row (streaming) windows, a request with a newer maxSeq inside the 250 ms window gets the previous build. Clients refetch only on the events-appended hint plus one trailing refetch and never poll, so when the last event before a pause (for example item/started of a long tool call) lands during an in-flight fetch, every client misses it until the next append, which can be minutes. Fix: keep same-key sharing but drop the cross-revision floor, or arm a one-shot trailing rebuild for the rest of the window (or emit events-appended when full.maxSeq < maxSeq).
  3. The bb.realtime.publish token bucket tail-drops terminal signalsapps/server/src/services/plugins/plugin-api.ts:948. Twelve script automations on one cron minute publish 24 frames synchronously inside sweepDueAutomations; the bucket admits 20; the finally publishes ~400 ms later find ~5 tokens. The automations UI has no poll or reconnect fallback, so finished runs stay "running". Fix: coalesce latest-per-(plugin, channel) and flush when a token frees instead of dropping, or exempt terminal signals. Document the 64 KB rejection and the rate limit in the PluginRealtime.publish JSDoc (packages/plugin-sdk/src/backend-contract.ts:229), the bb-plugin-authoring skill, and the SDK changelog; today every plugin-facing surface still promises unconditional broadcast.
  4. The 200-row GET /threads cap is invisible to the SDK and CLIapps/cli/src/commands/thread/list.ts:51. The only signal is the x-bb-thread-list-has-more header: sdk.threads.list discards it, bb thread list has no --limit/--offset and prints no notice, CORS does not expose the header, and the bb-cli skill and guide do not mention the cap. An agent running bb thread list --json on an install with 250 threads cannot find or reach the missing 50. The same truncation hits the "Assign parent thread" dropdown (apps/app/src/hooks/queries/thread-queries.ts:436) and the mobile home list. Per AGENTS.md, a behavior change must ship its SDK and CLI surfaces in the same change.

Also found

  • apps/server/test/services/plugins/plugin-log.test.ts:58 races appendFile open vs write and read an empty file under load (it failed once in our runs).
  • apps/server/test/services/plugins/plugin-wire.test.ts:708 depends on wall clock: a ≥100 ms stall mid-loop refills a token and yields 21 frames. Inject a clock.
  • plugin-api.ts:898: the refill does not clamp negative deltas, so a backward clock step (NTP) drops every publish until the deficit refills. Use a monotonic clock.
  • On the relayed path (bb connect), socket.raw.bufferedAmount measures a loopback socket that always drains, so the new lane never trips for a relayed phone; the bytes accumulate in the tunnel instead. That is pre-existing, but the body's "suspended phone" framing only holds for LAN clients.

What was clean

The deferred active-thread prune, the bb.log batching, and the backpressure lane on LAN had no findings.

Suggested salvage

Split it: land prune + log batching + backpressure now. Then fix the cap (explicit limits, hasMore in the SDK response, --limit/--offset or --all in the CLI, docs), the floor (trailing rebuild), and the bucket (coalesce), and reopen.

AGENT GENERATED

@SawyerHood SawyerHood closed this Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants