feat(raft-apply-decoupling): resolve pending continuations on demotion/truncation (task 1.4)
commit_resolve_pending_above(ce, lsn_floor, rc) fails every registered continuation for lsn > lsn_floor, reusing commit_resolve()'s existing exactly-once/idempotent guarantee (no parallel bookkeeping path). Truncation: a new on_truncate/truncate_cb_ctx callback on persist_t (same inversion-of-control shape as the existing on_durable_advance hook) fires from persist_truncate_wal_above after releasing p->lock, only when entries were actually removed. This one wrapper covers all three real truncation call sites: raft_cb_become_leader's post-election truncate, the library's on_truncate_from conflict-reconcile, and ha_demotion_convergence_fn's post-demotion truncate. Demotion: raft_cb_become_follower calls commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY) -- lsn_floor=0 fails everything, since a demoted node can never authoritatively confirm anything still pending (CP posture). Without this, a demoted or truncated node would leave any pending async command permanently unresolved -- the requester would simply never get a reply, worse than today's bounded-timeout behavior. Also closes bug-1407 (pre-existing test_config_repl flake, unrelated to this feature) -- confirmed fixed as a side effect of prior tasks in this series, 70/70 passing across 3 independent runs.
This commit is contained in:
parent
530a02bda3
commit
9b4b805a27
9 changed files with 748 additions and 301 deletions
|
|
@ -63,6 +63,7 @@
|
|||
#include "persist.h" /* persist_t, persist_truncate_wal_above */
|
||||
#include "repl.h" /* repl_add_standby / repl_set_commit_engine */
|
||||
#include "xce/cluster_def.h" /* full cluster_t layout */
|
||||
#include "xce/errnox.h" /* XCE_ERR_STANDBY (task 1.4 resolve rc) */
|
||||
|
||||
#ifdef XCE_TESTING
|
||||
|
||||
|
|
@ -271,6 +272,27 @@ static int dc_run_pending_task(delaymgr_t *mgr)
|
|||
return rc;
|
||||
}
|
||||
|
||||
/* ── Continuation recorder (raft-apply-decoupling task 1.4) ───────────────
|
||||
* Minimal local recorder for commit_submit_async continuations, mirroring
|
||||
* test_commit_resolve.c's continuation_recorder_t/recorder_fn pattern (kept
|
||||
* as a separate file-static copy -- no cross-TU sharing of test statics). */
|
||||
typedef struct {
|
||||
_Atomic int calls;
|
||||
uint64_t last_lsn;
|
||||
commit_outcome_t last_outcome;
|
||||
int last_rc;
|
||||
} dc_continuation_recorder_t;
|
||||
|
||||
static void dc_recorder_fn(void *ctx, uint64_t lsn, commit_outcome_t outcome,
|
||||
int rc)
|
||||
{
|
||||
dc_continuation_recorder_t *r = (dc_continuation_recorder_t *)ctx;
|
||||
r->last_lsn = lsn;
|
||||
r->last_outcome = outcome;
|
||||
r->last_rc = rc;
|
||||
atomic_fetch_add(&r->calls, 1);
|
||||
}
|
||||
|
||||
/* ── Tests ──────────────────────────────────────────────────────────────── */
|
||||
|
||||
/* 1.1: demotion from PRIMARY disables the persistence hooks synchronously in
|
||||
|
|
@ -376,6 +398,66 @@ static void test_self_restart_failure_invokes_fatal_hook(void)
|
|||
printf("test_self_restart_failure_invokes_fatal_hook: checked\n");
|
||||
}
|
||||
|
||||
/* raft-apply-decoupling task 1.4 (Req 1.3/1.4, 9.3/9.4): demotion from
|
||||
* PRIMARY must resolve every currently registered commit continuation as
|
||||
* FAILED -- otherwise a client/fact waiting on a pending commit is left
|
||||
* permanently unresolved once this node can never commit it. Registers TWO
|
||||
* continuations at different uncommitted LSNs to prove the resolve loop
|
||||
* handles the whole pending set, not just a single slot.
|
||||
* RED: before task 1.4, raft_cb_become_follower's was-PRIMARY branch never
|
||||
* calls into the commit engine, so both continuations stay pending forever
|
||||
* (calls == 0). */
|
||||
static void test_demotion_from_primary_resolves_pending_continuations_failed(void)
|
||||
{
|
||||
dc_setup("resolve", /*n_peers=*/3, /*n_standbys=*/2,
|
||||
/*appended_lsn=*/9, /*n_vcpus=*/8);
|
||||
atomic_store(&g_dc.persist.committed_lsn, 4);
|
||||
|
||||
dc_continuation_recorder_t low = {0};
|
||||
dc_continuation_recorder_t high = {0};
|
||||
|
||||
/* Both lsns sit above committed_lsn=4 (majority is unreachable in this
|
||||
* fixture -- standbys never ACK -- so both stay genuinely pending, not
|
||||
* already resolved COMMITTED by the time we demote). */
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
commit_submit_async(&g_dc.commit, 5, dc_recorder_fn, &low));
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
commit_submit_async(&g_dc.commit, 9, dc_recorder_fn, &high));
|
||||
|
||||
/* Act: demotion edge from a LIVE PRIMARY. This fixture never wires a
|
||||
* vtaskmgr pool (commit_set_pool is never called), so commit_resolve's
|
||||
* dispatch falls back to the inline fallback -- the recorder runs
|
||||
* synchronously on this thread, no polling required. */
|
||||
raft_cb_become_follower_with_cluster_for_test(&g_dc, /*term=*/7);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&low.calls),
|
||||
"demotion must resolve every pending continuation, including one "
|
||||
"for an lsn that could still reach majority under normal operation "
|
||||
"(Req 1.3/1.4/9.4)");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE((int)COMMIT_OUTCOME_FAILED,
|
||||
(int)low.last_outcome, "demotion must resolve as FAILED, not COMMITTED");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-XCE_ERR_STANDBY, low.last_rc,
|
||||
"demotion must use the existing -XCE_ERR_STANDBY vocabulary (the "
|
||||
"same code the HA dispatch gate already uses to reject writes to a "
|
||||
"non-primary)");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(5, low.last_lsn,
|
||||
"the recorder must observe the lsn it was registered for");
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&high.calls),
|
||||
"demotion must resolve the uncommitted-tail continuation as FAILED "
|
||||
"exactly once");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE((int)COMMIT_OUTCOME_FAILED,
|
||||
(int)high.last_outcome, "demotion must resolve as FAILED, not COMMITTED");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-XCE_ERR_STANDBY, high.last_rc,
|
||||
"demotion must use the existing -XCE_ERR_STANDBY vocabulary");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(9, high.last_lsn,
|
||||
"the recorder must observe the lsn it was registered for");
|
||||
|
||||
dc_teardown();
|
||||
printf("test_demotion_from_primary_resolves_pending_continuations_failed: "
|
||||
"checked\n");
|
||||
}
|
||||
|
||||
/* Regression: demotion from STANDBY must NOT truncate, NOT restart, NOT disable
|
||||
* hooks beyond today's behavior — byte-equivalent to the pre-spec path. A
|
||||
* STANDBY has no registered hooks and no convergence work. */
|
||||
|
|
@ -814,6 +896,7 @@ void test_demotion_convergence(void)
|
|||
printf("\n>> demotion convergence + self-restart tests (RED, task 2.1)\n\n");
|
||||
|
||||
RUN_TEST(test_demotion_from_primary_disables_hooks);
|
||||
RUN_TEST(test_demotion_from_primary_resolves_pending_continuations_failed);
|
||||
RUN_TEST(test_demotion_schedules_convergence_truncate_and_restart);
|
||||
RUN_TEST(test_self_restart_failure_invokes_fatal_hook);
|
||||
RUN_TEST(test_demotion_from_standby_no_truncation_no_restart);
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@
|
|||
#include "persist.h"
|
||||
#include "wal_types.h"
|
||||
#include "config.h"
|
||||
#include "commit.h" /* raft-apply-decoupling task 1.4: commit_engine_t,
|
||||
* commit_submit_async, commit_outcome_t */
|
||||
|
||||
/* ── Forward declaration of function-under-test (not yet implemented) ─────── */
|
||||
extern int persist_truncate_wal_above(persist_t *p, uint64_t cutoff_lsn);
|
||||
|
|
@ -230,6 +232,149 @@ static void test_truncate_noop_at_current_lsn(void)
|
|||
printf("test_truncate_noop_at_current_lsn: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Test: truncation resolves invalidated commit continuations (task 1.4) ── */
|
||||
|
||||
/* Minimal local recorder for commit_submit_async continuations -- mirrors
|
||||
* test_commit_resolve.c's continuation_recorder_t/recorder_fn (kept as a
|
||||
* separate file-static copy; no cross-TU sharing of test statics). */
|
||||
typedef struct {
|
||||
_Atomic int calls;
|
||||
uint64_t last_lsn;
|
||||
commit_outcome_t last_outcome;
|
||||
int last_rc;
|
||||
} trunc_continuation_recorder_t;
|
||||
|
||||
static void trunc_recorder_fn(void *ctx, uint64_t lsn, commit_outcome_t outcome,
|
||||
int rc)
|
||||
{
|
||||
trunc_continuation_recorder_t *r = (trunc_continuation_recorder_t *)ctx;
|
||||
r->last_lsn = lsn;
|
||||
r->last_outcome = outcome;
|
||||
r->last_rc = rc;
|
||||
atomic_fetch_add(&r->calls, 1);
|
||||
}
|
||||
|
||||
/* raft-apply-decoupling task 1.4 (Req 1.3/1.4, 9.3/9.4): a real truncation
|
||||
* through persist_truncate_wal_above must resolve every commit continuation
|
||||
* registered ABOVE the new tail as FAILED -- those entries no longer exist
|
||||
* in the log and can never commit -- while a continuation registered AT OR
|
||||
* BELOW the new tail is left untouched (its entry is still safely in the
|
||||
* log and may still reach majority).
|
||||
*
|
||||
* Uses n_peers=3 with NO standbys registered so majority is permanently
|
||||
* unreachable (majority_candidate_locked's zero-fill keeps committed_lsn at
|
||||
* 0 regardless of local append/durable progress) -- every appended entry
|
||||
* stays genuinely pending, giving a stable LSN range to register
|
||||
* continuations against without racing the commit engine's own majority
|
||||
* evaluation.
|
||||
*
|
||||
* RED: before task 1.4, persist_truncate_wal_above never calls into the
|
||||
* commit engine, so all three continuations stay pending forever (calls ==
|
||||
* 0 for all). */
|
||||
static void test_truncate_above_resolves_pending_continuations_as_failed(void)
|
||||
{
|
||||
char tmpdir[] = "/tmp/xce-trunc-resolve-XXXXXX";
|
||||
TEST_ASSERT_NOT_NULL(mkdtemp(tmpdir));
|
||||
|
||||
persist_t *p = make_fresh_persist(tmpdir);
|
||||
TEST_ASSERT_NOT_NULL(p);
|
||||
|
||||
commit_engine_t ce;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
commit_engine_init(&ce, p, /*n_peers=*/3, /*n_vcpus=*/8));
|
||||
|
||||
int dummy = 0;
|
||||
for (int i = 0; i < 5; i++)
|
||||
persist_wal_sync_append(p, WAL_JOB_SUBMIT, &dummy, sizeof(dummy), NULL);
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(6, p->current_lsn,
|
||||
"fixture: 5 entries appended, current_lsn must be 6");
|
||||
|
||||
trunc_continuation_recorder_t below = {0};
|
||||
trunc_continuation_recorder_t above1 = {0};
|
||||
trunc_continuation_recorder_t above2 = {0};
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, commit_submit_async(&ce, 2, trunc_recorder_fn, &below));
|
||||
TEST_ASSERT_EQUAL_INT(0, commit_submit_async(&ce, 4, trunc_recorder_fn, &above1));
|
||||
TEST_ASSERT_EQUAL_INT(0, commit_submit_async(&ce, 5, trunc_recorder_fn, &above2));
|
||||
|
||||
/* Act: truncate above lsn=3 -- entries 4 and 5 are removed. */
|
||||
int rc = persist_truncate_wal_above(p, 3);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(4, p->current_lsn,
|
||||
"current_lsn must collapse to cutoff_lsn + 1");
|
||||
|
||||
/* Assert: continuations ABOVE the new tail resolve FAILED exactly once.
|
||||
* No vtaskmgr pool is wired on this engine (commit_set_pool never
|
||||
* called), so commit_resolve's dispatch falls back to the inline
|
||||
* fallback -- synchronous with the persist_truncate_wal_above call
|
||||
* above, no polling required. */
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&above1.calls),
|
||||
"a continuation above the new tail must resolve on truncation");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE((int)COMMIT_OUTCOME_FAILED,
|
||||
(int)above1.last_outcome, "truncation must resolve as FAILED");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-ENOENT, above1.last_rc,
|
||||
"truncation must use the existing -ENOENT vocabulary already "
|
||||
"exercised in test_commit_resolve.c (task 1.2)");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(4, above1.last_lsn,
|
||||
"the recorder must observe the lsn it was registered for");
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&above2.calls),
|
||||
"a second continuation above the new tail must also resolve");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE((int)COMMIT_OUTCOME_FAILED,
|
||||
(int)above2.last_outcome, "truncation must resolve as FAILED");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-ENOENT, above2.last_rc,
|
||||
"truncation must use the existing -ENOENT vocabulary");
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(5, above2.last_lsn,
|
||||
"the recorder must observe the lsn it was registered for");
|
||||
|
||||
/* Assert: a continuation AT OR BELOW the new tail is left untouched --
|
||||
* its entry is still in the log and may still reach majority. */
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&below.calls),
|
||||
"a continuation at/below the new tail must NOT be resolved by "
|
||||
"truncation -- only entries strictly above the new tail were "
|
||||
"removed from the log");
|
||||
|
||||
commit_engine_fini(&ce);
|
||||
teardown_persist(p);
|
||||
printf("test_truncate_above_resolves_pending_continuations_as_failed: PASS\n");
|
||||
}
|
||||
|
||||
/* Regression: a no-op truncate (cutoff at/above the current tail) must NOT
|
||||
* fire the truncation wake -- a pending continuation for any lsn must stay
|
||||
* untouched, since nothing was actually removed from the log. */
|
||||
static void test_truncate_noop_does_not_resolve_pending_continuations(void)
|
||||
{
|
||||
char tmpdir[] = "/tmp/xce-trunc-resolve-noop-XXXXXX";
|
||||
TEST_ASSERT_NOT_NULL(mkdtemp(tmpdir));
|
||||
|
||||
persist_t *p = make_fresh_persist(tmpdir);
|
||||
TEST_ASSERT_NOT_NULL(p);
|
||||
|
||||
commit_engine_t ce;
|
||||
TEST_ASSERT_EQUAL_INT(0,
|
||||
commit_engine_init(&ce, p, /*n_peers=*/3, /*n_vcpus=*/8));
|
||||
|
||||
int dummy = 0;
|
||||
persist_wal_sync_append(p, WAL_JOB_SUBMIT, &dummy, sizeof(dummy), NULL);
|
||||
persist_wal_sync_append(p, WAL_JOB_SUBMIT, &dummy, sizeof(dummy), NULL);
|
||||
|
||||
trunc_continuation_recorder_t rec = {0};
|
||||
TEST_ASSERT_EQUAL_INT(0, commit_submit_async(&ce, 2, trunc_recorder_fn, &rec));
|
||||
|
||||
/* Truncate AT the current boundary -- a no-op (nothing above cutoff). */
|
||||
uint64_t cutoff = p->current_lsn - 1;
|
||||
int rc = persist_truncate_wal_above(p, cutoff);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&rec.calls),
|
||||
"a no-op truncate (nothing above cutoff_lsn) must not resolve any "
|
||||
"pending continuation");
|
||||
|
||||
commit_engine_fini(&ce);
|
||||
teardown_persist(p);
|
||||
printf("test_truncate_noop_does_not_resolve_pending_continuations: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Test group entry point ────────────────────────────────────────────────── */
|
||||
|
||||
void test_persist_truncate(void)
|
||||
|
|
@ -240,5 +385,7 @@ void test_persist_truncate(void)
|
|||
RUN_TEST(test_truncate_idempotent);
|
||||
RUN_TEST(test_truncate_updates_applied_lsn);
|
||||
RUN_TEST(test_truncate_noop_at_current_lsn);
|
||||
RUN_TEST(test_truncate_above_resolves_pending_continuations_as_failed);
|
||||
RUN_TEST(test_truncate_noop_does_not_resolve_pending_continuations);
|
||||
UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue