diff --git a/src/controller/persist.c b/src/controller/persist.c index 91f1d704..9b2c93b9 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -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; } diff --git a/src/controller/testsuite/test_commit_durability.c b/src/controller/testsuite/test_commit_durability.c index 3b86fdc6..d62a7a47 100644 --- a/src/controller/testsuite/test_commit_durability.c +++ b/src/controller/testsuite/test_commit_durability.c @@ -975,6 +975,71 @@ static void test_durable_majority_commit_end_to_end(void) printf("test_durable_majority_commit_end_to_end: PASS\n"); } +/* ------------------------------------------------------------------ * + * bug-1564: persist_flush() must fire the durable-advance callback, * + * same as the async fsync_worker does on its own successful flush. * + * * + * persist_flush is the synchronous durability point tests/clean * + * shutdown use in place of waiting for the background flusher. Before * + * this fix it advanced durable_lsn but never notified * + * on_durable_advance -- any WAIT-blocked commit caller relying on that * + * callback would only ever be woken by fsync_worker's OWN next * + * successful flush, never by this call. bug-1563's un-throttled-retry * + * fix (fsync_worker now backs off between failed attempts instead of * + * spinning) is what turned this from a latent, never-triggered gap * + * into a real, deterministic timeout * + * (test_durable_majority_commit_end_to_end, above) -- pre-1563, * + * fsync_worker's busy-spin happened to notice and fire the callback * + * itself within microseconds of any state change, masking the gap. * + * ------------------------------------------------------------------ */ +static _Atomic int g_durable_advance_fire_count; + +static void durable_advance_count_cb(void* ctx) +{ + (void)ctx; + atomic_fetch_add(&g_durable_advance_fire_count, 1); +} + +static void test_persist_flush_fires_durable_advance_callback(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_durflushcb_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + persist_t* p = make_durability_persist(tmp); + TEST_ASSERT_NOT_NULL(p); + + atomic_store(&g_durable_advance_fire_count, 0); + persist_set_durable_cb(p, durable_advance_count_cb, NULL); + + /* Slow background cadence (wal_sync_ms=5000, make_durability_persist) + * means the async fsync_worker will not race in and fire this on its + * own within the test's lifetime -- persist_flush is the only thing + * that can advance durable_lsn here. */ + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p, WAL_JOB_UPDATE, "a", 1)); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&g_durable_advance_fire_count)); + + TEST_ASSERT_EQUAL_INT(0, persist_flush(p)); + + TEST_ASSERT_TRUE_MESSAGE( + atomic_load(&g_durable_advance_fire_count) >= 1, + "persist_flush must fire on_durable_advance when it advances " + "durable_lsn (bug-1564), same as fsync_worker's own successful " + "flush -- otherwise a WAIT-blocked commit caller relying on this " + "callback is never woken by this path"); + + /* A second flush with nothing new to sync must NOT re-fire (mirrors + * fsync_worker's own "only on actual advance" gate). */ + int before_second = atomic_load(&g_durable_advance_fire_count); + TEST_ASSERT_EQUAL_INT(0, persist_flush(p)); + TEST_ASSERT_EQUAL_INT_MESSAGE( + before_second, atomic_load(&g_durable_advance_fire_count), + "a flush with nothing new to sync must not re-fire the callback"); + + teardown_durability_persist(p); + printf("test_persist_flush_fires_durable_advance_callback: PASS\n"); +} + /* ------------------------------------------------------------------ * * Test 5.1b (Req 1.5 / 2.5): losing the sole durable contributor * * leaves the acknowledged op present on a surviving member. * @@ -2517,6 +2582,7 @@ void test_commit_durability(void) RUN_TEST(test_commit_never_exceeds_durable_invariant); /* task 5.1 — durable-majority commit wired end to end */ RUN_TEST(test_durable_majority_commit_end_to_end); + RUN_TEST(test_persist_flush_fires_durable_advance_callback); RUN_TEST(test_sole_durable_contributor_loss_survives); RUN_TEST(test_durability_work_off_iomgr_preserves_liveness); RUN_TEST(test_recover_verdict_clean_intact_log);