Skip to content

Commit a69f416

Browse files
waleedlatif1claude
andauthored
fix(redis): bound the three unbudgeted stream writers by bytes (#7568)
* fix(redis): bound the three unbudgeted stream writers by bytes The copilot stream buffer, the Tables event log and the realtime file-doc streams were each bounded by entry count and nothing else. An entry cap bounds how many entries a key holds and says nothing about how large each one is, so a single writer emitting large entries reaches gigabytes well inside its cap — which is how one file-edit stream filled a shared Redis under a 100,000-entry cap and evicted the whole keyspace. Each writer gets the bound its read semantics allow: - Copilot's replay buffer is read from a cursor and must stay contiguous, so it now reserves bytes against per-stream and per-user ceilings inside the same Lua that appends, and the writer soft-stops persistence on refusal rather than failing the live stream. - The Tables event log is a live feed whose readers already refetch on a prune, so it drops oldest-first once past a byte ceiling — the existing `pruned` path carries it, with the running total kept in meta under the same TTL as the bytes it counts. - The file-doc streams are Yjs deltas replayed in full by a task attaching later, so dropping the oldest would lose edits and a native MAXLEN bound is unsafe. Compaction, which folds deltas into a snapshot first, is lossless — it now triggers on appended bytes as well as entry count. Also folds `lib/execution/redis-budget.server.ts` into the shared module rather than leaving two definitions of the same prefix and ceilings writing the same Redis keys. Key layout and every execution limit are unchanged, and pinned by test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(realtime): count only deltas toward the compaction byte threshold Re-seeding the counter with the snapshot's own size left any document larger than the ceiling permanently over it, forcing a full snapshot append on every subsequent keystroke — the write amplification the threshold exists to prevent. The counter measures edit churn since the last fold, so a stream settles at one snapshot plus that much churn. Also self-corrects the Tables byte counter whenever its buffer trims to a single entry, so an independently evicted events key cannot leave the accumulator over-reporting and pin the buffer at one entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(redis): address review findings on the byte bounds - Measure ceilings in UTF-8 bytes on both the copilot and Tables paths, so the TypeScript checks bound a stream the same way the Lua's `string.len` does rather than under-reporting every non-ASCII frame. - Split an oversized copilot batch on the per-write ceiling instead of refusing it. A flush carries whatever accumulated since the last one, so a run of large frames can exceed the ceiling collectively while each frame is individually writable; refusing that stopped replay for the rest of the stream over a batching artefact. A single frame past the ceiling is still refused. - Re-check the copilot soft stop when an in-flight append resolves, not only at enqueue, so a batch queued behind a refusal cannot land and leave replay holding later events but not the refused ones. - Deduct rather than zero the file-doc compaction counter, and only once the trim succeeds, so a failed fold leaves the trigger armed and a concurrent publish's bytes survive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(redis): account for deltas a fold retains, and release cleared counters The compaction counter was a single total, so a fold deducted bytes for entries `XTRIM MINID` had retained — anything published past the fold boundary the tailer had not yet integrated. Those bytes are still in Redis, so the trigger disarmed while the stream kept growing. Deltas are now tracked as `{id, bytes}` and dropped only once a trim provably removed them. Arming the trigger on retained bytes would be the opposite fault: a fold that reclaims nothing would re-arm immediately and force a full snapshot append per publish. Only bytes at or before the fold boundary arm it, and because that boundary moves in the tailer rather than on publish, the tailer re-checks it — otherwise a burst of large edits followed by silence would sit unfolded until the next keystroke. Also releases the copilot owner counter when the buffer is cleared, crediting the user counter by exactly what the owner held. Those keys are deleted rather than expired, so the counter otherwise outlived its data and a retry reusing the streamId would be refused against bytes that no longer exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(redis): make buffer cleanup atomic and match MINID's inclusive boundary Deleting a copilot buffer and releasing its reservation were two round trips, so a concurrent append landing between them kept its events stored with its reservation already erased. Both now run in one script, composed from a rendered release fragment the same way the reservation is. `XTRIM MINID upTo` is inclusive — it keeps the entry whose id equals the boundary. Accounting treated that entry as folded, so its bytes stopped counting while they were still in Redis, and a large paste landing exactly on the boundary could leave the trigger disarmed. Both directions now use the same strict/inclusive split: only entries strictly before the boundary arm the trigger, and only those are dropped once a trim removes them. Replaces the tests' `any` casts with a typed accessor, per the repository's TypeScript rule. This immediately caught injected test rooms missing `pendingDeltas`, which made compaction throw into its catch while the assertions still passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(redis): never credit the shared user counter from a buffer delete An owner id is not proof of who wrote the bytes, so crediting the user counter on clear let anyone able to name a stream decrement a ceiling they never charged. That is the one direction that must not be possible: a counter driven down grants writes rather than denying them. The clear now drops the owner counter only. The user counter's fixed window settles it instead — it already tolerates accruing bytes Redis has dropped, and this is the same over-count bounded by the same window. The scope threading that existed only to credit it is removed with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(redis): keep counters outliving their data, and stop a failed fold retrying hot The copilot owner counter used a fixed one-hour window while the stream TTL is configurable and defaults to exactly that. Raising COPILOT_STREAM_TTL_SECONDS would have let the counter expire under live data, and the next write would see zero reserved and grant another full ceiling. The window is now the larger of the two, so a counter can never expire before what it accounts for. A failed fold deliberately leaves the trigger armed, but the snapshot XADD lands before the XTRIM — so a persistent trim failure retried immediately, appending a full-document snapshot every time and turning a Redis blip into the write amplification the threshold exists to prevent. A forced fold now waits out a cooldown after a failure. The entry-count path is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(realtime): adopt byte accounting for a stream taken over A room attaching to an existing stream started from an empty ledger, so a multi-megabyte stream under the entry threshold stayed unfolded while that room's own heartbeat kept refreshing its TTL — a restart or handoff could hold one open indefinitely. `catchUp` already reads every entry to rebuild the doc, so adopting their bytes costs no extra work. Plain deltas only: a compaction snapshot is the result of a fold rather than something a fold can reclaim, so counting one would arm the trigger against itself. The trigger is re-checked once, after catch-up, since nothing else re-checks until the next local publish and a read-only participant never makes one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(realtime): mark a fold's output explicitly, and account in the tailer A fold of an agent-only stream is stamped with the agent marker to preserve the no-persist guarantee, which made it indistinguishable from an ordinary agent preview frame. Excluding that marker from accounting therefore dropped preview deltas — the largest payloads there are, and the ones that caused the incident — while including it would let a snapshot arm the trigger against its own output. A dedicated field settles it without touching origin selection. With the ambiguity gone, accounting moves from the publish path to `applyEntry`, which observes every entry the room tails: this task's appends, a peer task's, and one published with no room attached anywhere. Publish-side accounting could only ever see local writes, and contributed nothing to the trigger before the tailer caught up regardless, since only entries at or before the fold boundary arm it. The ledger is now a Map keyed by entry id, so an entry observed twice is recorded once. Entries written before this field carry no marker and count as deltas, which over-arms by at most one fold that then trims them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(redis): drop the clear-buffer script and the unused error class A variadic DEL is already a single atomic command, so the Lua script and the render function that built it achieved nothing a plain `del(events, seq, abort, ownerBudget)` does not. The keys carry no hash tag either, so the script had the same cluster-slot constraint it appeared to avoid. Also deletes `RedisBudgetExceededError`, which was defined and never thrown, and consolidates four rounds of stacked comments in the fold down to the one that still describes the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 99d69af commit a69f416

16 files changed

Lines changed: 1439 additions & 208 deletions

File tree

apps/realtime/src/handlers/file-doc-store.test.ts

Lines changed: 267 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,31 @@ vi.mock('redis', () => ({ createClient: () => makeClient() }))
128128
import { FileDocStore, REDIS_AGENT_ORIGIN, REDIS_ORIGIN } from '@/handlers/file-doc-store'
129129

130130
const REDIS_URL = 'redis://fake'
131+
132+
interface StoreRoomInternals {
133+
lastId: string
134+
pendingDeltas: Map<string, number>
135+
realEdited: boolean
136+
publishes: number
137+
compactRetryAfter: number
138+
doc: Y.Doc
139+
seededObserved: boolean
140+
}
141+
142+
interface FileDocStoreInternals {
143+
rooms: Map<string, StoreRoomInternals>
144+
applyEntry(room: StoreRoomInternals, id: string, message: Record<string, string>): void
145+
appendUpdate(name: string, update: Uint8Array, agent?: boolean): Promise<void>
146+
write: { xTrim: (...args: unknown[]) => Promise<unknown> }
147+
maybeCompact(name: string, force?: boolean): Promise<void>
148+
}
149+
150+
/** Reaches the private state these tests assert on, without `any`. */
151+
function internals(store: object): FileDocStoreInternals {
152+
return store as unknown as FileDocStoreInternals
153+
}
154+
155+
const COMPACT_THRESHOLD_ENTRIES = 400
131156
const NAME = 'workspace-file-doc:file-1'
132157

133158
function docWithText(text: string): Y.Doc {
@@ -338,14 +363,16 @@ describe('FileDocStore', () => {
338363
const a = await newStore()
339364
// This task has integrated only up to entry 400 (all no-ops) — its local doc is empty and lags the
340365
// two peer entries. Inject that lagging room directly (a real edit was integrated → realEdited).
341-
;(a as any).rooms.set(NAME, {
366+
internals(a).rooms.set(NAME, {
342367
doc: new Y.Doc(),
343368
lastId: '400-0',
344369
publishes: 0,
370+
compactRetryAfter: 0,
371+
pendingDeltas: new Map(),
345372
seededObserved: true,
346373
realEdited: true,
347374
})
348-
await (a as any).maybeCompact(NAME)
375+
await internals(a).maybeCompact(NAME)
349376

350377
// A fresh catch-up must still reconstruct the peer content — compaction must not have trimmed 401/402.
351378
const doc = new Y.Doc()
@@ -391,16 +418,241 @@ describe('FileDocStore', () => {
391418
const a = await newStore()
392419
const doc = new Y.Doc()
393420
await a.attachRoom(NAME, doc)
394-
const room = (a as any).rooms.get(NAME)
421+
const room = internals(a).rooms.get(NAME)!
395422
expect(room.realEdited).toBe(false)
396423
// Kick off a real (non-agent) append but do NOT await it: realEdited must already be true before the
397424
// xAdd/expire awaits resolve, so any compaction racing on the awaits sees the real edit.
398-
const pending = (a as any).appendUpdate(NAME, updateFor('real user edit'))
425+
const pending = internals(a).appendUpdate(NAME, updateFor('real user edit'))
399426
expect(room.realEdited).toBe(true)
400427
await pending
401428
doc.destroy()
402429
})
403430

431+
it('compacts on appended bytes, before the entry threshold is anywhere near reached', async () => {
432+
const streamKey = `filedoc:stream:${NAME}`
433+
const a = await newStore()
434+
const doc = new Y.Doc()
435+
await a.attachRoom(NAME, doc)
436+
437+
// A handful of large pastes: far below COMPACT_THRESHOLD entries, far above the byte ceiling.
438+
// Before bytes were counted this stream held tens of megabytes and never compacted.
439+
const updates: Uint8Array[] = []
440+
doc.on('update', (u: Uint8Array) => updates.push(u))
441+
for (let i = 0; i < 4; i++) {
442+
doc.getText('body').insert(0, 'x'.repeat(3 * 1024 * 1024))
443+
}
444+
for (const update of updates) {
445+
await a.publishAndWait(NAME, update)
446+
}
447+
448+
await vi.waitFor(
449+
() => {
450+
const stream = state.backing!.streams.get(streamKey)!
451+
expect(stream.length).toBeLessThan(COMPACT_THRESHOLD_ENTRIES)
452+
expect(stream.some((entry) => entry.message.s === '1')).toBe(true)
453+
},
454+
{ timeout: 5000 }
455+
)
456+
457+
// Compaction must be lossless: the whole document is still reconstructable from what remains.
458+
const rebuilt = new Y.Doc()
459+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
460+
expect(rebuilt.getText('body').length).toBe(4 * 3 * 1024 * 1024)
461+
rebuilt.destroy()
462+
doc.destroy()
463+
})
464+
465+
it('does not re-compact on every publish once the document itself exceeds the byte ceiling', async () => {
466+
const streamKey = `filedoc:stream:${NAME}`
467+
const a = await newStore()
468+
const doc = new Y.Doc()
469+
await a.attachRoom(NAME, doc)
470+
471+
const updates: Uint8Array[] = []
472+
doc.on('update', (u: Uint8Array) => updates.push(u))
473+
// Grow the document past the byte ceiling so its own snapshot exceeds it, then keep editing.
474+
// Counting the snapshot as appended bytes would leave the threshold permanently breached and
475+
// force a full snapshot append per keystroke — the amplification the threshold exists to stop.
476+
doc.getText('body').insert(0, 'x'.repeat(12 * 1024 * 1024))
477+
for (let i = 0; i < 30; i++) doc.getText('body').insert(0, 'tiny')
478+
for (const update of updates) {
479+
await a.publishAndWait(NAME, update)
480+
}
481+
await vi.waitFor(() => {
482+
const stream = state.backing!.streams.get(streamKey)!
483+
expect(stream.some((entry) => entry.message.s === '1')).toBe(true)
484+
})
485+
486+
const snapshots = state
487+
.backing!.streams.get(streamKey)!
488+
.filter((entry) => entry.message.s === '1').length
489+
expect(snapshots).toBeLessThanOrEqual(2)
490+
491+
const rebuilt = new Y.Doc()
492+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
493+
expect(rebuilt.getText('body').toString().startsWith('tiny')).toBe(true)
494+
expect(rebuilt.getText('body').length).toBe(12 * 1024 * 1024 + 30 * 4)
495+
rebuilt.destroy()
496+
doc.destroy()
497+
})
498+
499+
it('keeps the byte trigger armed when compaction fails', async () => {
500+
const a = await newStore()
501+
const doc = new Y.Doc()
502+
await a.attachRoom(NAME, doc)
503+
const room = internals(a).rooms.get(NAME)!
504+
room.pendingDeltas = new Map([['1-0', 9 * 1024 * 1024]])
505+
room.realEdited = true
506+
507+
const write = internals(a).write
508+
const original = write.xTrim.bind(write)
509+
write.xTrim = async () => {
510+
throw new Error('redis blip')
511+
}
512+
await internals(a).maybeCompact(NAME, true)
513+
514+
// A failed fold must not disarm the trigger — otherwise the stream stays oversized until
515+
// this task happens to append another full threshold's worth of deltas.
516+
expect([...room.pendingDeltas]).toEqual([['1-0', 9 * 1024 * 1024]])
517+
518+
// But it must not retry immediately either: the snapshot XADD lands before the XTRIM, so a
519+
// persistent trim failure would append a full-document snapshot on every attempt.
520+
const snapshotsAfterFailure = state.backing!.streams.get(`filedoc:stream:${NAME}`)?.length ?? 0
521+
await internals(a).maybeCompact(NAME, true)
522+
await internals(a).maybeCompact(NAME, true)
523+
expect(state.backing!.streams.get(`filedoc:stream:${NAME}`)?.length ?? 0).toBe(
524+
snapshotsAfterFailure
525+
)
526+
527+
write.xTrim = original
528+
doc.destroy()
529+
})
530+
531+
it('keeps counting deltas the trim retained because they sit past the fold boundary', async () => {
532+
const a = await newStore()
533+
const doc = new Y.Doc()
534+
await a.attachRoom(NAME, doc)
535+
const room = internals(a).rooms.get(NAME)!
536+
room.realEdited = true
537+
// The tailer has integrated up to 5-0, so `MINID 5-0` retains both 5-0 (the boundary is
538+
// INCLUSIVE) and 9-0. Their bytes are still in Redis, and dropping them would disarm the
539+
// byte trigger while the stream kept growing.
540+
room.lastId = '5-0'
541+
room.pendingDeltas = new Map([
542+
['3-0', 4 * 1024 * 1024],
543+
['5-0', 6 * 1024 * 1024],
544+
['9-0', 7 * 1024 * 1024],
545+
])
546+
547+
await internals(a).maybeCompact(NAME, true)
548+
549+
expect([...room.pendingDeltas]).toEqual([
550+
['5-0', 6 * 1024 * 1024],
551+
['9-0', 7 * 1024 * 1024],
552+
])
553+
doc.destroy()
554+
})
555+
556+
it('adopts accounting for a stream it takes over, and folds it if already over the ceiling', async () => {
557+
const streamKey = `filedoc:stream:${NAME}`
558+
// A stream left behind by a previous task: two entries, so far under the entry threshold, and
559+
// far over the byte ceiling. A fresh room starting from an empty ledger would never fold it,
560+
// while its own heartbeat kept refreshing the TTL.
561+
const seedDoc = new Y.Doc()
562+
const updates: Uint8Array[] = []
563+
seedDoc.on('update', (u: Uint8Array) => updates.push(u))
564+
seedDoc.getText('body').insert(0, 'x'.repeat(9 * 1024 * 1024))
565+
seedDoc.getText('body').insert(0, 'tail')
566+
state.backing!.streams.set(
567+
streamKey,
568+
updates.map((update, index) => ({
569+
id: `${index + 1}-0`,
570+
message: { u: Buffer.from(update).toString('base64') },
571+
}))
572+
)
573+
state.backing!.seq = updates.length
574+
575+
const a = await newStore()
576+
const doc = new Y.Doc()
577+
await a.attachRoom(NAME, doc)
578+
579+
// Either marker counts as a fold: this room only ever replayed entries, so it never observed
580+
// a real edit and its snapshot is stamped as an agent frame (the no-persist guarantee).
581+
await vi.waitFor(() => {
582+
const stream = state.backing!.streams.get(streamKey)!
583+
expect(stream.some((entry) => entry.message.s === '1' || entry.message.a === '1')).toBe(true)
584+
})
585+
586+
// Lossless: the adopted content survives the fold it triggered.
587+
const rebuilt = new Y.Doc()
588+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
589+
expect(rebuilt.getText('body').length).toBe(9 * 1024 * 1024 + 4)
590+
rebuilt.destroy()
591+
doc.destroy()
592+
seedDoc.destroy()
593+
})
594+
595+
it('counts agent preview deltas, which share a marker with an agent-only snapshot', async () => {
596+
const streamKey = `filedoc:stream:${NAME}`
597+
// Agent preview frames are the LARGE ones — a copilot file edit re-serialising a document is
598+
// what filled Redis. They carry the same marker as a fold of an agent-only stream, so keying
599+
// exclusion on that marker would drop exactly the payloads this bound exists for.
600+
const seedDoc = new Y.Doc()
601+
const updates: Uint8Array[] = []
602+
seedDoc.on('update', (u: Uint8Array) => updates.push(u))
603+
seedDoc.getText('body').insert(0, 'x'.repeat(9 * 1024 * 1024))
604+
seedDoc.getText('body').insert(0, 'tail')
605+
state.backing!.streams.set(
606+
streamKey,
607+
updates.map((update, index) => ({
608+
id: `${index + 1}-0`,
609+
message: { u: Buffer.from(update).toString('base64'), a: '1' },
610+
}))
611+
)
612+
state.backing!.seq = updates.length
613+
614+
const a = await newStore()
615+
const doc = new Y.Doc()
616+
await a.attachRoom(NAME, doc)
617+
618+
await vi.waitFor(() => {
619+
const stream = state.backing!.streams.get(streamKey)!
620+
expect(stream.some((entry) => entry.message.c === '1')).toBe(true)
621+
})
622+
623+
const rebuilt = new Y.Doc()
624+
Y.applyUpdate(rebuilt, (await a.getStreamState(NAME))!)
625+
expect(rebuilt.getText('body').length).toBe(9 * 1024 * 1024 + 4)
626+
rebuilt.destroy()
627+
doc.destroy()
628+
seedDoc.destroy()
629+
})
630+
631+
it("never counts a fold's own output, so a large document cannot arm the trigger against itself", async () => {
632+
const a = await newStore()
633+
const doc = new Y.Doc()
634+
await a.attachRoom(NAME, doc)
635+
const room = internals(a).rooms.get(NAME)!
636+
637+
internals(a).applyEntry(room, '7-0', { u: 'x'.repeat(9 * 1024 * 1024), a: '1', c: '1' })
638+
639+
expect(room.pendingDeltas.has('7-0')).toBe(false)
640+
doc.destroy()
641+
})
642+
643+
it('counts a delta published by a peer task, which this room only ever tails', async () => {
644+
const a = await newStore()
645+
const doc = new Y.Doc()
646+
await a.attachRoom(NAME, doc)
647+
const room = internals(a).rooms.get(NAME)!
648+
649+
// Never published locally, so publish-side accounting would miss it entirely.
650+
internals(a).applyEntry(room, '4-0', { u: 'x'.repeat(1024) })
651+
652+
expect(room.pendingDeltas.get('4-0')).toBe(1024)
653+
doc.destroy()
654+
})
655+
404656
it('stamps a compaction snapshot of an agent-ONLY stream as an agent frame (never persisted)', async () => {
405657
const streamKey = `filedoc:stream:${NAME}`
406658
const noop = Buffer.from(Y.encodeStateAsUpdate(new Y.Doc())).toString('base64')
@@ -414,14 +666,16 @@ describe('FileDocStore', () => {
414666
state.backing!.seq = 400
415667

416668
const a = await newStore()
417-
;(a as any).rooms.set(NAME, {
669+
internals(a).rooms.set(NAME, {
418670
doc: agentDoc,
419671
lastId: '400-0',
420672
publishes: 0,
673+
compactRetryAfter: 0,
674+
pendingDeltas: new Map(),
421675
seededObserved: true,
422676
realEdited: false,
423677
})
424-
await (a as any).maybeCompact(NAME)
678+
await internals(a).maybeCompact(NAME)
425679

426680
// The snapshot must carry the AGENT marker, NOT the snapshot marker, so a peer catch-up applies it as
427681
// REDIS_AGENT_ORIGIN and never marks the doc edited — the no-persist guarantee survives compaction.
@@ -580,21 +834,25 @@ describe('FileDocStore', () => {
580834
const b = await newStore()
581835
const docA = new Y.Doc()
582836
Y.applyUpdate(docA, peerUpdates[0]) // A integrated up to 401
583-
;(a as any).rooms.set(NAME, {
837+
internals(a).rooms.set(NAME, {
584838
doc: docA,
585839
lastId: '401-0',
586840
publishes: 0,
841+
compactRetryAfter: 0,
842+
pendingDeltas: new Map(),
587843
seededObserved: true,
588844
realEdited: true,
589845
})
590-
;(b as any).rooms.set(NAME, {
846+
internals(b).rooms.set(NAME, {
591847
doc: new Y.Doc(),
592848
lastId: '400-0',
593849
publishes: 0,
850+
compactRetryAfter: 0,
851+
pendingDeltas: new Map(),
594852
seededObserved: true,
595853
realEdited: true,
596854
})
597-
await Promise.all([(a as any).maybeCompact(NAME), (b as any).maybeCompact(NAME)])
855+
await Promise.all([internals(a).maybeCompact(NAME), internals(b).maybeCompact(NAME)])
598856

599857
const doc = new Y.Doc()
600858
Y.applyUpdate(doc, (await a.getStreamState(NAME))!)

0 commit comments

Comments
 (0)