From 2de410c877d59d71e19c2f92b47300011d850c9a Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 5 Jul 2026 01:46:38 +0000 Subject: [PATCH] test(raft-apply-decoupling): mixed facts+commands quorum-loss catch-up ordering (task 6.4) Extend test_apply_pump_integration.c with 2 tests, no production changes. Test 3 interleaves commands and facts through a real quorum-loss outage and proves strict original-log-order catch-up via a sequence spy. Test 4 wires a real 4-vcpu vtaskmgr_t to drive 4 commands + 2 facts through one covering ACK, creating genuine N-way concurrent races across task 6.2's self-healing fix rather than one isolated forced pair -- stress-testing bug-1415's fix under a realistic catch-up burst. Closes out task-group 6 (ordered apply pump integration): the pump is now live, hands off command replies correctly, and is proven correct for isolated and mixed/bursty command+fact scenarios under both normal and quorum-loss conditions. --- .kiro/specs/raft-apply-decoupling/tasks.md | 5 +- .../testsuite/test_apply_pump_integration.c | 620 +++++++++++++++++- 2 files changed, 613 insertions(+), 12 deletions(-) diff --git a/.kiro/specs/raft-apply-decoupling/tasks.md b/.kiro/specs/raft-apply-decoupling/tasks.md index 78cab8d..54d81a6 100644 --- a/.kiro/specs/raft-apply-decoupling/tasks.md +++ b/.kiro/specs/raft-apply-decoupling/tasks.md @@ -128,7 +128,7 @@ - _Requirements: 8.4_ - _Depends: 5.2_ -- [ ] 6. Ordered apply pump integration +- [x] 6. Ordered apply pump integration - [x] 6.1 Build the strict-order walk that applies every newly committed entry - Walk entries from the last-applied position up to the current committed position, in ascending order, with no skipping or reordering - Invoke the shared apply dispatch function for every entry in that range, regardless of its commit mode @@ -151,7 +151,7 @@ - _Requirements: 2.2, 4.3, 5.2, 5.3_ - _Depends: 6.2, 4.1_ -- [ ] 6.4 Integration test quorum-loss-then-restore catch-up ordering through the full pump +- [x] 6.4 Integration test quorum-loss-then-restore catch-up ordering through the full pump - Submit a mix of facts and commands while majority is unreachable, then restore majority and confirm every retained entry commits and applies in its original log order with no local-apply escape hatch exercised - Observable: the test demonstrates full, correctly-ordered catch-up after quorum returns, with zero out-of-order application - _Requirements: 6.3, 7.3, 9.4_ @@ -193,6 +193,7 @@ - **1.2**: implemented `commit_resolve()` + wired it into all 6 commit.c-internal paths that can advance `committed_lsn`/`writable_lsn` (the COMMITTED/majority-ack outcome). Chose `VTASK_LIVE_BLOCK` for dispatch (never admission-gated, unlike `VTASK_REQ_BLOCK` which can fast-reject — unacceptable for "exactly-once, never unresolved"). **Reviewer caught a real task-graph gap, not a defect in 1.2 itself**: converting `COMMIT_MODE_WAIT` away from a blocking wait also removes the *only* mechanism that ever detected a commit timing out (the caller's own `pthread_cond_timedwait` deadline) — nothing yet arms an equivalent timeout for async-registered continuations, and nothing yet wires demotion/log-truncation detection to call `commit_resolve(FAILED, ...)`. Without this, a pending async command during quorum loss would hang forever instead of failing — worse than today's behavior. Split out as new tasks **1.3** (timeout arming) and **1.4** (demotion/truncation wiring), inserted before the renumbered **1.5** (was 1.3, concurrency test — now exercises the real detection paths from 1.3/1.4 instead of calling `commit_resolve()` directly). Task 3.2 now also depends on 1.3/1.4, since its own "failure/timeout outcome replies" observable is meaningless without them. - **1.3**: added a dedicated per-commit-engine sweep thread (not the shared `delaymgr` — would violate boundary + delaymgr's 200ms poll granularity vs. `DEFAULT_SYNC_TIMEOUT_MS`=500ms would add timing jitter). Race rule: majority-satisfied always wins over an already-expired deadline when both are true under the same lock hold. **Round 1 REJECTED**: `commit_sweep_once()` computed the next-deadline snapshot under `ce->lock`, unlocked to dispatch, then slept on the *stale* snapshot — a `commit_submit_async()` registration/broadcast landing during that unlocked dispatch window was lost (no thread parked on the condvar to receive it), so the sweep thread could sleep on an absent/late deadline indefinitely (worst case: unbounded delay in a low-traffic quorum-loss scenario, violating the "identical failure timing" no-regression bar). **Fixed** by holding `ce->lock` continuously across scan→wait decisions (releasing only for the unlocked dispatch step) and always re-scanning under the lock immediately before ever calling `cond_wait`/`cond_timedwait` — the standard recheck-then-wait idiom. Round 2 reviewer independently reproduced the round-1 bug shape to confirm the fix actually closes it, not just narrows it. **Precedent for 1.4**: any future code that needs to interrupt the sweep thread's wait (e.g., a demotion/truncation handler resolving a pending LSN out-of-band) must go through the same lock+broadcast discipline `commit_submit_async()` uses, or it risks the identical lost-wakeup class of bug. - **1.4**: added `commit_resolve_pending_above(ce, lsn_floor, rc)` (snapshot-then-`commit_resolve()`-per-entry, no parallel bookkeeping). Truncation: a single new `on_truncate`/`truncate_cb_ctx` callback slot 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 a truncation actually removed entries — 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, `ha_demotion_convergence_fn`'s post-demotion truncate), verified by the reviewer reading all three. Demotion: `raft_cb_become_follower` calls `commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY)` (lsn_floor=0 → fails everything — a demoted node can never authoritatively confirm anything pending; verified against the CP posture). **Known residual, non-blocking**: a fourth internal truncation path (`persist.c` inside `persist_wal_write_replicated`'s tip-conflict-repair branch, STANDBY replicated-apply only) calls `truncate_wal_above_locked()` directly, bypassing the new wrapper — safe today only because demotion resolves ALL pending continuations before role flips to STANDBY (so nothing can still be registered by the time that path runs), but this is an *implicit* ordering invariant, not an asserted one. Flag for whoever next touches the STANDBY replicated-apply path: add a defensive comment/assertion there, or route it through the wrapper too, before relying on it further. Also: bug-1407 was believed closed at this point (70/70 across 3 runs) — **see 1.5's note below, this was a false reading, not a real fix**. +- **6.4**: closes out task-group 6. Approved first round, zero production changes — group ends 2-for-4 on real bugs found (6.1, 6.2), 2-for-4 clean confirmations (6.3, 6.4). Extended `test_apply_pump_integration.c` (task 6.3) with 2 tests covering the MIXED facts+commands scenario 6.3 explicitly deferred here. **Test 3**: genuinely interleaved COMMAND/FACT/COMMAND/FACT/COMMAND sequence through a real `repl_remove_standby` outage, polls for zero commit/apply/reply throughout, restores quorum, and asserts STRICT original-log-order catch-up via a real append-only sequence spy (not a set-membership check) — an adversarial probe (corrupting the expected slot-0 jobid) was independently reproduced by the reviewer to confirm the assertion genuinely discriminates reordering, not just typos. **Test 4 — the highest-value test in this task**: wires a REAL 4-vcpu `vtaskmgr_t` (mirroring task 6.2's bug-1415 regression harness) to drive 4 commands + 2 facts through ONE covering ACK, independently traced by the reviewer to confirm this genuinely creates 5 vtasks (4 continuation dispatches + 1 apply-pump walk) competing for 4 real worker threads — true N-way concurrent racing of task 6.2's self-healing fix, not just one isolated forced pair like 6.2's own test. The "exactly once" check is per-jobid (not an aggregate total), so a skewed double-fire-on-one/hang-on-another distribution would be caught even though the aggregate count would look fine. Adversarial probe (corrupting expected reply count) independently reproduced. **Two honestly-scoped, explicitly non-blocking caveats, not gaps requiring fixes**: (1) task 6.2's own reviewer flagged a narrow, nanosecond-scale TOCTOU residual (unlocked self-heal check vs. locked list-link in `persist_register_pending_apply()`) as a non-blocking fast-follow — Test 4 does NOT deliberately stress this specific window (no injected synchronization primitive forces it, unlike 6.2's own barrier-based test); 50 clean runs across two independent builds are consistent with "didn't hit it by chance," not proof it's closed — stated explicitly here so it isn't assumed covered by this task's "several concurrent races" framing. (2) Test 4 doesn't directly re-assert per-command reply-after-apply ordering the way Test 1 (task 6.3) does via `reply_already_enqueued` — this property holds BY CONSTRUCTION (both the walk's take-call and the self-heal's `lsn <= last_applied_lsn` check only ever fire after `last_applied_lsn` already reflects that entry, verified in the actual code) and is independently covered by task 6.2's own dedicated test, so this is non-blocking, not a real gap. **Task-group 6 final summary**: the ordered apply pump is now live, activated (6.1), correctly hands off command replies (6.2), proven correct for both isolated (6.3) and mixed/bursty (6.4) command+fact scenarios under both normal and quorum-loss conditions — with 2 real, independently-verified, production-severity bugs found and fixed along the way (6.1's job-state corruption from stale receipt-time computation; 6.2's client-hang race from unordered async dispatch), both closed with dedicated regression coverage, the first ever in this spec to wire a real multi-worker `vtaskmgr_t`. **Optional fast-follow noted, not blocking**: a dedicated stress test that deliberately widens the `persist_register_pending_apply()` check-to-lock window (e.g. via a test-only injection point) to reliably exercise 6.2's disclosed residual TOCTOU gap, since neither 6.2's nor 6.4's tests reliably reach it today. - **6.3**: pure integration-test close-out, approved first round, zero production changes — breaks task-group 6's 2-for-2 real-bug streak (6.1, 6.2 each found genuine production-severity bugs; 6.3 confirmed the pump's existing behavior is correct rather than finding a new one). New `test_apply_pump_integration.c` (2 tests) exercises the FULL live pump (real commit engine, real apply dispatch, real hook subscribers — `pool=NULL` like every other fixture, does not overclaim bug-1415/6.2's dispatch-race coverage, which is that test's own job) for both a job submission and a fact. **Key design decision, independently verified true by the reviewer, not just asserted**: `apply_committed_entry()` fires ALL job-state facts via the generic `HOOK_DETAIL_JOB_UPDATE` only (task 2.2's documented deviation, confirmed UNCHANGED by 6.1's `_pad` fix — that fix changed which state-computation core runs, not which hook detail fires) — so `sched_job_start_fun`/`_exit_fun`/`_fail_fun`/apiserver.c's equivalents are structurally unreachable for any live-applied fact, meaning ONE reaction spy wired to `HOOK_DETAIL_JOB_NEW`+`HOOK_DETAIL_JOB_UPDATE` legitimately covers all three named reactions (worker slot release, scheduler dispatch trigger, client/apiserver notify) since they're all downstream of the identical single firing. **Strong independent confirmation this is load-bearing, not incidental**: the type-specific handlers are unconditionally `va_arg`-typed for `job_pidinfo_t*`/`xce_jobfinish_t*`/`job_failinfo_t*` — types `apply_committed_entry()` never passes (always `job_t*`) — so if a type-specific hook ever DID fire from the live pump, it would be an immediate type-confusion crash, not just a coverage gap. Repo-wide re-audit (not just job.c/persist.c) confirms exactly 2 live call sites ever fire these hook details: `apply_committed_entry()` (guarded) and `job_ctrl`'s `mark_job_update` (legitimate SIGCONT/SIGSTOP) — no orphaned third site. Both tests prove REAL ordering, not coincidental timing: reply-after-visibility is sampled synchronously inside the reaction-spy callback (not two separately-timed checks); Req 4.3's "no separate watermark re-check needed" is proven by construction (the walk only ever invokes `apply_committed_entry()` for LSNs already `<= committed_lsn`, so `committed_lsn_at_fire >= entry_lsn` holds structurally, not by scheduling luck) rather than merely asserted in prose. Delayed-commit and genuine quorum-loss windows are both polled repeatedly across real time (5×20ms), not checked once. RED phase independently reproduced by both implementer and reviewer via re-injecting the pre-4.1 eager-mutation bug class. **Task-group 6 running tally**: 3 tasks in, 2 real production-severity bugs found and fixed (6.1 data corruption, 6.2 client-hang race), 1 clean confirmation (6.3) — the pump's core mechanics and hook-routing design are now thoroughly adversarially tested from multiple independent angles. Remaining: 6.4 (quorum-loss catch-up ordering, mixing facts AND commands — 6.3's fact-only quorum-loss scenario is explicitly non-duplicative defense-in-depth per the reviewer, not a preemption of 6.4's charter). - **6.2**: closes the standing operational gap flagged since task 3.2 — successful job submissions finally get a client reply again. Wired `persist_apply_pump_run()` (task 6.1) to call `persist_take_pending_apply(p, lsn, &out_fn, &out_ctx)` immediately after each entry's `apply_committed_entry()` call, invoking the continuation when found — strictly after the mutation, per the task's named observable. Facts need no special-casing: `persist_take_pending_apply()` naturally returns 0 for them since the fact-receipt path (task 4.1) never calls `persist_register_pending_apply()`. Added `jobsubmit_reply_success_once()` (job.c, sibling of the pre-existing `jobsubmit_reply_failure_once()`) — `jobsubmit_apply_complete()` now calls it once every array element resolves, sending `RequestReply_data_jobid` to the client. Took 2 review rounds. **Round 1 REJECTED — a second real, production-severity bug found this task-group, on top of task 6.1's data-corruption bug**: self-disclosed as bug-1415, confirmed by the reviewer via direct code trace: `commit_resolve_pending()` (commit.c) enqueues the WAIT-mode continuation's dispatch and `persist_apply_pump_kick()`'s walk as TWO INDEPENDENT `VTASK_LIVE_BLOCK` vtasks with NO completion-order guarantee — `vdispatcher_entry()` binds each ready task to a different idle vcpu and does not wait for a prior dispatch to complete. Confirmed reachable on EVERY default production deployment (`cluster_init_phase2()` always wires a real `vtaskmgr_t`, default `n_vcpus=8`, never NULL) — and confirmed completely untested by every fixture in this entire spec, all of which use `pool=NULL` (which trivially serializes the two dispatches in enqueue order by falling back to synchronous inline execution, and so can never exercise the race). **Consequence**: if the apply-pump task wins, it applies the entry and advances `last_applied_lsn` past the LSN before the continuation task ever calls `persist_register_pending_apply()` — when that registration finally arrives for an already-passed LSN, nothing will ever come back to consume it. **The job is correctly constructed and visible, but the client hangs forever, plus a leaked `jobsubmit_async_ctx_t`/endpoint ref** — a coin-flip-or-worse failure mode on the feature's own primary success path, for every default multi-vcpu production controller. **Round 2 fixed it** with a self-healing check at the top of `persist_register_pending_apply()`: `if (lsn <= persist_last_applied_lsn(p)) { on_applied(ctx, lsn); return; }` — using the existing `persist_last_applied_lsn()` accessor, no signature change. Verified race-safe in both interleavings: non-racing case is a no-op (confirmed unchanged behavior for every existing test, all of which use `pool=NULL` and so never trigger the check); racing case fires the callback exactly once, immediately, on the registering thread — and can never double-fire because a self-healed registration is never linked onto `pending_applies`, so the walk's own `persist_take_pending_apply()` call for that LSN (which already ran and found nothing, which is precisely how we know the registration is late) has nothing left to find. **First and only test in this entire spec to wire a REAL multi-vcpu `vtaskmgr_t`** (`n_vcpus=4`, via `commit_set_pool()`) — every other fixture in the spec deliberately uses `pool=NULL` and structurally cannot reproduce this class of bug; the new test's poll-barrier ties directly to the real race predicate (`persist_last_applied_lsn(p) < lsn`), not a timing guess, and self-checks its own precondition so it fails loudly rather than silently degrading if the forced interleaving doesn't occur. RED→GREEN independently reproduced by both rounds' reviewers. **One narrow, non-blocking residual finding for a fast-follow, not required to close bug-1415**: the self-heal check (an unlocked atomic read) and the subsequent list-link (under `pending_applies_lock`) are not one atomic unit — a sufficiently long preemption between the check and the lock could theoretically still reproduce a vastly narrower version of the original race. Recommended fix: perform the self-heal check while already holding `pending_applies_lock` (the same lock `persist_take_pending_apply()` uses), making check-and-link atomic with respect to the walk's take. Not blocking since the reported, reproduced, production-relevant (dispatch-level) race is genuinely and completely closed. **Two pre-existing tests in test_commit_barrier_reply.c still fail as EXPECTED** (unchanged from task 6.1 — same root cause, task 7.3's job). **Task-group-6 running tally: 2 of 2 tasks landed so far have each required a full REJECTED→fixed round for a genuine, independently-verified production-severity bug** (6.1: job-state corruption from stale receipt-time fact computation; 6.2: a client-hang race from unordered async dispatch) — this task group is where deferred-apply's real concurrency implications are surfacing, exactly as expected for the task that finally activates the live path. Remaining: 6.3/6.4 (integration tests through the full pump) should be reviewed with the same level of scrutiny for further concurrency surprises. - **6.1**: the most consequential task in the entire spec — this is what finally makes `apply_committed_entry()` live (previously only restart replay, task 2.3, ever called it). Built `persist_apply_pump_run()` (walks `[last_applied_lsn+1..committed_lsn]` strictly ascending, calls `apply_committed_entry()` for every entry regardless of commit mode) and `persist_apply_pump_kick()` (off-thread trigger via `vtask_enq_classified(VTASK_LIVE_BLOCK)`, inline fallback for pool-less fixtures, CAS-coalesced so at most one caller runs the walk at a time — the loser hands off asynchronously, by design, to whichever thread wins). **Design deviation, verified correct and a strict superset**: wired the trigger into `commit_resolve_pending()` (commit.c) rather than only ha.c's `on_commit_advanced` — task 1.2 already discovered `commit_resolve_pending` is the single choke point all 6 internal `committed_lsn`-advancing paths funnel through (verified: exactly 6 call sites, commit.c:353/558/1042/1060/1099/1116, all firing after `ce->lock` releases), while `on_commit_advanced` covers only 1 of them; wiring only there would silently starve the pump for single-node/self-quorum clusters. design.md should be updated to reflect this as the actual integration point. **Fixed Hazard 1 (double-append, flagged since task 4.1)**: a naive `detail==HOOK_DETAIL_JOB_UPDATE` filter would have broken `job_ctrl`'s legitimate live SIGCONT/SIGSTOP append (same hook detail, still-legitimate command path) — fixed instead with a `_Thread_local bool t_apply_seam_notifying` call-provenance guard set only around `apply_committed_entry()`'s own hook-firing calls; `persist_job_hook_fun`/`persist_queue_hook_fun`/`persist_worker_hook_fun` early-return when set. Also caught and fixed an UNDOCUMENTED instance of the identical hazard class in the queue/worker hook functions (neither had ANY mode-based early-return before this, unlike job's pre-existing dead `WAL_MODE_WAIT` branch) — would have double-appended+re-triggered a nested majority-commit wait for every live-applied `WAL_QUEUE_CTRL`/`WAL_WORKER_CTRL` entry. Took 2 review rounds. **Round 1 REJECTED — the most severe finding of the entire session, a real data-corruption bug, not a doc/log issue**: Hazard 2 (merge-staleness, also flagged since 4.1) was investigated and ESCALATED as requiring wire-format changes beyond scope, while still landing the pump that activates it. Round-1 review proved with field-level precision this was wrong to ship: job state DONE, correctly baked by a FINISH fact — followed shortly after (an entirely ordinary HPC lifecycle event, not adversarial) by a POST_EXEC-stage FAIL fact whose receipt-time snapshot is STILL stale (RUN, since FINISH hadn't applied yet, apply now being deferred) — bakes `RUN|EXIT|PERR` into its WAL entry. Both apply in correct LSN order (FINISH then FAIL), but FAIL's stale absolute overwrite wins over the now-correct DONE, because the terminal/non-terminal guard doesn't catch it (both values happen to carry a terminal-ish bit). **Final result: a job's correct DONE completion silently replaced by RUN|EXIT|PERR — a Req 11.3 regression, and a real, operator-and-billing-visible corruption newly introduced by deferring apply (this feature's whole purpose), reachable under completely ordinary traffic.** The reviewer also found a scoped fix contradicting the "needs wire-format enrichment" escalation: `wal_jobstate_t`'s existing unused `_pad` field had room. **Round 2 fixed it properly**: `_pad` now carries a discriminator (`WAL_JOB_FAIL_STAGE_FLAG`/`WAL_JOB_UPDATE_POSTDONE_MARK`) plus the minimal data needed (FAIL's `stage`) to RECOMPUTE against the job's ACTUAL CURRENT state AT APPLY TIME via the real `apply_job_fail_transition()`/new `apply_job_postdone_transition()` guard cores — no more baking a stale absolute value at receipt time. `WAL_JOB_START`/`WAL_JOB_FINISH` are unchanged (confirmed safe on their own — deterministic, state-independent payloads that were never the problem, only the thing getting clobbered). Backward-compatible: `_pad==0` (all pre-existing/legacy data) deterministically routes to the same absolute-overwrite behavior that was universal before this fix — no crash, no new corruption, correctly out of scope for further hardening since this feature has never been deployed. **Bonus finding during round-2 review — a real test-flakiness hazard, root-caused and fixed on the spot, not just reported**: a rare (initially estimated ~2%, actually verified ~7.5%) flake in the new regression test was root-caused to a genuine SECOND, independent kicker of the apply pump (`commit_on_durable_advance`, fired from the real `fsync_worker` background thread on every `durable_lsn` advance, entirely separate from `repl_on_ack`) that the test's own header comment didn't account for — this is by-design eventually-consistent behavior (task 6.2's continuation-firing will correctly rely on it), not a production bug, but the TEST needed a proper poll-for-applied-state helper instead of asserting immediately. Fixed (`tap_wait_applied()` polling helper added to 2 tests) and a SECOND, related flake found during the investigation in a third test, also fixed — 120/120 clean runs afterward. Also fixed a stale orphaned forward-declaration left over from the round-2 rename. **Also fixed a genuine, unrelated bug found via regression sweep, logged as bug-1409**: an early version of `apply_pump_drain`'s retry loop didn't check `persist_apply_pump_run`'s return code, causing an unbounded busy-loop (18.5M+ log lines) in `test_demotion_convergence` whenever `committed_lsn` advances without a corresponding real on-disk WAL entry (a legitimate scenario for commit-engine-focused unit fixtures) — fixed by returning immediately on a negative rc. **Hand-off built for task 6.2**: `int persist_take_pending_apply(persist_t* p, uint64_t lsn, pending_apply_notify_fn* out_fn, void** out_ctx)` — call AFTER `apply_committed_entry()` applies `lsn`; returns 1+populates once (atomically unlinking) when a registration exists (the command path, task 3.2), 0 otherwise (the common case, facts never register); a second call for the same LSN is guaranteed to return 0; safe to call unconditionally for every applied LSN. Task 6.2 should call this right after each apply inside/after the walk and invoke the returned continuation — deliberately NOT wired into the walk itself, per boundary. **Two pre-existing tests in test_commit_barrier_reply.c now fail as EXPECTED (not a regression)**: their own assertion text literally says "no job may be constructed until task 6's apply pump exists" — now correctly invalidated by this task landing; explicitly task 7.3's job to update, per task 3.2's own Implementation Notes anticipating exactly this. **Minor, non-blocking coverage gap for a fast-follow**: no test explicitly replays a `WAL_JOB_FAIL`/`WAL_JOB_UPDATE` entry with a real `_pad` marker through the REPLAY path specifically (only through live apply) — low risk given `apply_committed_entry()` is the literal shared function both paths call, but untested for that specific combination. diff --git a/src/controller/testsuite/test_apply_pump_integration.c b/src/controller/testsuite/test_apply_pump_integration.c index 4092df4..26ff014 100644 --- a/src/controller/testsuite/test_apply_pump_integration.c +++ b/src/controller/testsuite/test_apply_pump_integration.c @@ -1,4 +1,4 @@ -/* test_apply_pump_integration.c — raft-apply-decoupling task 6.3 +/* test_apply_pump_integration.c — raft-apply-decoupling tasks 6.3 and 6.4 * * Integration coverage for command AND fact apply/notify timing through the * SAME full pump (task 6.1's ordered walk + task 6.2's continuation @@ -6,6 +6,19 @@ * (which predated the pump and could only pin "no phantom job", not * "notified only after commit"). * + * Tests 1-2 are task 6.3's (command+fact interleave through one commit; fact + * reactions withheld/restored across a genuine outage). Tests 3-4 are task + * 6.4's: a genuine quorum-loss-then-restore catch-up over a MIX of commands + * AND facts (Test 2 above is fact-only, by design — see task 6.1's + * Implementation Notes calling that out as non-duplicative of 6.4's + * charter), plus a real-multi-vcpu-pool burst variant that stress-tests task + * 6.2's bug-1415 self-heal (persist_register_pending_apply) under several + * concurrent command continuations racing the walk at once, not just the + * one forced interleaving test_apply_pump.c's own regression test covers. + * See Test 3 and Test 4's own header comments below for the full scope + * split and why NULL-pool determinism vs a real pool each earn their own + * test rather than one doing both jobs. + * * Two things this file does NOT re-prove, because they are already covered * elsewhere in this exact spec's own suite: * - Pure command reply-after-visibility ordering for job submit — @@ -72,13 +85,16 @@ * * Fixture mirrors test_apply_pump.c's tap_setup (real persist_init + * persist_start over a private tmpdir, a real 3-member commit_engine_t with - * two stub standbys, ACKs delivered explicitly via repl_on_ack, commit_ - * set_pool never called so the apply pump's NULL-pool inline fallback - * applies) plus test_commit_barrier_reply.c's reply-observation harness - * (a zeroed, no-iomgr requester endpoint; xce_reply's packets stay queued - * on ep->outq and are parsed with the raw capnp reader). + * two stub standbys, ACKs delivered explicitly via repl_on_ack) plus + * test_commit_barrier_reply.c's reply-observation harness (a zeroed, + * no-iomgr requester endpoint; xce_reply's packets stay queued on ep->outq + * and are parsed with the raw capnp reader). tpi_setup() takes a + * pool_n_vcpus parameter (task 6.4): 0 (Tests 1-3) leaves ce->pool == NULL + * so the apply pump's inline fallback applies, exactly as tasks 6.3's own + * tests always ran; > 0 (Test 4 only) wires a REAL vtaskmgr_t, mirroring + * test_apply_pump.c's task 6.2 tpr_setup. * - * Both tests below follow this series' established disciplines: capture + * All four tests below follow this series' established disciplines: capture * every assertion-relevant observable into locals, then teardown * unconditionally, THEN assert (a failed TEST_ASSERT's longjmp must never * skip teardown); poll for eventual apply-pump consistency rather than @@ -113,6 +129,7 @@ #include "persist.h" #include "repl.h" #include "request.h" +#include "vtaskmgr.h" #include "wal_types.h" #include "xce/cluster_def.h" #include "xce/types.h" @@ -128,6 +145,16 @@ static commit_engine_t g_tpi_hce; static int g_tpi_hep_a, g_tpi_hep_b; /* dummy standby endpoints */ static char g_tpi_tmpdir[256]; +/* task 6.4: a REAL multi-vcpu pool, wired only when a test explicitly asks + * for one via tpi_setup's pool_n_vcpus parameter -- every fixture through + * task 6.3 keeps ce->pool == NULL (see test_apply_pump.c's task 6.2 bug-1415 + * regression test header for why that trivially serializes commit_resolve_ + * pending()'s two independent dispatches and so cannot reproduce that race + * class). g_tpi_pool_active tracks whether tpi_teardown owes a + * vtaskmgr_fini this run. */ +static vtaskmgr_t g_tpi_pool; +static int g_tpi_pool_active; + /* Test-injectable send hook (repl.c, XCE_TESTING) — keeps repl_broadcast * from dereferencing the dummy standby endpoints above. */ extern int (*repl_broadcast_send_hook)(void *ep, wal_catchup_entry_t **entries, @@ -147,11 +174,23 @@ static int tpi_drop_send_hook(void *ep, wal_catchup_entry_t **entries, static endpoint_t g_tpi_req_ep; static ep_private_t g_tpi_req_priv; +/* task 6.4: bounded cap on how many individual replies tpi_reply_obs_t can + * distinguish by jobid/status -- separate from TPI_SPY_CAP below since the + * two arrays serve different structs; sized generously above the largest + * batch this file's tests submit. */ +#define TPI_REPLY_CAP 8 + typedef struct { int n_replies; int32_t last_status; int32_t last_data_which; uint64_t last_jobid; + /* task 6.4: per-reply jobid/status, in receipt order, so a mixed-batch + * test can confirm EVERY submitted command got its own exactly-one + * reply rather than only inspecting the last one observed. Unused by + * tasks 6.3's tests, which only ever check n_replies/last_*. */ + uint64_t jobids[TPI_REPLY_CAP]; + int32_t statuses[TPI_REPLY_CAP]; } tpi_reply_obs_t; static void tpi_scan_replies(endpoint_t *ep, tpi_reply_obs_t *obs) @@ -170,6 +209,12 @@ static void tpi_scan_replies(endpoint_t *ep, tpi_reply_obs_t *obs) if (rq.body_which == Request_body_replymsg) { struct RequestReply rr; read_RequestReply(&rr, rq.body.replymsg); + if (obs->n_replies < TPI_REPLY_CAP) { + obs->jobids[obs->n_replies] = + (rr.data_which == RequestReply_data_jobid) ? rr.data.jobid + : 0; + obs->statuses[obs->n_replies] = rr.status; + } obs->n_replies += 1; obs->last_status = rr.status; obs->last_data_which = rr.data_which; @@ -182,6 +227,26 @@ static void tpi_scan_replies(endpoint_t *ep, tpi_reply_obs_t *obs) } } +/* task 6.4: true iff `jobid` appears in obs's captured replies exactly once, + * with a success (0) status -- catches both a hang (0 occurrences, since a + * missing reply never appears) and a bug-1415-class double-fire (>1 + * occurrence) with one call, for the mixed-batch tests below. */ +static bool tpi_reply_has_exactly_one_success(const tpi_reply_obs_t *obs, + uint64_t jobid) +{ + int count = 0; + int cap = obs->n_replies < TPI_REPLY_CAP ? obs->n_replies : TPI_REPLY_CAP; + + for (int i = 0; i < cap; i++) { + if (obs->jobids[i] == jobid) { + if (obs->statuses[i] != 0) + return false; + count++; + } + } + return count == 1; +} + /* ── Reaction spy: stands in for sched.c's sched_job_update_fun / * apiserver.c's apiserver_job_update_fun (the real, only-reachable-for- * facts subscribers — see file header). Captures, at the EXACT moment the @@ -263,7 +328,13 @@ static void tpi_spy_reset(void) memset(g_tpi_spy, 0, sizeof(g_tpi_spy)); } -static void tpi_setup(void) +/* pool_n_vcpus == 0 (every task-6.3 test, and task 6.4's deterministic + * ordering test) keeps ce->pool == NULL exactly as before -- no behavior + * change for any existing caller. pool_n_vcpus > 0 (task 6.4's burst test + * only) wires a REAL vtaskmgr_t, mirroring test_apply_pump.c's task 6.2 + * tpr_setup, so commit_resolve_pending()'s two independent dispatches run + * as genuine concurrent vtasks instead of the inline fallback. */ +static void tpi_setup(int pool_n_vcpus) { kvmgr_params_t str_params = { .key_type = KVMGR_KEY_STRING, .nslots = 32, @@ -326,6 +397,14 @@ static void tpi_setup(void) repl_set_commit_engine(&g_tpi_c.persist.repl, &g_tpi_hce); repl_broadcast_send_hook = tpi_drop_send_hook; + g_tpi_pool_active = 0; + if (pool_n_vcpus > 0) { + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_init(&g_tpi_pool, pool_n_vcpus)); + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_start(&g_tpi_pool)); + commit_set_pool(&g_tpi_hce, &g_tpi_pool); + g_tpi_pool_active = 1; + } + TEST_ASSERT_EQUAL_INT(0, xce_add_hook(&g_tpi_c.hooksys, XCE_CONTROLLER_HOOK_JOB, tpi_reaction_spy, &g_tpi_c)); @@ -340,6 +419,13 @@ static void tpi_teardown(void) tpi_scan_replies(&g_tpi_req_ep, &drain); /* free any leftover packets */ repl_broadcast_send_hook = NULL; commit_engine_fini(&g_tpi_hce); + if (g_tpi_pool_active) { + /* Mirrors test_apply_pump.c's tpr_teardown ordering exactly: + * vtaskmgr_fini AFTER commit_engine_fini (so nothing can dispatch a + * new vtask onto a pool mid-teardown) but BEFORE persist_stop. */ + vtaskmgr_fini(&g_tpi_pool); + g_tpi_pool_active = 0; + } persist_stop(&g_tpi_c.persist); persist_fini(&g_tpi_c.persist); @@ -408,6 +494,26 @@ static void tpi_wait_applied(uint64_t lsn, uint32_t deadline_ms) } } +/* task 6.4: waits until at least `want_n` replies have been observed on the + * requester endpoint, accumulating across repeated tpi_scan_replies calls + * (each call only drains packets newly enqueued since the last one). Needed + * specifically for the real-pool burst test: tpi_wait_applied only proves + * the WALK finished, not that every command's continuation hand-off has + * fired -- a hand-off that self-heals late (persist_register_pending_apply, + * bug-1415) runs on its OWN vtask thread, independently of and potentially + * after the walk. */ +static void tpi_wait_n_replies(tpi_reply_obs_t *obs, int want_n, + uint32_t deadline_ms) +{ + for (uint32_t waited = 0; waited < deadline_ms; waited += 5) { + tpi_scan_replies(&g_tpi_req_ep, obs); + if (obs->n_replies >= want_n) + return; + usleep(5000); + } + tpi_scan_replies(&g_tpi_req_ep, obs); /* one last drain past the deadline */ +} + static void tpi_sleep_ms(uint32_t ms) { struct timespec ts; @@ -468,7 +574,7 @@ static void test_pump_command_and_fact_interleave_through_same_commit(void) const uint64_t jobid_x = 9500; /* submitted via handle_jobsubmit */ const uint64_t jobid_y = 9501; /* pre-existing, receives a START fact */ - tpi_setup(); + tpi_setup(0); job_t *jy = tpi_seed_job(jobid_y, JOB_STAT_PEND); int seeded_ok = (jy != NULL); @@ -630,7 +736,7 @@ static void test_pump_fact_reactions_withheld_during_outage_then_fire_after_rest { const uint64_t jobid_z = 9600; - tpi_setup(); + tpi_setup(0); job_t *jz = tpi_seed_job(jobid_z, JOB_STAT_PEND); int seeded_ok = (jz != NULL); @@ -751,6 +857,498 @@ static void test_pump_fact_reactions_withheld_during_outage_then_fire_after_rest "after_restore: PASS\n"); } +/* ═══════════════════════════════════════════════════════════════════════ + * Test 3 — task 6.4: MIXED facts AND commands through a genuine quorum-loss- + * then-restore window, proving strict original-log-order catch-up. + * ═══════════════════════════════════════════════════════════════════════ + * + * Distinguishing scope vs this file's Test 2 above (task 6.3) and + * test_commit_barrier_reply.c's test_fact_state_unchanged_throughout_ + * quorum_loss_outage (task 4.2), both of which drive a genuine repl_remove_ + * standby outage with FACTS ONLY: this test interleaves THREE job-submit + * COMMANDS with TWO facts, five entries total, appended in COMMAND, FACT, + * COMMAND, FACT, COMMAND order specifically so a bug that grouped all + * commands before all facts (or vice versa) -- rather than genuinely + * preserving append order -- would be caught by the strict per-slot + * sequence assertion below, not merely "everything eventually applied, in + * some order." Requirements 6.3 (retained entries commit/apply in original + * log order) and 7.3 (never applies ahead of an earlier-committing entry) + * are the direct target; 9.4 is checked negatively -- the outage must never + * spuriously resolve a registered command continuation as a FAILURE (the + * demotion/truncation path that requirement's other clause governs), only + * ever as a genuine, later, COMMITTED success once quorum returns. + * + * Uses the standard NULL-pool fixture (tpi_setup(0)) deliberately, exactly + * like this file's other two tests: with ce->pool == NULL, commit_resolve_ + * pending()'s continuation dispatch and the apply-pump kick both fall back + * to their synchronous inline path (dispatch_continuation, apply_pump_drain + * -- see commit.c), so the ENTIRE catch-up (register, apply, notify, reply) + * happens deterministically on this test's own thread inside the single + * repl_on_ack() call below. That determinism is what makes the strict, + * order-sensitive sequence assertion below meaningful rather than flaky -- + * Test 4 below separately re-verifies the same ordering invariant survives + * genuine concurrency, and additionally stress-tests task 6.2's bug-1415 + * self-heal under a real multi-entry burst. */ +static void test_pump_mixed_facts_and_commands_catchup_original_order(void) +{ + const uint64_t jobid_a = 9700; /* command 1 */ + const uint64_t jobid_w = 9701; /* pre-existing, receives both facts */ + const uint64_t jobid_b = 9702; /* command 2 */ + const uint64_t jobid_c = 9703; /* command 3 */ + + tpi_setup(0); + + job_t *jw = tpi_seed_job(jobid_w, JOB_STAT_PEND); + int seeded_ok = (jw != NULL); + + /* Genuine quorum loss -- same mechanism as this file's Test 2 (task + * 6.3) and test_commit_barrier_reply.c's task 4.2 test: both standbys + * removed, self alone is 1 of 3, never a majority. */ + repl_remove_standby(&g_tpi_c.persist.repl, &g_tpi_hep_a); + repl_remove_standby(&g_tpi_c.persist.repl, &g_tpi_hep_b); + + pthread_mutex_lock(&g_tpi_c.persist.lock); + uint64_t lsn_before = g_tpi_c.persist.current_lsn; + pthread_mutex_unlock(&g_tpi_c.persist.lock); + + /* Append order: COMMAND, FACT, COMMAND, FACT, COMMAND -- deliberately + * NOT grouped by type. */ + tpi_arrange_jobsubmit(jobid_a); + int submit_a_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + static unsigned int w_pids[1] = { 7201 }; + job_pidinfo_t w_start = { .jobid = jobid_w, + .npids = 1, + .pids = &w_pids[0] }; + int start_rc = on_job_start(&g_tpi_c, &w_start); + + tpi_arrange_jobsubmit(jobid_b); + int submit_b_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + xce_jobfinish_t w_finish = { .jobid = jobid_w, .exit_code = 0 }; + int finish_rc = on_job_finish(&g_tpi_c, &w_finish); + + tpi_arrange_jobsubmit(jobid_c); + int submit_c_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + pthread_mutex_lock(&g_tpi_c.persist.lock); + uint64_t watermark = g_tpi_c.persist.current_lsn - 1; + pthread_mutex_unlock(&g_tpi_c.persist.lock); + + /* ── Outage window: poll repeatedly across real time -- all five + * entries are durably appended and retained, but structurally cannot + * commit while both standbys remain disconnected (Req 6.1). ── */ + enum { OUTAGE_POLLS = 5, OUTAGE_INTERVAL_MS = 20 }; + bool outage_a_absent[OUTAGE_POLLS]; + bool outage_b_absent[OUTAGE_POLLS]; + bool outage_c_absent[OUTAGE_POLLS]; + uint32_t outage_w_state[OUTAGE_POLLS]; + int outage_spy_n[OUTAGE_POLLS]; + uint64_t outage_committed[OUTAGE_POLLS]; + int outage_replies[OUTAGE_POLLS]; + + for (int i = 0; i < OUTAGE_POLLS; i++) { + tpi_sleep_ms(OUTAGE_INTERVAL_MS); + outage_a_absent[i] = (tpi_find_job(jobid_a) == NULL); + outage_b_absent[i] = (tpi_find_job(jobid_b) == NULL); + outage_c_absent[i] = (tpi_find_job(jobid_c) == NULL); + outage_w_state[i] = seeded_ok ? jw->job.state : 0; + outage_spy_n[i] = g_tpi_spy_n; + outage_committed[i] = atomic_load(&g_tpi_c.persist.committed_lsn); + tpi_reply_obs_t obs = { 0 }; + tpi_scan_replies(&g_tpi_req_ep, &obs); + outage_replies[i] = obs.n_replies; + } + + /* ── Restore quorum and deliver ONE covering ACK past all five + * entries. ── */ + TEST_ASSERT_TRUE( + repl_add_standby(&g_tpi_c.persist.repl, &g_tpi_hep_a, "sb_a", 7700) >= 0); + TEST_ASSERT_TRUE( + repl_add_standby(&g_tpi_c.persist.repl, &g_tpi_hep_b, "sb_b", 7701) >= 0); + tpi_wait_durable(watermark, 2000); + repl_on_ack(&g_tpi_c.persist.repl, &g_tpi_hep_a, watermark); + tpi_wait_applied(watermark, 2000); + + tpi_reply_obs_t final_obs = { 0 }; + tpi_scan_replies(&g_tpi_req_ep, &final_obs); + + uint64_t committed_final = atomic_load(&g_tpi_c.persist.committed_lsn); + job_t *ja_final = tpi_find_job(jobid_a); + job_t *jb_final = tpi_find_job(jobid_b); + job_t *jc_final = tpi_find_job(jobid_c); + uint32_t jw_state_final = seeded_ok ? jw->job.state : 0; + int spy_n_final = g_tpi_spy_n; + tpi_spy_entry_t spy_snap[TPI_SPY_CAP]; + memcpy(spy_snap, g_tpi_spy, sizeof(spy_snap)); + + tpi_teardown(); + + /* Arrangement sanity */ + TEST_ASSERT_TRUE(seeded_ok); + TEST_ASSERT_EQUAL_INT(0, submit_a_rc); + TEST_ASSERT_EQUAL_INT(0, start_rc); + TEST_ASSERT_EQUAL_INT(0, submit_b_rc); + TEST_ASSERT_EQUAL_INT(0, finish_rc); + TEST_ASSERT_EQUAL_INT(0, submit_c_rc); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_before + 4, watermark, + "exactly five entries (3 commands, 2 facts) must durably append " + "during the outage"); + + for (int i = 0; i < OUTAGE_POLLS; i++) { + TEST_ASSERT_TRUE_MESSAGE(outage_committed[i] < lsn_before, + "the outage must still be genuinely in effect at this poll -- " + "committed_lsn must not have advanced past any retained " + "entry"); + TEST_ASSERT_TRUE_MESSAGE(outage_a_absent[i], + "command A's job must not exist before its entry commits, at " + "every poll of the outage window (Req 2.1) -- no local-apply " + "escape hatch exists"); + TEST_ASSERT_TRUE_MESSAGE(outage_b_absent[i], + "command B's job must not exist before its entry commits, at " + "every poll of the outage window (Req 2.1)"); + TEST_ASSERT_TRUE_MESSAGE(outage_c_absent[i], + "command C's job must not exist before its entry commits, at " + "every poll of the outage window (Req 2.1)"); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_PEND, outage_w_state[i], + "job W's facts must not mutate live state before their " + "entries commit, at every poll of the outage window (Req 6.1, " + "6.2)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, outage_spy_n[i], + "no reaction for any of the five retained entries may fire " + "while quorum is unreachable, at every poll (Req 5.3, 6.1, " + "6.2)"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, outage_replies[i], + "no client reply for any of the three retained commands may " + "be observable while quorum is unreachable -- confirms the " + "outage never spuriously resolves a registered continuation " + "as a failure either (Req 9.4)"); + } + + /* After the covering ACK: all five entries committed and applied. */ + TEST_ASSERT_TRUE_MESSAGE(committed_final >= watermark, + "a covering ACK after reconnect must advance committed_lsn over " + "the entire retained suffix"); + TEST_ASSERT_NOT_NULL_MESSAGE(ja_final, + "command A's job must be visible once its entry commits and " + "applies"); + TEST_ASSERT_NOT_NULL_MESSAGE(jb_final, + "command B's job must be visible once its entry commits and " + "applies"); + TEST_ASSERT_NOT_NULL_MESSAGE(jc_final, + "command C's job must be visible once its entry commits and " + "applies"); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_DONE, jw_state_final, + "job W must reflect both applied facts (START then FINISH) once " + "quorum is restored and its entries commit"); + + /* The actual new ground this test exists for: strict, original-log- + * order catch-up across a MIX of commands and facts -- not grouped by + * type, not reordered by any priority. A bug that applied all commands + * before all facts (or vice versa) would still pass a "some order" + * check but would fail this exact per-slot sequence assertion. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(5, spy_n_final, + "exactly five reactions must fire: one per retained entry, none " + "lost, none duplicated"); + TEST_ASSERT_EQUAL_INT_MESSAGE(HOOK_DETAIL_JOB_NEW, spy_snap[0].detail, + "slot 0 (lowest lsn) must be command A's submit -- original " + "append order"); + TEST_ASSERT_EQUAL_UINT64(jobid_a, spy_snap[0].jobid); + TEST_ASSERT_EQUAL_INT_MESSAGE(HOOK_DETAIL_JOB_UPDATE, spy_snap[1].detail, + "slot 1 must be W's START fact, applying SECOND even though it is " + "a fact sandwiched between two commands"); + TEST_ASSERT_EQUAL_UINT64(jobid_w, spy_snap[1].jobid); + TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, spy_snap[1].state_at_fire); + TEST_ASSERT_EQUAL_INT_MESSAGE(HOOK_DETAIL_JOB_NEW, spy_snap[2].detail, + "slot 2 must be command B's submit -- a bug that grouped both " + "facts before both remaining commands would place W's FINISH here " + "instead"); + TEST_ASSERT_EQUAL_UINT64(jobid_b, spy_snap[2].jobid); + TEST_ASSERT_EQUAL_INT_MESSAGE(HOOK_DETAIL_JOB_UPDATE, spy_snap[3].detail, + "slot 3 must be W's FINISH fact"); + TEST_ASSERT_EQUAL_UINT64(jobid_w, spy_snap[3].jobid); + TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE, spy_snap[3].state_at_fire); + TEST_ASSERT_EQUAL_INT_MESSAGE(HOOK_DETAIL_JOB_NEW, spy_snap[4].detail, + "slot 4 (highest lsn) must be command C's submit -- last in, last " + "applied"); + TEST_ASSERT_EQUAL_UINT64(jobid_c, spy_snap[4].jobid); + + for (int i = 0; i < 5; i++) { + uint64_t entry_lsn = lsn_before + (uint64_t)i; + TEST_ASSERT_TRUE_MESSAGE( + spy_snap[i].committed_lsn_at_fire >= entry_lsn, + "each reaction must fire only once committed_lsn already " + "covers its own lsn -- no separate watermark check is ever " + "needed (Req 4.3)"); + } + + /* Every retained command's continuation resolves as a genuine, later + * success -- never a spurious failure from the outage itself (Req 9.3, + * 9.4). */ + TEST_ASSERT_EQUAL_INT_MESSAGE(3, final_obs.n_replies, + "exactly three replies must be observable: one per retained " + "command, none lost, none duplicated"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_a), + "command A must receive exactly one successful reply"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_b), + "command B must receive exactly one successful reply"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_c), + "command C must receive exactly one successful reply"); + + printf("test_pump_mixed_facts_and_commands_catchup_original_order: " + "PASS\n"); +} + +/* ═══════════════════════════════════════════════════════════════════════ + * Test 4 — task 6.4: a REAL multi-entry catch-up burst on a genuine + * multi-vcpu pool, stress-confirming task 6.2's bug-1415 self-healing fix + * under realistic concurrency rather than one forced interleaving. + * ═══════════════════════════════════════════════════════════════════════ + * + * test_apply_pump.c's own task 6.2 regression test (test_apply_pump_late_ + * pending_apply_registration_self_heals) already proves the fix for ONE + * specific, artificially-forced interleaving on a single lsn, deliberately + * blocking the continuation until the walk has already passed it. That is + * necessary but not sufficient evidence for THIS task's charter: a genuine + * quorum-loss-then-restore catch-up commits a whole BATCH of retained + * entries at once, which fires MULTIPLE independent continuation-dispatch + * vtasks concurrently against the SAME single walk instance -- N + * independent race instances in flight together, not one. This test drives + * that real scenario: four job-submit commands and two facts, all durably + * retained during a genuine repl_remove_standby outage, released by ONE + * covering ACK on a REAL 4-vcpu vtaskmgr_t (unlike every other fixture in + * this file and spec, which keep ce->pool == NULL and so structurally + * cannot reproduce this race class at all -- see test_apply_pump.c's task + * 6.2 test header for why). + * + * Two invariants must both hold under this real concurrency, not just when + * pool == NULL: + * 1. The apply-pump walk itself is single-threaded per persist_t (CAS- + * coalesced kicks, `p->apply_pump_lock`) -- so the six reactions must + * STILL fire in strict original append order, exactly as Test 3 above + * proves for the NULL-pool case. Re-asserting the identical per-slot + * sequence here proves that invariant survives four independent + * continuation vtasks concurrently racing the walk, not just the + * deterministic inline-fallback path. + * 2. Every one of the four commands must receive EXACTLY one successful + * reply -- zero (a bug-1415-class permanent hang) and more than one + * (a double-fire) are both failures this test is built to catch, + * whichever side of the per-lsn race actually occurs on a given run. */ +static void test_pump_burst_catchup_real_pool_no_hang_no_double_fire(void) +{ + const int N_VCPUS = 4; /* > 1 is the whole point -- see header */ + const uint64_t jobid_d = 9800; /* command 1 */ + const uint64_t jobid_w2 = 9810; /* pre-existing, receives both facts */ + const uint64_t jobid_e = 9801; /* command 2 */ + const uint64_t jobid_f = 9802; /* command 3 */ + const uint64_t jobid_g = 9803; /* command 4 */ + + tpi_setup(N_VCPUS); + + job_t *jw2 = tpi_seed_job(jobid_w2, JOB_STAT_PEND); + int seeded_ok = (jw2 != NULL); + + repl_remove_standby(&g_tpi_c.persist.repl, &g_tpi_hep_a); + repl_remove_standby(&g_tpi_c.persist.repl, &g_tpi_hep_b); + + pthread_mutex_lock(&g_tpi_c.persist.lock); + uint64_t lsn_before = g_tpi_c.persist.current_lsn; + pthread_mutex_unlock(&g_tpi_c.persist.lock); + + /* Append order: D, W-START, E, F, W-FINISH, G -- two commands back to + * back (E, F) to also stress adjacent-command dispatch concurrency. */ + tpi_arrange_jobsubmit(jobid_d); + int submit_d_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + static unsigned int w2_pids[1] = { 7301 }; + job_pidinfo_t w2_start = { .jobid = jobid_w2, + .npids = 1, + .pids = &w2_pids[0] }; + int start_rc = on_job_start(&g_tpi_c, &w2_start); + + tpi_arrange_jobsubmit(jobid_e); + int submit_e_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + tpi_arrange_jobsubmit(jobid_f); + int submit_f_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + xce_jobfinish_t w2_finish = { .jobid = jobid_w2, .exit_code = 0 }; + int finish_rc = on_job_finish(&g_tpi_c, &w2_finish); + + tpi_arrange_jobsubmit(jobid_g); + int submit_g_rc = + handle_jobsubmit(&g_tpi_c, &g_tpi_req_ep, &g_tpi_jobsub_req); + + pthread_mutex_lock(&g_tpi_c.persist.lock); + uint64_t watermark = g_tpi_c.persist.current_lsn - 1; + pthread_mutex_unlock(&g_tpi_c.persist.lock); + + enum { OUTAGE_POLLS = 5, OUTAGE_INTERVAL_MS = 20 }; + bool outage_d_absent[OUTAGE_POLLS]; + bool outage_g_absent[OUTAGE_POLLS]; + uint32_t outage_w2_state[OUTAGE_POLLS]; + int outage_spy_n[OUTAGE_POLLS]; + uint64_t outage_committed[OUTAGE_POLLS]; + int outage_replies[OUTAGE_POLLS]; + + for (int i = 0; i < OUTAGE_POLLS; i++) { + tpi_sleep_ms(OUTAGE_INTERVAL_MS); + outage_d_absent[i] = (tpi_find_job(jobid_d) == NULL); + outage_g_absent[i] = (tpi_find_job(jobid_g) == NULL); + outage_w2_state[i] = seeded_ok ? jw2->job.state : 0; + outage_spy_n[i] = g_tpi_spy_n; + outage_committed[i] = atomic_load(&g_tpi_c.persist.committed_lsn); + tpi_reply_obs_t obs = { 0 }; + tpi_scan_replies(&g_tpi_req_ep, &obs); + outage_replies[i] = obs.n_replies; + } + + TEST_ASSERT_TRUE( + repl_add_standby(&g_tpi_c.persist.repl, &g_tpi_hep_a, "sb_a", 7700) >= 0); + TEST_ASSERT_TRUE( + repl_add_standby(&g_tpi_c.persist.repl, &g_tpi_hep_b, "sb_b", 7701) >= 0); + tpi_wait_durable(watermark, 2000); + repl_on_ack(&g_tpi_c.persist.repl, &g_tpi_hep_a, watermark); + tpi_wait_applied(watermark, 3000); + + /* The walk finishing (tpi_wait_applied above) does not itself prove + * every command's client reply has fired -- see file header: a LATE- + * registering continuation's hand-off fires on its OWN vtask thread, + * independently of and potentially after the walk. Poll separately, + * bounded, for all four replies. */ + tpi_reply_obs_t final_obs = { 0 }; + tpi_wait_n_replies(&final_obs, 4, 3000); + + uint64_t committed_final = atomic_load(&g_tpi_c.persist.committed_lsn); + uint64_t applied_final = persist_last_applied_lsn(&g_tpi_c.persist); + job_t *jd_final = tpi_find_job(jobid_d); + job_t *je_final = tpi_find_job(jobid_e); + job_t *jf_final = tpi_find_job(jobid_f); + job_t *jg_final = tpi_find_job(jobid_g); + uint32_t jw2_state_final = seeded_ok ? jw2->job.state : 0; + int spy_n_final = g_tpi_spy_n; + tpi_spy_entry_t spy_snap[TPI_SPY_CAP]; + memcpy(spy_snap, g_tpi_spy, sizeof(spy_snap)); + + int pending_d = persist_has_pending_apply(&g_tpi_c.persist, lsn_before); + int pending_e = + persist_has_pending_apply(&g_tpi_c.persist, lsn_before + 2); + int pending_f = + persist_has_pending_apply(&g_tpi_c.persist, lsn_before + 3); + int pending_g = + persist_has_pending_apply(&g_tpi_c.persist, lsn_before + 5); + + tpi_teardown(); + + TEST_ASSERT_TRUE(seeded_ok); + TEST_ASSERT_EQUAL_INT(0, submit_d_rc); + TEST_ASSERT_EQUAL_INT(0, start_rc); + TEST_ASSERT_EQUAL_INT(0, submit_e_rc); + TEST_ASSERT_EQUAL_INT(0, submit_f_rc); + TEST_ASSERT_EQUAL_INT(0, finish_rc); + TEST_ASSERT_EQUAL_INT(0, submit_g_rc); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_before + 5, watermark, + "exactly six entries (4 commands, 2 facts) must durably append " + "during the outage"); + + for (int i = 0; i < OUTAGE_POLLS; i++) { + TEST_ASSERT_TRUE_MESSAGE(outage_committed[i] < lsn_before, + "the outage must still be genuinely in effect at this poll"); + TEST_ASSERT_TRUE_MESSAGE(outage_d_absent[i], + "command D's job must not exist before its entry commits, at " + "every poll of the outage window"); + TEST_ASSERT_TRUE_MESSAGE(outage_g_absent[i], + "command G's job must not exist before its entry commits, at " + "every poll of the outage window"); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_PEND, outage_w2_state[i], + "job W2's facts must not mutate live state before their " + "entries commit, at every poll of the outage window"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, outage_spy_n[i], + "no reaction for any of the six retained entries may fire " + "while quorum is unreachable, at every poll"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, outage_replies[i], + "no client reply for any of the four retained commands may be " + "observable while quorum is unreachable"); + } + + TEST_ASSERT_TRUE_MESSAGE(committed_final >= watermark, + "a covering ACK after reconnect must advance committed_lsn over " + "the entire retained suffix"); + TEST_ASSERT_TRUE_MESSAGE(applied_final >= watermark, + "the apply pump must fully catch up through the retained suffix"); + TEST_ASSERT_NOT_NULL(jd_final); + TEST_ASSERT_NOT_NULL(je_final); + TEST_ASSERT_NOT_NULL(jf_final); + TEST_ASSERT_NOT_NULL(jg_final); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_DONE, jw2_state_final, + "job W2 must reflect both applied facts once quorum is restored"); + + /* Invariant 1 (see file header): the walk is single-threaded per + * persist_t regardless of pool -- strict original append order must + * survive real concurrent continuation dispatch, not just the + * deterministic NULL-pool case Test 3 above covers. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(6, spy_n_final, + "exactly six reactions must fire: one per retained entry, none " + "lost, none duplicated, even under real concurrent dispatch"); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_NEW, spy_snap[0].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_d, spy_snap[0].jobid); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_UPDATE, spy_snap[1].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_w2, spy_snap[1].jobid); + TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, spy_snap[1].state_at_fire); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_NEW, spy_snap[2].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_e, spy_snap[2].jobid); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_NEW, spy_snap[3].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_f, spy_snap[3].jobid); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_UPDATE, spy_snap[4].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_w2, spy_snap[4].jobid); + TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE, spy_snap[4].state_at_fire); + TEST_ASSERT_EQUAL_INT(HOOK_DETAIL_JOB_NEW, spy_snap[5].detail); + TEST_ASSERT_EQUAL_UINT64(jobid_g, spy_snap[5].jobid); + + /* Invariant 2 (see file header): task 6.2's bug-1415 self-heal must + * resolve EVERY command's continuation exactly once, whichever side of + * the walk-vs-registration race actually occurred for it on this run + * -- neither a hang (fewer than 4 replies observed within the bounded + * poll) nor a double-fire (more than 4, or any jobid appearing twice) + * is acceptable. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(4, final_obs.n_replies, + "exactly four replies must be observable: one per retained " + "command -- fewer means a bug-1415-class hang, more means a " + "double-fire"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_d), + "command D must receive exactly one successful reply"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_e), + "command E must receive exactly one successful reply"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_f), + "command F must receive exactly one successful reply"); + TEST_ASSERT_TRUE_MESSAGE( + tpi_reply_has_exactly_one_success(&final_obs, jobid_g), + "command G must receive exactly one successful reply"); + + /* No orphaned registration may remain linked on pending_applies for + * any of the four commands once every reply has fired (mirrors + * test_apply_pump.c's task 6.2 regression test's own final check). */ + TEST_ASSERT_EQUAL_INT(0, pending_d); + TEST_ASSERT_EQUAL_INT(0, pending_e); + TEST_ASSERT_EQUAL_INT(0, pending_f); + TEST_ASSERT_EQUAL_INT(0, pending_g); + + printf("test_pump_burst_catchup_real_pool_no_hang_no_double_fire: " + "PASS\n"); +} + /* ── Test group entry point ───────────────────────────────────────────── */ void test_apply_pump_integration(void) @@ -759,5 +1357,7 @@ void test_apply_pump_integration(void) RUN_TEST(test_pump_command_and_fact_interleave_through_same_commit); RUN_TEST( test_pump_fact_reactions_withheld_during_outage_then_fire_after_restore); + RUN_TEST(test_pump_mixed_facts_and_commands_catchup_original_order); + RUN_TEST(test_pump_burst_catchup_real_pool_no_hang_no_double_fire); UNITY_END(); }