Fix the flusher wedging on an empty flushPending() - #269
Conversation
runFlush() with nothing staged ran its async body to completion synchronously, so the finally cleared `flushing` before the outer assignment pinned it to a settled promise. Every later flush became a no-op until the stall breaker re-staged the whole snapshot. Any no-write POST (durability barrier), worker job or bounty-keeper tick triggered it; with the keeper on it looped every ~50s. Also log the breaker's stall age, real pending count and whether a flush was in flight before reconcileAll() re-stages the snapshot and resets the clock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
DevAsign Code Review
No issues found
✅ Merge score: 100/100
3 of 3 acceptance criteria met.
The change fixes the empty-flushPending() wedge by short-circuiting runFlush() before the async body is created (so the finally can't pin flushing to a settled promise), and re-orders breakStall()'s log capture to record the real stall age, pending count and in-flight state before reconcileAll() r
Tests: 1 passed, 2 unverifiable — see the "Tests by DevAsign" comment.
Security: 2 pre-existing security findings touch files in this PR (not introduced by it) — view on the Security page.
Tests by DevAsign✅ 1 of 3 criteria verified by tests, 2 unverifiable. Each verdict below links to its evidence. 1 — After an empty flushPending() following an idle period, the write-through stall breaker does not fire (no false STALLED report is produced for the empty flush). (unverifiable)Verdict: unverifiable Test errored before asserting the criterion: db.insert is not a function threw during setup, so the empty-flush stall-breaker behavior was never exercised. Test: 2 — After an empty flushPending() has occurred, a subsequently staged write is still flushed and persisted on its own flush (the flusher is not left permanently wedged). (unverifiable)Verdict: unverifiable Test errored before asserting the criterion: db.insert is not a function threw during setup, so the post-empty-flush persistence path was never exercised. Test: 3 — When the stall breaker fires, its log reports the real stall age, the actual pending write count, and whether a flush was in flight, captured before the snapshot is re-staged and the clock reset (e.g. 1 pending with flush in flight, not the reconciled snapshot count with ~0s). (pass)Verdict: pass Test passed asserting the stall-breaker log reports the real stall age and backlog rather than the reconciled snapshot. Test: |
Problem
The write-through stall breaker has been firing with
STALLED 0s with 12628 write(s) pending and NO error recorded. On the new Cloud Run service it fired after webhook POSTs that wrote nothing, and since the bounty keeper was enabled it fires every ~50s. Each firing re-writes the full snapshot (~12.6k rows) and rebuilds the pool. The row-version sequence (~754M for a 12.6k-row DB) shows this has been going on for weeks on Render too.Root cause
runFlush()setsflushing = (async () => { try { while (hasPending()) … } finally { flushing = null } })(). With nothing staged, the async body runs to completion synchronously, so thefinallyclearsflushingbefore the outer assignment pins it to a settled promise. From then on everyrunFlush()returns that promise and persists nothing. Nothing errors, and no timeout or watchdog fires, because nothing is running.Anything that calls
flushPending()with an empty backlog triggers it: the durability barrier on a POST that wrote nothing, the worker after a job that wrote nothing, and the bounty keeper at the end of every 12s tick. The breaker then fires at its next check, becauseflushing !== nulland the progress clock is stale. While the flusher is wedged, a write that does get staged waits ~45s, and its POST is acknowledged before it's durable.Changes
runFlush()returns early when there's no pool or nothing is pending, before the async body is created.breakStall()records the stall age, the real pending count and whether a flush was in flight beforereconcileAll()re-stages the snapshot, which also resets the clock. The old log reported the reconciled snapshot and ~0s.Tests
Two new
db-flush.test.tscases, both failing on the old code:flushPending()after idle doesn't trip the breaker, and the next write persists on its own flush.The full backend suite passes on Node 24.11.1 (1852 tests), and
tsc --noEmitis clean.Deploy note
Merging to
maindeploys prod automatically (triggerdevasign-api-main). After deploy, breaker lines should become rare, and a real one will read likeSTALLED 45.2s with N write(s) pending (flush in flight: yes).🤖 Generated with Claude Code