Skip to content

Commit 0b7e0ad

Browse files
committed
fix(realtime): end the reader's failure streak on an idle read, not a busy one
Review findings, both accurate. The streak reset sat after the entries were applied, so a blocking read that timed out with nothing new — the idle steady state — skipped it via `continue`. A healed outage's count therefore survived through normal polling, and the next unrelated blip opened at the backoff cap: minutes of avoidable split-brain, and a log line claiming a failure count it never earned. The streak now ends on the read RETURNING, which is what proves the connection works. Also: the new test built a raw `setTimeout` promise instead of the shared `sleep`, which CLAUDE.md calls out by name.
1 parent 8e2733c commit 0b7e0ad

2 files changed

Lines changed: 40 additions & 3 deletions

File tree

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { sleep } from '@sim/utils/helpers'
45
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
56
import * as Y from 'yjs'
67

@@ -74,7 +75,7 @@ function makeClient(): any {
7475
if (after.length) res.push({ name: key, messages: after.map((e) => ({ ...e })) })
7576
}
7677
if (res.length) return res
77-
await new Promise((r) => setTimeout(r, 5))
78+
await sleep(5)
7879
return null
7980
},
8081
set: async (key: string, val: string, opts?: { NX?: boolean }) => {
@@ -173,7 +174,7 @@ describe('FileDocStore', () => {
173174

174175
state.backing!.connects = 0 // ignore the two `init` connects; count only recovery attempts
175176
const before = state.backing!.reads
176-
await new Promise((r) => setTimeout(r, 3000))
177+
await sleep(3000)
177178
const attempts = state.backing!.reads - before
178179

179180
// A fixed 500ms retry manages 6–7 attempts in this window; backing off (500 → 1s → 2s → …) manages
@@ -185,6 +186,37 @@ describe('FileDocStore', () => {
185186
doc.destroy()
186187
})
187188

189+
/**
190+
* The streak has to end on a read that RETURNS, not on one that carries messages: a blocking read
191+
* timing out with nothing new is the idle steady state. Counting only message-bearing reads would
192+
* keep a healed outage's streak alive through normal polling, so the next unrelated blip would open
193+
* at the backoff cap — minutes of unnecessary split-brain — and log a count it never earned.
194+
*/
195+
it('ends the failure streak on an idle read, so a later blip starts over', async () => {
196+
const store = await newStore()
197+
const doc = new Y.Doc()
198+
await store.attachRoom(NAME, doc)
199+
200+
// Build a streak of two failures (retries back off ~0.5s, then ~1s).
201+
state.backing!.readerClosed = true
202+
await sleep(800)
203+
// Redis comes back. Wait past the pending backoff so a read actually lands — and it returns
204+
// nothing new, which is the idle case this test is about.
205+
state.backing!.readerClosed = false
206+
await sleep(1000)
207+
208+
// A fresh blip must retry at the START of the backoff curve, not at the cap.
209+
state.backing!.readerClosed = true
210+
const before = state.backing!.reads
211+
await sleep(1900)
212+
const attempts = state.backing!.reads - before
213+
214+
// Streak reset ⇒ retries at ~0, ~0.5s, ~1.5s ⇒ 3 attempts (2 even if the machine is loaded and
215+
// every sleep overshoots by half). Streak carried over ⇒ ~2s then ~4s ⇒ at most 1.
216+
expect(attempts).toBeGreaterThanOrEqual(2)
217+
doc.destroy()
218+
})
219+
188220
it('elects exactly one seeder across tasks (no split-brain seed)', async () => {
189221
const a = await newStore()
190222
const b = await newStore()

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,12 @@ export class FileDocStore {
676676
[...snapshot].map(([name, room]) => ({ key: streamKey(name), id: room.lastId })),
677677
{ BLOCK: READ_BLOCK_MS, COUNT: READ_COUNT }
678678
)
679+
// The streak ends HERE, on the read returning at all — not further down once entries are
680+
// applied. A blocking read that times out with nothing new is the idle steady state, and it
681+
// proves the connection works just as well as one carrying messages; leaving the streak
682+
// standing through it would keep an old outage's count alive indefinitely, so the next
683+
// unrelated blip would open at the backoff cap and log a failure count it never earned.
684+
failures = 0
679685
if (!res) continue
680686
for (const stream of res) {
681687
const name = stream.name.slice(STREAM_PREFIX.length)
@@ -686,7 +692,6 @@ export class FileDocStore {
686692
if (!room || room !== snapshot.get(name)) continue
687693
for (const entry of stream.messages) this.applyEntry(room, entry.id, entry.message)
688694
}
689-
failures = 0
690695
} catch (error) {
691696
if (!this.running) break
692697
await this.recoverReader(++failures, error)

0 commit comments

Comments
 (0)