Skip to content

Commit 3475eb7

Browse files
waleedlatif1claude
andcommitted
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>
1 parent 2487350 commit 3475eb7

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ interface StoreRoomInternals {
134134
pendingDeltas: Array<{ id: string; bytes: number }>
135135
realEdited: boolean
136136
publishes: number
137+
compactRetryAfter: number
137138
doc: Y.Doc
138139
seededObserved: boolean
139140
}
@@ -365,6 +366,7 @@ describe('FileDocStore', () => {
365366
doc: new Y.Doc(),
366367
lastId: '400-0',
367368
publishes: 0,
369+
compactRetryAfter: 0,
368370
pendingDeltas: [],
369371
seededObserved: true,
370372
realEdited: true,
@@ -507,11 +509,21 @@ describe('FileDocStore', () => {
507509
throw new Error('redis blip')
508510
}
509511
await internals(a).maybeCompact(NAME, true)
510-
write.xTrim = original
511512

512513
// A failed fold must not disarm the trigger — otherwise the stream stays oversized until
513514
// this task happens to append another full threshold's worth of deltas.
514515
expect(room.pendingDeltas).toEqual([{ id: '1-0', bytes: 9 * 1024 * 1024 }])
516+
517+
// But it must not retry immediately either: the snapshot XADD lands before the XTRIM, so a
518+
// persistent trim failure would append a full-document snapshot on every attempt.
519+
const snapshotsAfterFailure = state.backing!.streams.get(`filedoc:stream:${NAME}`)?.length ?? 0
520+
await internals(a).maybeCompact(NAME, true)
521+
await internals(a).maybeCompact(NAME, true)
522+
expect(state.backing!.streams.get(`filedoc:stream:${NAME}`)?.length ?? 0).toBe(
523+
snapshotsAfterFailure
524+
)
525+
526+
write.xTrim = original
515527
doc.destroy()
516528
})
517529

@@ -557,6 +569,7 @@ describe('FileDocStore', () => {
557569
doc: agentDoc,
558570
lastId: '400-0',
559571
publishes: 0,
572+
compactRetryAfter: 0,
560573
pendingDeltas: [],
561574
seededObserved: true,
562575
realEdited: false,
@@ -724,6 +737,7 @@ describe('FileDocStore', () => {
724737
doc: docA,
725738
lastId: '401-0',
726739
publishes: 0,
740+
compactRetryAfter: 0,
727741
pendingDeltas: [],
728742
seededObserved: true,
729743
realEdited: true,
@@ -732,6 +746,7 @@ describe('FileDocStore', () => {
732746
doc: new Y.Doc(),
733747
lastId: '400-0',
734748
publishes: 0,
749+
compactRetryAfter: 0,
735750
pendingDeltas: [],
736751
seededObserved: true,
737752
realEdited: true,

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,15 @@ const COMPACT_CHECK_EVERY = 64
162162
/** Compaction critical section (snapshot + xAdd + xTrim) is fast; a generous TTL covers a slow Redis
163163
* round-trip without risking expiry mid-compact. Released via compare-and-delete regardless. */
164164
const COMPACT_LOCK_TTL_MS = 10_000
165+
/**
166+
* Quiet period after a failed fold before another may be forced.
167+
*
168+
* A failed fold deliberately leaves the trigger armed so the bytes are not forgotten, but the
169+
* snapshot `XADD` lands before the `XTRIM` — so if the trim is what failed, retrying immediately
170+
* appends another full-document snapshot each time, turning a Redis blip into exactly the write
171+
* amplification the threshold exists to prevent. The entry-count path is unaffected.
172+
*/
173+
const COMPACT_RETRY_COOLDOWN_MS = 30_000
165174
/** Retry a failed stream append this many times before giving up, so a transient Redis blip doesn't
166175
* silently drop an edit from the shared log (which no peer would then ever see). */
167176
const PUBLISH_MAX_RETRIES = 3
@@ -253,6 +262,8 @@ interface StoreRoom {
253262
lastId: string
254263
/** Local publish count, to pace compaction checks. */
255264
publishes: number
265+
/** Epoch ms before which no forced fold is attempted, after one failed. */
266+
compactRetryAfter: number
256267
/**
257268
* Deltas this task has appended and not yet folded, as `{id, bytes}` pairs in append order.
258269
*
@@ -349,6 +360,7 @@ export class FileDocStore {
349360
doc,
350361
lastId: '0',
351362
publishes: 0,
363+
compactRetryAfter: 0,
352364
pendingDeltas: [],
353365
seededObserved: false,
354366
realEdited: false,
@@ -804,6 +816,7 @@ export class FileDocStore {
804816
const room = this.rooms.get(name)
805817
if (!room) return
806818
try {
819+
if (force && Date.now() < room.compactRetryAfter) return
807820
if (!force && (await this.write.xLen(streamKey(name))) < COMPACT_THRESHOLD) return
808821
const key = `${COMPACT_LOCK_PREFIX}${name}`
809822
const token = await this.acquireLock(key, COMPACT_LOCK_TTL_MS)
@@ -844,6 +857,7 @@ export class FileDocStore {
844857
await this.releaseLock(key, token)
845858
}
846859
} catch (error) {
860+
room.compactRetryAfter = Date.now() + COMPACT_RETRY_COOLDOWN_MS
847861
logger.warn(`FileDocStore compaction failed for ${name}`, { error: getErrorMessage(error) })
848862
}
849863
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,13 @@ export async function appendEvents(
260260
...(scope?.userId ? { userId: scope.userId } : {}),
261261
}
262262
const budgetKeys = getRedisBudgetKeys(budgetScope)
263+
/*
264+
A counter must never expire before the data it accounts for: the next write would then
265+
see zero reserved and let the stream grow by another full ceiling. `COPILOT_STREAM_TTL_SECONDS`
266+
is configurable and defaults to exactly the budget window, so raising it would otherwise
267+
break that invariant silently.
268+
*/
269+
const budgetTtlSeconds = Math.max(limits.ttlSeconds, config.ttlSeconds)
263270

264271
/*
265272
Redis measures a member in UTF-8 bytes, so the ceiling has to be measured the same
@@ -321,7 +328,7 @@ export async function appendEvents(
321328
config.eventLimit,
322329
limits.maxOwnerBytes,
323330
limits.maxUserBytes,
324-
limits.ttlSeconds,
331+
budgetTtlSeconds,
325332
String(chunk.members[chunk.members.length - 1].seq),
326333
...zaddArgs
327334
)

0 commit comments

Comments
 (0)