Skip to content

Commit 339569e

Browse files
waleedlatif1claude
andcommitted
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>
1 parent 3de3602 commit 339569e

6 files changed

Lines changed: 132 additions & 23 deletions

File tree

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ describe('FileDocStore', () => {
475475
const doc = new Y.Doc()
476476
await a.attachRoom(NAME, doc)
477477
const room = (a as any).rooms.get(NAME)
478-
room.appendedBytes = 9 * 1024 * 1024
478+
room.pendingDeltas = [{ id: '1-0', bytes: 9 * 1024 * 1024 }]
479479
room.realEdited = true
480480

481481
const write = (a as any).write
@@ -488,7 +488,27 @@ describe('FileDocStore', () => {
488488

489489
// A failed fold must not disarm the trigger — otherwise the stream stays oversized until
490490
// this task happens to append another full threshold's worth of deltas.
491-
expect(room.appendedBytes).toBe(9 * 1024 * 1024)
491+
expect(room.pendingDeltas).toEqual([{ id: '1-0', bytes: 9 * 1024 * 1024 }])
492+
doc.destroy()
493+
})
494+
495+
it('keeps counting deltas the trim retained because they sit past the fold boundary', async () => {
496+
const a = await newStore()
497+
const doc = new Y.Doc()
498+
await a.attachRoom(NAME, doc)
499+
const room = (a as any).rooms.get(NAME)
500+
room.realEdited = true
501+
// The tailer has integrated up to 5-0, so MINID retains 9-0. Its bytes are still in Redis,
502+
// and dropping them would disarm the byte trigger while the stream kept growing.
503+
room.lastId = '5-0'
504+
room.pendingDeltas = [
505+
{ id: '3-0', bytes: 4 * 1024 * 1024 },
506+
{ id: '9-0', bytes: 7 * 1024 * 1024 },
507+
]
508+
509+
await (a as any).maybeCompact(NAME, true)
510+
511+
expect(room.pendingDeltas).toEqual([{ id: '9-0', bytes: 7 * 1024 * 1024 }])
492512
doc.destroy()
493513
})
494514

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

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,22 @@ const READER_ERROR_LOG_EVERY = 20
186186

187187
const streamKey = (name: string) => `${STREAM_PREFIX}${name}`
188188

189+
/**
190+
* Unfolded delta bytes a compaction could actually reclaim right now.
191+
*
192+
* Only entries at or before `room.lastId` count: a fold trims to that boundary, so bytes past it
193+
* would survive the trim and re-arm the trigger immediately, forcing a full snapshot append per
194+
* publish that reclaims nothing. They stay in `pendingDeltas` and start counting once the tailer
195+
* has integrated them.
196+
*/
197+
function foldableDeltaBytes(room: StoreRoom): number {
198+
let bytes = 0
199+
for (const delta of room.pendingDeltas) {
200+
if (!isAfterStreamId(delta.id, room.lastId)) bytes += delta.bytes
201+
}
202+
return bytes
203+
}
204+
189205
/**
190206
* Decode one stream entry's base64 Yjs update and apply it to `doc`. A malformed entry is logged and
191207
* SKIPPED — never thrown — so one bad frame can neither wedge the tailer nor abort a headless
@@ -236,14 +252,20 @@ interface StoreRoom {
236252
/** Local publish count, to pace compaction checks. */
237253
publishes: number
238254
/**
239-
* Delta bytes this task has appended since the last compaction it performed, so the byte
240-
* threshold costs no extra round-trip. Counts deltas only — never the snapshot a compaction
241-
* writes, which is a function of document size rather than of edit volume and would make a
242-
* large document breach the threshold permanently. Locally tracked, so it under-counts a peer
243-
* task's appends: it is a trigger, not an accounting, and {@link COMPACT_THRESHOLD} still
244-
* covers many small edits arriving from elsewhere.
255+
* Deltas this task has appended and not yet folded, as `{id, bytes}` pairs in append order.
256+
*
257+
* Keyed by stream id rather than summed, because a fold trims to `room.lastId` and RETAINS
258+
* anything published past it — those bytes are still in Redis, so deducting them would
259+
* disarm the trigger while the stream keeps growing. Entries are dropped only once an
260+
* `XTRIM` has provably removed them.
261+
*
262+
* Counts deltas only — never the snapshot a compaction writes, which is a function of
263+
* document size rather than of edit volume and would make a large document breach the
264+
* threshold permanently. Locally tracked, so it under-counts a peer task's appends: it is a
265+
* trigger, not an accounting, and {@link COMPACT_THRESHOLD} still covers many small edits
266+
* arriving from elsewhere.
245267
*/
246-
appendedBytes: number
268+
pendingDeltas: Array<{ id: string; bytes: number }>
247269
/** Set once the doc has been observed seeded, so the seed transition itself is never mistaken for an
248270
* edit (mirrors the relay's `seededObserved`). */
249271
seededObserved: boolean
@@ -325,7 +347,7 @@ export class FileDocStore {
325347
doc,
326348
lastId: '0',
327349
publishes: 0,
328-
appendedBytes: 0,
350+
pendingDeltas: [],
329351
seededObserved: false,
330352
realEdited: false,
331353
}
@@ -390,9 +412,10 @@ export class FileDocStore {
390412
const encoded = Buffer.from(update).toString('base64')
391413
const fields: Record<string, string> = { [UPDATE_FIELD]: encoded }
392414
if (agent) fields[AGENT_FIELD] = '1'
415+
let appendedId: string | null = null
393416
for (let attempt = 0; attempt <= PUBLISH_MAX_RETRIES; attempt++) {
394417
try {
395-
await this.write.xAdd(streamKey(name), '*', fields)
418+
appendedId = await this.write.xAdd(streamKey(name), '*', fields)
396419
break
397420
} catch (error) {
398421
if (attempt === PUBLISH_MAX_RETRIES) {
@@ -407,11 +430,11 @@ export class FileDocStore {
407430
await this.write.expire(streamKey(name), STREAM_TTL_SEC).catch(() => {})
408431
const room = this.rooms.get(name)
409432
if (!room) return
410-
room.appendedBytes += encoded.length
433+
if (appendedId) room.pendingDeltas.push({ id: appendedId, bytes: encoded.length })
411434
// Bytes are checked every publish: one entry can cross the ceiling on its own, so pacing this
412435
// check the way the entry count is paced would let a stream sit far over the ceiling for up to
413-
// COMPACT_CHECK_EVERY more appends. The check itself is a local comparison.
414-
const overBytes = room.appendedBytes >= COMPACT_BYTES_THRESHOLD
436+
// COMPACT_CHECK_EVERY more appends. The check itself is a local sum over unfolded entries.
437+
const overBytes = foldableDeltaBytes(room) >= COMPACT_BYTES_THRESHOLD
415438
if (overBytes || ++room.publishes % COMPACT_CHECK_EVERY === 0) {
416439
void this.maybeCompact(name, overBytes)
417440
}
@@ -726,6 +749,11 @@ export class FileDocStore {
726749
// but wasteful re-delivery). The new room caught itself up via xRange already.
727750
if (!room || room !== snapshot.get(name)) continue
728751
for (const entry of stream.messages) this.applyEntry(room, entry.id, entry.message)
752+
// Foldability is decided by `lastId`, which only the tailer advances — so a burst of
753+
// large edits followed by silence would otherwise sit unfolded until the next publish
754+
// happened to re-evaluate the trigger. Re-check it where the boundary actually moved.
755+
if (foldableDeltaBytes(room) >= COMPACT_BYTES_THRESHOLD)
756+
void this.maybeCompact(name, true)
729757
}
730758
} catch (error) {
731759
if (!this.running) break
@@ -787,10 +815,7 @@ export class FileDocStore {
787815
// appended snapshot id instead would silently drop those un-integrated peer entries.
788816
const upTo = room.lastId
789817
const snapshot = Buffer.from(Y.encodeStateAsUpdate(room.doc)).toString('base64')
790-
// Bytes this fold is accountable for. Deducted only once the trim succeeds, so a failed
791-
// compaction leaves the trigger armed instead of silently disarming it — and deducting
792-
// rather than zeroing preserves whatever a concurrent publish added while it ran.
793-
const foldedBytes = room.appendedBytes
818+
// Captured with `upTo` so the two agree: exactly the entries this fold will trim.
794819
// Stamp the snapshot by what it folds: a real edit → SNAPSHOT_FIELD (a fresh catch-up treats it
795820
// as edited content, not a bare seed). An agent-ONLY stream (no real edit yet) → AGENT_FIELD, so a
796821
// peer catching up applies it as REDIS_AGENT_ORIGIN and never marks the doc edited — preserving
@@ -806,7 +831,11 @@ export class FileDocStore {
806831
// Never the snapshot's own size: a document whose snapshot already exceeds the ceiling
807832
// would re-breach it the instant compaction finished and force a full snapshot append on
808833
// every subsequent keystroke — the write amplification this threshold exists to prevent.
809-
room.appendedBytes = Math.max(0, room.appendedBytes - foldedBytes)
834+
// Drop only what the trim provably removed. An entry published past `upTo` is retained by
835+
// MINID and its bytes are still in Redis, so it stays counted; dropping it would disarm the
836+
// trigger while the stream kept growing. Done after the trim, so a failed fold changes
837+
// nothing and leaves the trigger armed.
838+
room.pendingDeltas = room.pendingDeltas.filter((delta) => isAfterStreamId(delta.id, upTo))
810839
} finally {
811840
await this.releaseLock(key, token)
812841
}

apps/sim/lib/copilot/request/lifecycle/start.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,10 @@ export function createSSEStream(params: StreamingOrchestrationParams): ReadableS
206206
}
207207
| undefined
208208

209-
await Promise.all([resetBuffer(streamId), clearFilePreviewSessions(streamId)])
209+
await Promise.all([
210+
resetBuffer(streamId, { streamId, ...(userId ? { userId } : {}) }),
211+
clearFilePreviewSessions(streamId),
212+
])
210213

211214
if (chatId) {
212215
createRunSegment({

apps/sim/lib/copilot/request/session/buffer.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,4 +398,24 @@ describe('mothership-stream-outbox', () => {
398398
expect(result.persisted).toBe(false)
399399
expect(mockRedis.eval).not.toHaveBeenCalled()
400400
})
401+
402+
it('releases the owner counter and credits the user when the buffer is cleared', async () => {
403+
// The buffer keys are deleted rather than expired, so a counter left behind would refuse a
404+
// retry that reuses the same streamId against bytes that no longer exist anywhere.
405+
await clearBuffer('stream-1', 'clear_outbox', { streamId: 'stream-1', userId: 'user-1' })
406+
407+
expect(mockRedis.del).toHaveBeenCalled()
408+
const evalCall = mockRedis.eval.mock.calls.at(-1)
409+
expect(evalCall?.[1]).toBe(2)
410+
expect(evalCall?.[2]).toBe('execution:redis-budget:copilot_stream:stream-1')
411+
expect(evalCall?.[3]).toBe('execution:redis-budget:user:user-1')
412+
})
413+
414+
it('releases only the owner counter when no user is in scope', async () => {
415+
await clearBuffer('stream-1')
416+
417+
const evalCall = mockRedis.eval.mock.calls.at(-1)
418+
expect(evalCall?.[1]).toBe(1)
419+
expect(evalCall?.[2]).toBe('execution:redis-budget:copilot_stream:stream-1')
420+
})
401421
})

apps/sim/lib/copilot/request/session/buffer.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
getRedisBudgetLimits,
99
logRedisBudgetRefusal,
1010
parseRedisBudgetRefusal,
11+
REDIS_BUDGET_RELEASE_SCRIPT,
1112
type RedisBudgetRefusal,
1213
renderRedisBudgetLua,
1314
} from '@/lib/core/redis/byte-budget.server'
@@ -102,13 +103,28 @@ export async function allocateCursor(streamId: string): Promise<{
102103
return { seq, cursor: String(seq) }
103104
}
104105

105-
export async function resetBuffer(streamId: string): Promise<void> {
106-
await clearBuffer(streamId, 'reset_outbox')
106+
export async function resetBuffer(streamId: string, scope?: StreamBudgetScope): Promise<void> {
107+
await clearBuffer(streamId, 'reset_outbox', scope)
107108
}
108109

109-
export async function clearBuffer(streamId: string, operation = 'clear_outbox'): Promise<void> {
110+
export async function clearBuffer(
111+
streamId: string,
112+
operation = 'clear_outbox',
113+
scope?: StreamBudgetScope
114+
): Promise<void> {
110115
await withRedisRetry({ operation, streamId }, async (redis) => {
111116
await redis.del(getEventsKey(streamId), getSeqKey(streamId), getAbortKey(streamId))
117+
/*
118+
The counter outlives the data it accounts for unless it is released here: the keys
119+
above are deleted rather than expired, so without this a retry reusing the same
120+
streamId would be refused against bytes that no longer exist anywhere.
121+
*/
122+
const budgetKeys = getRedisBudgetKeys({
123+
kind: 'copilot_stream',
124+
id: streamId,
125+
...(scope?.userId ? { userId: scope.userId } : {}),
126+
})
127+
await redis.eval(REDIS_BUDGET_RELEASE_SCRIPT, budgetKeys.length, ...budgetKeys)
112128
})
113129
}
114130

apps/sim/lib/core/redis/byte-budget.server.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,27 @@ end
201201
`
202202
}
203203

204+
/**
205+
* Releases an owner's whole reservation when its data is deleted rather than expired.
206+
*
207+
* The owner counter is dropped and the user counter credited by exactly what the owner
208+
* held, in one script — crediting the user from a separately read value would let a
209+
* concurrent write land in between and be released twice.
210+
*
211+
* KEYS: [ownerKey] or [ownerKey, userKey], as {@link getRedisBudgetKeys} returns them.
212+
*/
213+
export const REDIS_BUDGET_RELEASE_SCRIPT = `
214+
local owner_bytes = tonumber(redis.call('GET', KEYS[1]) or '0')
215+
redis.call('DEL', KEYS[1])
216+
if #KEYS >= 2 and owner_bytes > 0 then
217+
local user_next = redis.call('DECRBY', KEYS[2], owner_bytes)
218+
if user_next <= 0 then
219+
redis.call('DEL', KEYS[2])
220+
end
221+
end
222+
return owner_bytes
223+
`
224+
204225
/** Parses the `{0, resource, current}` refusal a guarded script returns. */
205226
export function parseRedisBudgetRefusal(
206227
result: unknown,

0 commit comments

Comments
 (0)