Skip to content

Commit f62e99f

Browse files
committed
fix(collab-doc): close durability gaps at deploy boundaries + audit polish
From a comprehensive from-scratch audit (correctness, SOTA, cleanliness, feature-completeness): - Persist max-wait: a continuous edit burst kept resetting the 5s debounce and never persisted; cap it so a burst flushes at least every 20s, bounding unpersisted edits. - Graceful-shutdown flush: flushAllFileDocRooms awaited in shutdown so a rolling deploy / scale-in secures open edited rooms to durable markdown before exit, instead of relying on the stream + a surviving task. - Compacted-snapshot catch-up now marks the doc edited (REDIS_SNAPSHOT_ORIGIN): a snapshot folds seed+edits into one frame, so a task catching up purely from it no longer treats real edits as an unedited seed and skips persisting. - Polish: delete dead __setFileDocStoreForTest; bounded retry loop; rename acquirePersistSlot -> tryClaimPersistWindow with accurate docs; tailer object-identity guard; fix stale comments (seed route, edit-content autosave).
1 parent 9476b21 commit f62e99f

7 files changed

Lines changed: 139 additions & 39 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,11 @@ describe('FileDocStore', () => {
220220
Y.applyUpdate(doc, (await a.getStreamState(NAME))!)
221221
expect(doc.getText('body').toString()).toBe('PEER1PEER2')
222222
doc.destroy()
223+
224+
// The appended snapshot entry must carry the snapshot marker, so a fresh catch-up task treats it as
225+
// edited content (not a bare seed) and persists on last-disconnect.
226+
const stream = state.backing!.streams.get(streamKey)!
227+
expect(stream[stream.length - 1].message.s).toBe('1')
223228
})
224229

225230
it('retries a transient append failure so the edit is not lost from the shared log', async () => {

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

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,26 @@ const RELEASE_LOCK_SCRIPT =
5656
*/
5757
export const REDIS_ORIGIN = Symbol('file-doc-redis')
5858

59+
/**
60+
* Origin for a COMPACTED SNAPSHOT applied from the stream. A snapshot folds the seed + all prior edits
61+
* into one entry, so a fresh task catching up from it would otherwise never see a separate post-seed
62+
* edit frame and would treat the doc as unedited. The relay's edit-tracker uses this origin to mark the
63+
* doc edited (a snapshot only exists after the stream crossed the compaction threshold, i.e. real edits
64+
* happened). Behaves like {@link REDIS_ORIGIN} otherwise (already in the stream — never re-published).
65+
*/
66+
export const REDIS_SNAPSHOT_ORIGIN = Symbol('file-doc-redis-snapshot')
67+
5968
const STREAM_PREFIX = 'filedoc:stream:'
6069
const SEED_LOCK_PREFIX = 'filedoc:seedlock:'
6170
const COMPACT_LOCK_PREFIX = 'filedoc:compactlock:'
6271
const PERSIST_LOCK_PREFIX = 'filedoc:persistlock:'
6372
const MERGE_LOCK_PREFIX = 'filedoc:mergelock:'
6473

65-
/** The single field each stream entry carries — a base64 Yjs update. */
74+
/** The field each stream entry carries — a base64 Yjs update. */
6675
const UPDATE_FIELD = 'u'
76+
/** Marks a stream entry as a compaction SNAPSHOT (folds seed + edits), so the tailer applies it with
77+
* {@link REDIS_SNAPSHOT_ORIGIN}. Present only on snapshot entries. */
78+
const SNAPSHOT_FIELD = 's'
6779

6880
/** Sentinel token a DISABLED store returns from a lock acquire, so single-replica callers proceed
6981
* without special-casing; {@link FileDocStore.releaseLock} treats it as a no-op. Not a real UUID, so it
@@ -228,12 +240,12 @@ export class FileDocStore {
228240
private async appendUpdate(name: string, update: Uint8Array): Promise<void> {
229241
if (!this.write) return
230242
const encoded = Buffer.from(update).toString('base64')
231-
for (let attempt = 0; ; attempt++) {
243+
for (let attempt = 0; attempt <= PUBLISH_MAX_RETRIES; attempt++) {
232244
try {
233245
await this.write.xAdd(streamKey(name), '*', { [UPDATE_FIELD]: encoded })
234246
break
235247
} catch (error) {
236-
if (attempt >= PUBLISH_MAX_RETRIES) {
248+
if (attempt === PUBLISH_MAX_RETRIES) {
237249
logger.error(`FileDocStore append failed for ${name}`, { error: getErrorMessage(error) })
238250
throw error
239251
}
@@ -352,12 +364,14 @@ export class FileDocStore {
352364
}
353365

354366
/**
355-
* Try to claim the right to persist this file for the current debounce window, so concurrent tasks
356-
* editing the same file don't each write a redundant blob version. Returns true (proceed) when
357-
* disabled, or when this task wins a short lock. The final last-collaborator flush does NOT gate on
358-
* this — it must always write.
367+
* A best-effort TTL dedup WINDOW (NOT a lock): claim the right to run a debounced persist for the next
368+
* `ttlMs`, so concurrent tasks editing the same file don't each write a redundant blob version. It is
369+
* never released — it simply expires after `ttlMs`, gating the debounced persist to ~once per window
370+
* cluster-wide. Fails OPEN (returns true on a Redis error): a redundant persist is a harmless
371+
* idempotent write, so it must never block a real one. The final last-collaborator flush does NOT gate
372+
* on this — it must always write.
359373
*/
360-
async acquirePersistSlot(name: string, ttlMs: number): Promise<boolean> {
374+
async tryClaimPersistWindow(name: string, ttlMs: number): Promise<boolean> {
361375
if (!this.enabled || !this.write) return true
362376
try {
363377
const won = await this.write.set(`${PERSIST_LOCK_PREFIX}${name}`, '1', {
@@ -388,7 +402,10 @@ export class FileDocStore {
388402

389403
private applyEntry(room: StoreRoom, id: string, message: Record<string, string>): void {
390404
room.lastId = id
391-
applyEntryToDoc(room.doc, id, message, REDIS_ORIGIN)
405+
// A compaction snapshot folds seed + edits into one frame; stamp it so the relay's edit-tracker
406+
// treats a fresh catch-up from it as edited (a snapshot only exists once real edits accumulated).
407+
const origin = message[SNAPSHOT_FIELD] ? REDIS_SNAPSHOT_ORIGIN : REDIS_ORIGIN
408+
applyEntryToDoc(room.doc, id, message, origin)
392409
}
393410

394411
/**
@@ -397,21 +414,24 @@ export class FileDocStore {
397414
*/
398415
private async runReader(): Promise<void> {
399416
while (this.running && this.read) {
400-
const snapshot = [...this.rooms.entries()]
401-
if (snapshot.length === 0) {
417+
const snapshot = new Map(this.rooms)
418+
if (snapshot.size === 0) {
402419
await sleep(IDLE_POLL_MS)
403420
continue
404421
}
405422
try {
406423
const res = await this.read.xRead(
407-
snapshot.map(([name, room]) => ({ key: streamKey(name), id: room.lastId })),
424+
[...snapshot].map(([name, room]) => ({ key: streamKey(name), id: room.lastId })),
408425
{ BLOCK: READ_BLOCK_MS, COUNT: READ_COUNT }
409426
)
410427
if (!res) continue
411428
for (const stream of res) {
412429
const name = stream.name.slice(STREAM_PREFIX.length)
413430
const room = this.rooms.get(name)
414-
if (!room) continue // detached mid-read; its doc is being destroyed
431+
// Skip if detached mid-read, OR replaced by a close→reopen (a DIFFERENT StoreRoom): applying
432+
// entries read against the OLD room's lastId to the new one could regress its lastId (harmless
433+
// but wasteful re-delivery). The new room caught itself up via xRange already.
434+
if (!room || room !== snapshot.get(name)) continue
415435
for (const entry of stream.messages) this.applyEntry(room, entry.id, entry.message)
416436
}
417437
} catch (error) {
@@ -446,7 +466,11 @@ export class FileDocStore {
446466
// appended snapshot id instead would silently drop those un-integrated peer entries.
447467
const upTo = room.lastId
448468
const snapshot = Buffer.from(Y.encodeStateAsUpdate(room.doc)).toString('base64')
449-
await this.write.xAdd(streamKey(name), '*', { [UPDATE_FIELD]: snapshot })
469+
// Mark it a snapshot so a fresh catch-up task treats it as edited content, not a bare seed.
470+
await this.write.xAdd(streamKey(name), '*', {
471+
[UPDATE_FIELD]: snapshot,
472+
[SNAPSHOT_FIELD]: '1',
473+
})
450474
// MINID keeps entries with id >= upTo: the snapshot, any un-integrated peer entries, and
451475
// `upTo` itself (redundant with the snapshot, harmless); it drops only the folded older deltas.
452476
await this.write.xTrim(streamKey(name), 'MINID', upTo)
@@ -486,8 +510,3 @@ export function getFileDocStore(): FileDocStore {
486510
if (!store) store = new FileDocStore(undefined)
487511
return store
488512
}
489-
490-
/** Test-only: reset the singleton so a test can install its own instance. */
491-
export function __setFileDocStoreForTest(next: FileDocStore | null): void {
492-
store = next
493-
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ vi.mock('@/handlers/file-doc-app', () => ({
3535
import {
3636
applyMarkdownToLiveFileDoc,
3737
cleanupFileDocForSocket,
38+
flushAllFileDocRooms,
3839
setupWorkspaceFileDocHandlers,
3940
} from '@/handlers/file-doc'
4041

@@ -296,6 +297,32 @@ describe('setupWorkspaceFileDocHandlers', () => {
296297
expect(mockFetchFileDocPersist).toHaveBeenCalled()
297298
})
298299

300+
it('flushAllFileDocRooms persists open EDITED rooms (graceful shutdown), skips unedited', async () => {
301+
mockFetchFileDocSeed.mockResolvedValue(encodedSeedUpdate('# From server'))
302+
const { io } = createIo()
303+
const { handlers } = setup('socket-1', io)
304+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
305+
await flushMicrotasks()
306+
307+
// Seed-only room: a graceful-shutdown flush must NOT persist it.
308+
mockFetchFileDocPersist.mockClear()
309+
await flushAllFileDocRooms()
310+
expect(mockFetchFileDocPersist).not.toHaveBeenCalled()
311+
312+
// After a real user edit, the same flush persists (edits would otherwise be lost on deploy).
313+
const edit = new Y.Doc()
314+
edit.getText(FILE_DOC_FIELD).insert(0, 'typed')
315+
handlers[FILE_DOC_EVENTS.MESSAGE](
316+
frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) =>
317+
syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(edit))
318+
)
319+
)
320+
await flushMicrotasks()
321+
mockFetchFileDocPersist.mockClear()
322+
await flushAllFileDocRooms()
323+
expect(mockFetchFileDocPersist).toHaveBeenCalled()
324+
})
325+
299326
it('joins the room, sends sync step 1, and seeds the document from the server', async () => {
300327
mockFetchFileDocSeed.mockResolvedValue(encodedSeedUpdate('# From server'))
301328
const { io } = createIo()

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

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import * as syncProtocol from 'y-protocols/sync'
4545
import * as Y from 'yjs'
4646
import { resolveAvatarUrl } from '@/handlers/avatar'
4747
import { fetchFileDocMerge, fetchFileDocPersist, fetchFileDocSeed } from '@/handlers/file-doc-app'
48-
import { getFileDocStore, REDIS_ORIGIN } from '@/handlers/file-doc-store'
48+
import { getFileDocStore, REDIS_ORIGIN, REDIS_SNAPSHOT_ORIGIN } from '@/handlers/file-doc-store'
4949
import { resolveRoomJoinAuth } from '@/handlers/room-join-auth'
5050
import type { AuthenticatedSocket } from '@/middleware/auth'
5151
import type { IRoomManager } from '@/rooms'
@@ -61,6 +61,10 @@ const SEED_ORIGIN = Symbol('file-doc-seed')
6161

6262
/** Debounce window for the server-side project-to-markdown persist while a doc is actively edited. */
6363
const PERSIST_DEBOUNCE_MS = 5_000
64+
/** Max-wait cap on the persist debounce: a CONTINUOUS edit burst keeps resetting the 5s debounce and
65+
* would otherwise never persist until an idle pause, so force a flush at least this often — bounding how
66+
* many edits are unpersisted (in the stream only) if the task dies mid-burst. */
67+
const PERSIST_MAX_WAIT_MS = 20_000
6468

6569
/** Cross-task merge lock. The TTL must exceed the whole critical section it guards — stream fold +
6670
* `fetchFileDocMerge` (bounded at `mergeRequestMs`) + the awaited publish — so the lock never expires
@@ -118,6 +122,9 @@ interface FileDocRoom {
118122
seededObserved: boolean
119123
/** The pending debounced persist timer, if any. */
120124
persistTimer: ReturnType<typeof setTimeout> | null
125+
/** Absolute time (ms) by which a debounced persist must fire even under continuous editing (the
126+
* max-wait cap); null when no persist is pending. */
127+
persistDeadline: number | null
121128
}
122129

123130
/** Live documents keyed by Socket.IO room name. Module-global: one Y.Doc per file. */
@@ -184,24 +191,29 @@ function broadcastLocal(
184191

185192
/**
186193
* Schedule a debounced server-side persist of the live doc back to durable markdown. Coalesces rapid
187-
* edits; a no-op until the room knows its workspace (set at join). The final flush on last-disconnect
188-
* is separate ({@link flushPersist} with `final`).
194+
* edits; a no-op until the room knows its workspace (set at join). A {@link PERSIST_MAX_WAIT_MS}
195+
* max-wait caps the debounce so a continuous burst still persists periodically. The final flush on
196+
* last-disconnect is separate ({@link flushPersist} with `final`).
189197
*/
190198
function schedulePersist(name: string, room: FileDocRoom): void {
191199
if (!room.workspaceId || !room.lastEditorUserId) return
200+
const now = Date.now()
201+
if (room.persistDeadline === null) room.persistDeadline = now + PERSIST_MAX_WAIT_MS
192202
if (room.persistTimer) clearTimeout(room.persistTimer)
203+
const delay = Math.max(0, Math.min(PERSIST_DEBOUNCE_MS, room.persistDeadline - now))
193204
room.persistTimer = setTimeout(() => {
194205
room.persistTimer = null
206+
room.persistDeadline = null
195207
void flushPersist(name, room, false)
196-
}, PERSIST_DEBOUNCE_MS)
208+
}, delay)
197209
}
198210

199211
/**
200212
* Project the live doc to markdown and write it durably via the app. `final` (last collaborator
201-
* leaving) always writes; a debounced mid-edit flush first claims a cross-task slot (held for the whole
202-
* write, so a concurrent task can't issue an overlapping blob write) so tasks don't each write a
203-
* redundant version. Best-effort: never throws (a failure is retried on the next debounce; the stream
204-
* holds the state meanwhile).
213+
* leaving) always writes; a debounced mid-edit flush first claims a best-effort cross-task dedup WINDOW
214+
* (a TTL key that just expires, so at most ~one persist per window cluster-wide) so concurrent tasks
215+
* editing the same file don't each write a redundant blob version. Best-effort: never throws (a failure
216+
* is retried on the next debounce; the stream holds the state meanwhile).
205217
*
206218
* Persists the AUTHORITATIVE shared state (the stream), not this task's local doc: a copilot merge — or
207219
* a peer's edit — published by another task may not be integrated into `room.doc` yet (and the stream
@@ -218,7 +230,7 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
218230
// this yields. Only meaningful once seeded; used only when the authoritative stream state is absent.
219231
const localState = isDocSeeded(room.doc) ? Y.encodeStateAsUpdate(room.doc) : null
220232
try {
221-
if (!final && !(await store.acquirePersistSlot(name, FILE_DOC_TIMEOUTS.persistRequestMs)))
233+
if (!final && !(await store.tryClaimPersistWindow(name, FILE_DOC_TIMEOUTS.persistRequestMs)))
222234
return
223235
let docState = localState
224236
if (store.enabled) {
@@ -294,6 +306,7 @@ function awarenessUpdateClientIds(update: Uint8Array): number[] {
294306
function destroyRoomIfIdle(name: string) {
295307
const room = fileDocRooms.get(name)
296308
if (!room || room.owners.size > 0) return
309+
room.persistDeadline = null
297310
if (room.persistTimer) {
298311
clearTimeout(room.persistTimer)
299312
room.persistTimer = null
@@ -307,6 +320,21 @@ function destroyRoomIfIdle(name: string) {
307320
fileDocRooms.delete(name)
308321
}
309322

323+
/**
324+
* Flush every open, edited room's converged doc to durable markdown, AWAITING the writes. Called on
325+
* graceful shutdown (rolling deploy / scale-in) so edits since the last debounce aren't left only in the
326+
* ephemeral stream — the per-socket disconnect flush is fire-and-forget and would race `process.exit`.
327+
* Best-effort and bounded by each persist's own timeout; never throws. Rooms are NOT torn down here (the
328+
* process is exiting); only their durable state is secured.
329+
*/
330+
export async function flushAllFileDocRooms(): Promise<void> {
331+
const flushes: Promise<void>[] = []
332+
for (const [name, room] of fileDocRooms) {
333+
if (room.edited) flushes.push(flushPersist(name, room, true))
334+
}
335+
await Promise.all(flushes)
336+
}
337+
310338
/**
311339
* Seed a room's document server-side, once, on the first join: ask the app to build the seed (the
312340
* file's current markdown → Yjs, through the exact editor engine) and apply it, which relays the
@@ -498,6 +526,7 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
498526
edited: false,
499527
seededObserved: false,
500528
persistTimer: null,
529+
persistDeadline: null,
501530
}
502531
// Register synchronously BEFORE the async catch-up so a concurrent join sees this room, not a second.
503532
fileDocRooms.set(name, room)
@@ -509,18 +538,27 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
509538
// Fan out to THIS task's clients only (excluding the origin socket if local). Cross-task delivery
510539
// rides the shared stream — every task's tailer applies + runs its own local fan-out.
511540
broadcastLocal(io, name, encoding.toUint8Array(encoder), originSocketId(origin))
512-
// Share every locally-originated update to the stream so peers converge. Skip REDIS_ORIGIN (already
513-
// in the stream) and SEED_ORIGIN — the seed is published EXPLICITLY and AWAITED under the seed lock
514-
// (so it lands before the lock releases), which a fire-and-forget publish here couldn't guarantee.
515-
if (origin !== REDIS_ORIGIN && origin !== SEED_ORIGIN) getFileDocStore().publish(name, update)
541+
// Share every locally-originated update to the stream so peers converge. Skip updates that already
542+
// came FROM the stream (REDIS_ORIGIN / REDIS_SNAPSHOT_ORIGIN) and SEED_ORIGIN — the seed is published
543+
// EXPLICITLY and AWAITED under the seed lock (so it lands before the lock releases), which a
544+
// fire-and-forget publish here couldn't guarantee.
545+
if (origin !== REDIS_ORIGIN && origin !== REDIS_SNAPSHOT_ORIGIN && origin !== SEED_ORIGIN)
546+
getFileDocStore().publish(name, update)
516547
// Edit tracking for persistence. Mark the doc dirty on any update applied AFTER it was seeded — a
517548
// local user edit (socket origin) OR a peer's edit relayed via the tailer (REDIS_ORIGIN) — so
518-
// whichever task is last to leave persists real edits, even one that only tailed them. The seed
519-
// transition itself is never counted (nor a purely-local copilot merge, already durable via
520-
// copilot's direct file write), so a seeded-but-unedited doc is never projected back over the file.
549+
// whichever task is last to leave persists real edits, even one that only tailed them. A compaction
550+
// snapshot on catch-up (REDIS_SNAPSHOT_ORIGIN) also counts: it folds real edits into one frame, so a
551+
// fresh task catching up purely from it must not treat the doc as unedited. The seed transition
552+
// itself is never counted (nor a purely-local copilot merge, already durable via copilot's direct
553+
// file write), so a seeded-but-unedited doc is never projected back over the file.
521554
const seededBefore = room.seededObserved
522555
if (isDocSeeded(room.doc)) room.seededObserved = true
523-
if (originSocketId(origin) || (seededBefore && origin === REDIS_ORIGIN)) room.edited = true
556+
if (
557+
originSocketId(origin) ||
558+
origin === REDIS_SNAPSHOT_ORIGIN ||
559+
(seededBefore && origin === REDIS_ORIGIN)
560+
)
561+
room.edited = true
524562
// Debounce a persist for LOCAL user edits only (peers debounce their own).
525563
if (originSocketId(origin)) schedulePersist(name, room)
526564
})

apps/realtime/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createSocketIOServer, shutdownSocketIOAdapter } from '@/config/socket'
66
import { assertSchemaCompatibility } from '@/database/preflight'
77
import { env } from '@/env'
88
import { setupAllHandlers } from '@/handlers'
9+
import { flushAllFileDocRooms } from '@/handlers/file-doc'
910
import { getFileDocStore, initFileDocStore } from '@/handlers/file-doc-store'
1011
import { type AuthenticatedSocket, authenticateSocket } from '@/middleware/auth'
1112
import { type IRoomManager, MemoryRoomManager, RedisRoomManager } from '@/rooms'
@@ -116,6 +117,15 @@ async function main() {
116117

117118
accessRevalidation.stop()
118119

120+
// Flush open collaborative docs to durable markdown BEFORE tearing down Redis/the store — the
121+
// per-socket disconnect flush is fire-and-forget and would race process exit.
122+
try {
123+
await flushAllFileDocRooms()
124+
logger.info('Flushed open collaborative documents')
125+
} catch (error) {
126+
logger.error('Error flushing collaborative documents on shutdown:', error)
127+
}
128+
119129
try {
120130
await roomManager.shutdown()
121131
logger.info('RoomManager shutdown complete')

apps/sim/app/api/internal/file-doc/seed/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const logger = createLogger('FileDocSeedAPI')
1414
* POST /api/internal/file-doc/seed — build a server-authoritative collaborative-document seed
1515
* (markdown → Yjs) for the realtime relay to apply on room creation. Internal only: gated on the
1616
* shared `x-api-key: INTERNAL_API_SECRET` secret, matching the header the realtime relay sends
17-
* (`apps/realtime/src/handlers/file-doc-seed.ts`) and the realtime server's own inbound validator.
17+
* (`apps/realtime/src/handlers/file-doc-app.ts`) and the realtime server's own inbound validator.
1818
*/
1919
export const POST = withRouteHandler(async (request: NextRequest) => {
2020
const auth = checkInternalApiKey(request)

0 commit comments

Comments
 (0)