Skip to content

Commit bca1f3e

Browse files
committed
Add canary assertion to verify leak path is exercised
The test now checks that >=80% of sync windows produce an eof with reason=time_limit. Without this, a fast test server could let syncs complete before the limit fires, silently bypassing the leak path (exactly what happened with time_limit=2s). Also: parse NDJSON responses to detect the time_limit eof marker. Made-with: Cursor Committed-By-Agent: cursor
1 parent 2e4de46 commit bca1f3e

1 file changed

Lines changed: 37 additions & 10 deletions

File tree

e2e/memory-leak.test.ts

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ const SEED_BATCH = 1000
2727
const WARMUP_ITERATIONS = 25
2828
const TEST_ITERATIONS = 50
2929
// Must be short enough that syncs don't complete before the limit fires.
30-
// 5000 rows at ~3ms/page = ~150ms total; 0.1s ensures early termination.
30+
// 5000 rows ÷ 100/page = 50 pages at ~3ms each = ~150ms total.
31+
// time_limit=0.1s processes ~33 pages, guaranteeing early termination.
32+
// Each response must include an eof with reason=time_limit — verified below.
3133
const TIME_LIMIT_SECONDS = 0.1
3234

3335
const RANGE_START = Math.floor(new Date('2021-04-03T00:00:00Z').getTime() / 1000)
@@ -104,13 +106,23 @@ function spawnEngine(port: number): { proc: ChildProcess; ready: Promise<void> }
104106
return { proc, ready }
105107
}
106108

107-
async function drainResponse(res: Response): Promise<void> {
109+
const decoder = new TextDecoder()
110+
111+
/** Drain an NDJSON response; returns true if an eof with reason=time_limit was seen. */
112+
async function drainResponse(res: Response): Promise<boolean> {
108113
const reader = res.body?.getReader()
109-
if (!reader) return
114+
if (!reader) return false
115+
let sawTimeLimit = false
116+
let buffer = ''
110117
while (true) {
111-
const { done } = await reader.read()
118+
const { done, value } = await reader.read()
112119
if (done) break
120+
buffer += decoder.decode(value, { stream: true })
121+
if (!sawTimeLimit && buffer.includes('"reason":"time_limit"')) {
122+
sawTimeLimit = true
123+
}
113124
}
125+
return sawTimeLimit
114126
}
115127

116128
// ── Test suite ───────────────────────────────────────────────────
@@ -228,15 +240,17 @@ describe('memory leak regression', { timeout: 600_000 }, () => {
228240
const pid = engineProc.pid!
229241
const rssSamples: number[] = []
230242
const totalIterations = WARMUP_ITERATIONS + TEST_ITERATIONS
243+
let timeLimitEofCount = 0
231244

232245
for (let i = 0; i < totalIterations; i++) {
233246
const res = await fetch(
234247
`http://localhost:${enginePort}/pipeline_sync?time_limit=${TIME_LIMIT_SECONDS}`,
235248
{ method: 'POST', headers: { 'X-Pipeline': pipelineHeader } }
236249
)
237-
await drainResponse(res)
250+
const sawTimeLimit = await drainResponse(res)
251+
if (sawTimeLimit) timeLimitEofCount++
238252

239-
// Brief pause to let GC run
253+
// Brief pause to let V8's incremental GC run
240254
await new Promise((r) => setTimeout(r, 500))
241255

242256
const rss = getRssKb(pid)
@@ -257,6 +271,20 @@ describe('memory leak regression', { timeout: 600_000 }, () => {
257271
)
258272
}
259273

274+
// ── Canary: verify the test is exercising the leak path ────
275+
// If time_limit never fires, syncs complete naturally and the
276+
// leak path (orphaned iterators after early termination) is
277+
// never exercised, making this test meaningless.
278+
const timeLimitPct = (timeLimitEofCount / totalIterations) * 100
279+
console.log(
280+
`\n Canary: ${timeLimitEofCount}/${totalIterations} windows ended by time_limit (${timeLimitPct.toFixed(0)}%)`
281+
)
282+
expect(
283+
timeLimitEofCount,
284+
`Only ${timeLimitEofCount}/${totalIterations} syncs hit time_limit — ` +
285+
`the test is not exercising the leak path. Reduce TIME_LIMIT_SECONDS or add more data.`
286+
).toBeGreaterThanOrEqual(totalIterations * 0.8)
287+
260288
// ── Assertions on post-warmup samples ──────────────────────
261289
const postWarmup = rssSamples.slice(WARMUP_ITERATIONS)
262290
expect(
@@ -274,10 +302,9 @@ describe('memory leak regression', { timeout: 600_000 }, () => {
274302
console.log(` Total growth: ${totalGrowthMb.toFixed(1)} MB`)
275303
console.log(` Slope: ${slope.toFixed(1)} KB/iteration`)
276304

277-
// Before the fix: unbounded leak grows 50-100+ MB per 60s window,
278-
// producing slopes well above 5000 KB/iter even with 2s windows.
279-
// After the fix: RSS plateaus with minor V8 old-space expansion,
280-
// typically <2000 KB/iter slope and <150 MB total growth.
305+
// Before the fix: orphaned iterators accumulate in pending arrays,
306+
// producing slopes >3000 KB/iter even with short windows.
307+
// After the fix: RSS plateaus with minor V8 heap noise.
281308
expect(slope, `RSS slope ${slope.toFixed(0)} KB/iter exceeds threshold`).toBeLessThan(3000)
282309

283310
expect(

0 commit comments

Comments
 (0)