fix(controller): persist_flush now fires durable-advance callback (bug-1564)

persist_flush()'s synchronous fdatasync path advanced durable_lsn on
success but never called on_durable_advance() -- only the async
fsync_worker did. This gap was previously masked by bug-1563's own
busy-spin (a fast accidental repoll covered for the missing
notification); fixing that spin unmasked it, causing
test_durable_majority_commit_end_to_end to fail deterministically
(3/3 runs).

Fixed by firing on_durable_advance() from persist_flush() too,
whenever it genuinely advances durable_lsn. Fixed via TDD (RED test
before the fix, GREEN after) and re-validated with a fresh 40-round
sequential + chaotic HA soak (both pass cleanly, zero fdatasync
errors, zero divergence/loss).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-17 10:20:24 +00:00
parent c3980cbfb3
commit 986a330472
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
2 changed files with 92 additions and 1 deletions

View file

@ -1525,12 +1525,15 @@ int persist_flush(persist_t* p)
* disk. Monotonic max; preserves durable_lsn <= current_lsn - 1 (tip read
* under the same p->lock). Gated on rc == 0 (fail-loud, Req 1.1/1.2): a
* failed flush must not advance the watermark. */
bool advanced = false;
if (rc == 0 && !flusher_fsync_forced_fail(p)) {
uint64_t tip = (p->current_lsn > 0) ? p->current_lsn - 1 : 0;
uint64_t dur = atomic_load_explicit(&p->durable_lsn,
memory_order_acquire);
if (tip > dur)
if (tip > dur) {
atomic_store_explicit(&p->durable_lsn, tip, memory_order_release);
advanced = true;
}
} else if (rc != 0 || flusher_fsync_forced_fail(p)) {
int e = (rc != 0) ? errno : EIO;
ERROR("persist: persist_flush fdatasync FAILED (errno=%d %s) — "
@ -1538,6 +1541,28 @@ int persist_flush(persist_t* p)
e, strerror(e));
}
pthread_mutex_unlock(&p->lock);
/* bug-1564: fire the same durable-advance wake fsync_worker fires on its
* own successful flush (identical gate: only when durable_lsn actually
* advanced; identical lock discipline: strictly after releasing p->lock,
* so the callback -> ce->lock ordering has no p->lock held underneath
* it, matching fsync_worker's own documented cycle-free reasoning
* above). Latent gap unmasked by bug-1563: this synchronous flush path
* (tests, clean shutdown) used to advance durable_lsn silently -- any
* WAIT-blocked commit caller relying on this callback to know a fresh
* durable-majority exists would only ever notice via the ASYNC
* fsync_worker's own next successful flush, not this call. Before
* bug-1563's fix, fsync_worker's un-throttled retry loop happened to
* paper over the gap (it re-checked and re-fired within microseconds of
* any state change); bug-1563's backoff correctly removed that spin,
* which is what surfaced this as a real, reproducible test timeout
* (test_durable_majority_commit_end_to_end: commit_submit(WAIT) timing
* out at -110/ETIMEDOUT, deterministic 3/3 direct runs) instead of a
* pre-existing gap nothing had ever pushed hard enough to expose. */
if (advanced && p->on_durable_advance != NULL) {
p->on_durable_advance(p->durable_cb_ctx);
}
return rc;
}