perf(streams,tasks,chat): cut storage row writes to legacy parity on the streaming hot path - #2191
Merged
Merged
Conversation
…the streaming hot path Streams' append fence becomes a read: a Durable Object executes one synchronous block at a time, so state-check + chunk-tail read + INSERT is exactly as atomic as the old guarded count-bump UPDATE — and one row write per append instead of two. The stream row is written only at open and settle; settlement stamps the final cursor, and live cursors/liveness derive from the chunk log through one #tail helper. Reader loops use narrow state reads and terminate on an empty poll observed terminal in the same synchronous block. The chat adapter's retention sweep decides abandonment in two phases (coarse row cutoff, then one indexed chunk-tail read per candidate), the legacy migration imports rows complete (final count and last activity up front; chunk imports are bare INSERTs — 1+N writes instead of 1+2N), destroy() no longer flushes chunks it deletes in the same call, the cleanup alarm scans the table once, and the write-only _segmentIndex field is gone. Tasks amortizes claim refreshes to one write per half claim-slack of wall time instead of one per step, journals already-elapsed sleeps born-completed in a single INSERT, skips startup job-queue upserts that already match the run's deadline, settles a parked-run cancel in one row write, dedupes identical status writes, and only re-syncs the wake mirror when its settle write actually landed. The in-suite benchmark now pins exact adapter/legacy write parity (240 vs 240 rows for 20 turns x 100 chunks; previously 440) and models the two-phase sweep (239 -> 42 rows read).
🦋 Changeset detectedLatest commit: 3783b30 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
…THOUT ROWID tables; bound replay memory Cloudflare bills rowsWritten, which counts index maintenance — and an ordinary rowid table's PRIMARY KEY is a hidden UNIQUE index, so every chunk append billed 2 rows while total_changes() (the parity benchmark's metric) reported 1. Measured empirically in workerd and pinned by a new write-accounting test: rowid composite-PK insert = 2, WITHOUT ROWID = 1, each touched explicit index +1, untouched indexes free. All five capability tables (streams, stream chunks, task runs, task steps, jobs — none released) go WITHOUT ROWID; the aperture's rowid ordering tiebreaks become stream_id. The task runs table drops its (state, next_at) index — a billed tax on every claim, refresh, and settle, paid only to speed the startup reconcile's one scan of a retention-bounded table. A 100-chunk chat turn now bills 13 rows vs 33 for the legacy schema (and 34 for the capability shape this PR started from). Replay memory is bounded too: the chat adapter's chunk replay becomes a generator over paged reads (readChunks replaces the aperture's readAll), so a reconnecting client holds one page of segments instead of the whole stored turn.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The snapshot pins the verbatim sqlite_master text; the WITHOUT ROWID change altered three tables' stored DDL. The templates now end exactly at the ROWID keyword so the stored text stays clean of trailing whitespace.
…inistic newest-first ordering Review flagged that WITHOUT ROWID on cf_agents_streams dropped the rowid insertion-order tiebreak: same-tag rows sharing a created_at millisecond (a retried turn's shape) would order by random nanoid, so recovery's latest-row lookups could pick an older turn. rowid IS the right tiebreak, and its hidden-index cost lands once per stream open — per turn, never per chunk — so the metadata table stays a rowid table while the chunk log keeps the WITHOUT ROWID billing win. The aperture queries get their rowid tiebreak back, a new test pins three same-tag same-created_at rows to insertion order, and the write-accounting test now documents the deliberate 3-billed-row stream open.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2173: an audit of every storage row write on the Streams/Tasks/chat paths. Two rounds: first cutting table-row writes to exact parity with the pre-capability chat pattern, then measuring the billed metric (
rowsWritten, which counts index maintenance —total_changes()doesn't) and restructuring the DDL around it. Behavior is unchanged — all 3,509 tests across agents/ai-chat/think pass (test edits are tighter assertions plus one new accounting test).On DO SQLite, rows written cost ~1000× rows read, so every trade converts writes into (few) reads. Explicit transactions don't reduce billed writes — each synchronous block is already one atomic commit; the billed unit is the row write itself.
Round 1: the append fence becomes a read
The old fence was a guarded count-bump
UPDATEon the stream row — one extra row write per append buying settled-write rejection, the cursor, andupdated_at. All three are derivable:#append's state-check + chunk-tail read +INSERTis exactly as atomic as the guarded UPDATE (the invariant is stamped on the method: nothing between the fence read and the INSERT may await or run user code; serialization runs first becausetoJSON()can re-enter).#tailhelper (writer cursor,status(),list(), sweep verification).readBatchesterminates on an empty poll observed terminal in the same synchronous block, and its liveness checks read one column, notSELECT *including metadata.Chat adapter: two-phase retention sweep (coarse
updated_atcutoff, then one indexed chunk-tail read per candidate — an actively appending stream is never swept, a quiet sweep reads zero chunk rows); migration imports rows complete (1+N writes instead of 1+2N, and imported terminal rows carry their exact cursor — pinned by the migration test);destroy()no longer flushes chunks it deletes in the same call; the cleanup alarm re-arms off the sweep's survivor count instead of a second table scan; the write-only_segmentIndexfield is deleted.Tasks: claim refreshes amortize to one run-row write per 15s (half the claim slack) of accumulated step time instead of one per step — a policy-respecting step still can't outlive its claim; already-elapsed sleeps journal born-completed in one INSERT; startup reconcile skips job-queue upserts that already match (an unchanged upsert is still a billed write), with one
rearm()for a lost alarm; parked-run cancel settles in one row write; identicalstatus()messages skip their write; settle paths only re-sync the wake mirror when their fenced write actually landed.Round 2: bill one row per hot-path write
Measured empirically in workerd and pinned by the new
write-accounting-probetest: an ordinary rowid table's PRIMARY KEY is a hidden UNIQUE index, so its INSERTs bill 2 rows;WITHOUT ROWIDmakes the PK the table and bills 1; each touched explicit index adds one; untouched indexes are free.WITHOUT ROWID. A chunk append bills exactly one row, asserted per-statement.(state, next_at)index — it taxed every claim, refresh, and settle write to speed one startup scan of a retention-bounded table. Thedefinitionindex stays (never touched by run updates; list-by-definition scales with retained runs).rowidordering tiebreaks becamestream_id, matching publiclist().Memory
readChunksreplaces the aperture's unboundedreadAll): a reconnecting client holds one page of segments, not the whole stored turn.Accounting (in-suite, real DO SQLite, 20 turns × 100 chunks)
toBerowsWritten)So on the billed metric the streaming path is now ~2.6× cheaper than what merged in #2173 and ~2.5× cheaper than the original pre-streams chat.
Declined / deferred (from the same audit)
created_atforlist(), partialstateindex): each costs +1 billed row per insert/delete to save reads — the wrong trade on this axis.readBatchesreturning terminal status (saves a per-connection read insseResponse): public API change, per-connection cost only.Verification: agents 1967/1967, ai-chat 655/655, think 887/887 (downstream suites against rebuilt dists), typecheck 117 projects, sherif/exports/oxfmt/oxlint clean.
design/rfc-streams.mdrecords both the fence/authority move and the billed-write model in its evolution section.