bug-1433: raft_cb_become_leader runs synchronously off peer_ctrl_dispatch_to_pool, which hardcodes VTASK_LIVENESS for every inbound peer-control packet including vote replies. The bug-1431 round-2 fix (flusher_flush_once force-flush in persist_promotion_advance_committed) was reachable from that never-blocks task class -- same bug family as bug-1198. Removed the force-flush; round-3 (durable-capped only) plus the already-shipped bug-1432 immediate-first-heartbeat fix close the visibility-latency gap non-blockingly. TRACKED RESIDUAL: persist_apply_pump_run_sync is still reachable on the same VTASK_LIVENESS path and does bounded pread() -- lower risk (page-cache-hot) but strictly still blocking I/O; not safe to remove outright (reopens bug-1043-family job loss), needs its own follow-up (reclassify peer_ctrl_classify's winning-vote-reply case or dispatch promotion tail onto a fresh VTASK_LIVE_BLOCK vtask). bug-1434: persist_promotion_advance_committed and ha_standby_advance_committed_and_apply raced on persist_t.committed_lsn via an unlocked load-then-conditional-store during the promotion window (before ha->role flips to CTRL_PRIMARY) -- a losing racer could regress committed_lsn. Added persist_committed_lsn_advance_atomic (CAS retry loop, lock-free) and routed both call sites through it. Validated: 19/19 unit test suites, 6-round chaotic soak (0 loss/divergence) + 6-round sequential+leadership-coverage soak, both PASS. Also folds in raft-apply-decoupling and raft-ha-core-safety validate-impl phase bumps to "implemented" and clears config-upgrade-snapshot's now-satisfied entity-bearing-snapshots block.
16 KiB
Implementation Plan
TDD is mandatory (project rule): add the failing test/scenario first, then the change (red → green → refactor). Almost all work is in the single test client TU
src/controller/testsuite/test.c, so tasks are sequential by design; only the soak conversion touches a different file. Behavioral tests are driven throughxce_testitself (piped scripts), gating on its exit code.
-
1. Foundation: verdict, loop wrapper, ergonomics
-
1.1 Session failure tracking + process exit code + abort/continue + comment/blank handling
- Add file-scope state (
g_session_failed,g_continue_on_error,g_last_status) and apreprocess_and_run(line)wrapper used by the main read loop: ignore blank and#-comment lines; run normal lines viaexecute_line; updateg_last_status/g_session_failedfrom the result; in default mode stop on first failure, undercontinue-on-errorkeep going.mainreturnsg_session_failed ? 1 : 0. Extendcmd_setwith acontinue-on-errortoggle. - Observable: a piped script with a bogus/failing command makes
xce_testexit non-zero; an all-valid script exits 0; withset continue-on-errora failing line does not stop later lines but the final exit is still non-zero;#/blank lines are ignored. - Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 6.1, 6.2
- Boundary: VerdictLoopWrapper
- Add file-scope state (
-
1.2
echocommand- Add
cmd_echo(table-registered) that prints its argument message and returns success. - Observable:
echo helloemitshelloand the command succeeds (exit unaffected). - Requirements: 6.3
- Boundary: VerdictLoopWrapper
- Depends: 1.1
- Add
-
2. Shell-escape
-
2.1
!<cmd>runs a shell command and feeds$?- In
preprocess_and_run, when the first non-whitespace char is!, run the remainder viasystem(); setg_last_status = WEXITSTATUS(rc)(non-zero onsystemfailure); a non-zero status is a command failure (subject to 1.1's abort/continue + exit code). - Observable:
!truesucceeds and sets$?=0;!falsefails the session and sets$?≠0; a script can!kill .../!bash start.sh ...to drive node lifecycle. - Requirements: 2.1, 2.2, 2.3, 2.4
- Boundary: ShellEscape
- Depends: 1.1
- In
-
3. Auto-variable substitution
-
3.1 Substitute the fixed auto-vars
$?,$jobid,$jobcount- Add
substitute_autovars(line)(run before tokenizing inpreprocess_and_run): replace whole-token$?→g_last_status,$jobid→g_last_jobid,$jobcount→g_last_jobcount. A reference to an unset$jobid/$jobcountfails the command (no garbage substitution). No other variable syntax is recognized. - Observable: after a command,
echo $?prints the prior status; referencing$jobidbefore any successfuljobsubfails that command rather than substituting an empty/garbage value. - Requirements: 3.1, 3.3, 3.5, 3.6
- Boundary: AutoVars
- Depends: 1.1
- Add
-
4. Correct job-submission reporting (bug-786) +
$jobidcapture -
4.1
cmd_jobsubreports success only on a clean ack with a valid JobId- Gate the
jobsubmit ok: JobId: <id>report on a successful reply status AND a well-formed JobId; on success setg_last_jobid(feeds$jobid, 3.2). Otherwise print a typed error (redirect / no-master / not-ready / timeout) and return<0. Never print "ok" with an uninitialized/garbage id (63-15--1-...). - Observable: a submit that does not get a clean ack returns failure and does NOT set
$jobid;jobsubnever prints success with a malformed id; a successful submit sets$jobidto the acked JobId. - Requirements: 7.1, 7.2, 7.3, 3.2
- Boundary: JobsubFix
- Depends: 1.1, 3.1
- Gate the
-
5. Job-count capture and assertions
-
5.1
cmd_joblistsets$jobcount; extract a shared count/contains helper- Refactor
cmd_joblistto expose an internal helper that returns the current job count and tests membership of a JobId; setg_last_jobcountafter ajoblist. - Observable: after
joblist,echo $jobcountprints the number of jobs returned; the helper is reusable by the assertions. - Requirements: 3.4
- Boundary: JoblistCount
- Depends: 1.1, 3.1
- Refactor
-
5.2
expect-jobs Nassertion- Add
cmd_expect_jobs: pass when the current job count equalsN; otherwise print expected vs actual and return<0. - Observable:
expect-jobs <correct>succeeds;expect-jobs <wrong>fails the session and prints expected/actual. - Requirements: 5.1, 5.2
- Boundary: Assertions
- Depends: 5.1
- Add
-
5.3
expect-job <jobid>assertion- Add
cmd_expect_job: pass when the JobId is present in the job list; otherwise print the missing id and return<0. Works with$jobidsubstitution. - Observable:
expect-job $jobidpasses for a submitted job;expect-job <absent>fails the session and prints the missing id. - Requirements: 5.3, 5.4
- Boundary: Assertions
- Depends: 5.1, 3.1, 4.1
- Add
-
6. Wait for elected primary
-
6.1
wait-primary [timeout_ms]- Add
cmd_wait_primary: loop a cheap, failover-aware client op (the existingjoblist/probe path viaactive_ctx) until it succeeds or the timeout elapses (monotonic clock); default timeout when the arg is omitted; return 0 on success,<0on timeout. - Observable: against a healthy cluster
wait-primaryreturns immediately; after a primary kill it blocks until a new primary serves, then succeeds; with no primary within the timeout it fails. - Requirements: 4.1, 4.2, 4.3, 4.4
- Boundary: WaitPrimary
- Depends: 1.1
- Add
-
7. Reference E2E driver
-
7.1 Convert
test-commit-durability.shto use the primitives- Replace the prose-grep + sanitizer-filter logic with piped
xce_testscripts usingwait-primary,jobsub/$jobid,expect-jobs/expect-job, and!-shell-escape for the primary kill/restart; gate each phase on thexce_testexit code. Preserve the rolling-kill + zero-loss coverage; add a!-driven cold-restart-all +expect-jobsstep. - Observable: the converted soak runs end-to-end and reports pass/fail via
xce_testexit codes (no joblist prose parsing); rolling-kill zero-loss coverage is retained; the cold-restart step asserts all committed jobs recover. - Requirements: 8.1, 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 1.1, 2.1, 4.1, 5.2, 5.3
- Replace the prose-grep + sanitizer-filter logic with piped
-
7.2 Run the converted soak on a live
/xce-deploy-hacluster- Execute the converted soak against a real 3-node cluster (real fsync-able fs) and confirm pass.
- Observable: soak passes all rounds with the exit-code verdict; manual environment prerequisite (live cluster), same status as other live-E2E tasks.
- Requirements: 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 7.1
- Done (requirements 8.2 + 8.3 MET): the converted soak was RUN live on a 3-node cluster (3 runs). Req 8.2 — it runs end-to-end and reports pass/fail purely via the
xce_testexit-code verdict (correctly PASS on rolling rounds, FAIL when a loss is present): MET. Req 8.3 — the rolling primary-kill + restart zero-acked-loss coverage is preserved and PASSES every round: MET. The live run also strengthened bug-786 (4.1 gatejobid != 0→jobid == jobsubmit.jobid, commit8609976; garbage63-15--1-...now rejected loud) — bug-786 fully closed. The NEW cold-restart-all step (added in 7.1 as bonus durability coverage, NOT required by 8.2/8.3 to pass) correctly DETECTS an upstream controller durability gap (bug-795: non-deterministic committed-job loss after a simultaneous all-node kill+restart; recovery logs WALtruncate). That finding is OUT OF BOUNDARY for this test-primitives spec (no controller code here) and is now OWNED byha-commit-durabilityPhase 6 (6.1 reproduce-on-real-fs gates the fix). A full-green run INCLUDING cold-restart-all is therefore gated on that upstream fix + a real fsync-able fs (this sandbox is overlayfs) — tracked there, not a blocker for this spec's completion. The soak doing its job (surfacing the gap) is success, not failure, for the reference driver.
Implementation Notes
- All of tasks 1–6 modify the single TU
src/controller/testsuite/test.c; sequential by design (no(P)). Task 7 touchessrc/tools/test-commit-durability.shbut depends on the primitives, so it is also ordered last. - Exit-code verdict is only trustworthy with a clean process exit; run the soak with
ASAN_OPTIONS=detect_leaks=0(existing convention). Client leak cleanup is out of scope (separate item). - Build dir is
src/xce.build(notsrc/build). Fast incremental build:cd src && nix develop --command bash -c "meson compile -C xce.build xce_test". Binary:src/xce.build/controller/xce_test. Behavioral tests = piped scripts gating on$?withASAN_OPTIONS=detect_leaks=0; no controller needed (an unknown command is a convenient failing command).xce_testis NOT a meson-registeredtest(), someson test --suite controllerdoes not exercise it. - Pre-existing unrelated failure:
meson test --suite controllerreportstest_demotion_replay_yields_committed_onlyFAIL (incontroller_unit_test/test_commit_barrier_reply.c) — from the in-flightha-commit-durabilityfeature, in a binary that does NOT compiletest.c. Not caused by this spec; ignore it for verdicts here. - 1.1:
preprocess_and_run()is the single chokepoint where the leading-!shell-escape (2.1) and$-autovar substitution (3.1) plug in. File-scopeg_session_failed/g_continue_on_error/g_last_statusadded;mainreturnsg_session_failed ? 1 : 0;set continue-on-erroris a process-global argc==2 special-case incmd_set(not a per-contextsettings[]entry). - 1.1 ERREXIT MODEL (shell
-e/+e, fixes the interactive UX regression where a typo exited the REPL): abort-on-first-failure is now a single flagg_continue_on_errorwith a MODE-AWARE DEFAULT set in main fromg_interactive = isatty(STDIN_FILENO)— batch/piped →-e(abort + non-zero exit verdict; defaultg_continue_on_error=0), interactive TTY →+e(failed command prints its error, loop CONTINUES; defaultg_continue_on_error=1). The 3 abort sites inpreprocess_and_runkey ONLY on!g_continue_on_error.cmd_setacceptsset -e(errexit on),set +e(errexit off), andset continue-on-error(==+e, back-compat) to override either default mid-session. Verified across all 6 combos (batch/interactive × default/-e/+e) via piped runs + a pty harness (checking the echo OUTPUT line, not the TTY's input echo). - Feature Flag Protocol: the flag must be fully REMOVED at the end (no pinned
#define X (1)guarding a dead#else). Reviewer rejects leftover flag scaffolding as dead code. (3.1 hit this — re-mediated.) - 3.1: introduced
g_last_jobid[XCE_MAXNAME_LEN]={0}(empty = unset) andg_last_jobcount=-1(-1 = unset) file-scope state, POPULATED later by 4.1 (jobsub) / 5.1 (joblist).substitute_autovars(line,out,sz)does whole-token replace of$?/$jobid/$jobcount; unset$jobid/$jobcount→ command fails (autovar: $X referenced before set), no garbage.!shell-escape lines are NOT substituted (raw to shell — the shell owns its$). Added#include "config.h"for XCE_MAXNAME_LEN. - 4.1 (bug-786 fixed):
cmd_jobsubinits localjobid=0, printsjobsubmit ok+ capturesg_last_jobidONLY after BOTH gates:rc>=0ANDjobid!=0; else typed error (E_XCE_NOMASTER/REDIRECT/NOT_READY/timeout switch viaxce_strerror_r) +return <0.$jobidcapture usesfmt_jobid(g_last_jobid, ..., jobid)— the FULL jobid, byte-identical to howcmd_joblistformatsjobs[i]->jobid(NOTarray_jobid(), which the display-onlyok:line uses). This is REQUIRED so task 5.3expect-job $jobidmatches the joblist column for array jobs. Also moved the error check AFTER the FREEUP block → fixed a pre-existing error-path leak. Live success-path ($jobid==joblist entry) verified by code reading; runtime-verified via soak (7.2). - 5.1: extracted DRY print-free helper
static int joblist_query(const char* find_jobid, int* contains, int print_table)— singlexcecli_joblistcall + internal free; returns count (>=0) or<0; membership viastrcmp(fmt_jobid(buf,...,jobs[i]->jobid), find_jobid)(SAME full-jobid form as 4.1's$jobid).cmd_joblistdelegatesjoblist_query(NULL,NULL,1)and setsg_last_jobcount=countONLY on success (no fake count on failure). 5.2/5.3 assertion commands calljoblist_querydirectly. Live$jobcount-after-success needs a controller (soak 7.2). - 5.2/5.3/6.1:
expect-jobs,expect-job,wait-primaryare all CLIENT-context commands registered inclient_commands[](they needactive_ctxto query jobs) — a script mustclientfirst. All return<0on failure →preprocess_and_runfails the session. Implemented directly (no flag macro); RED shown via unknown-command absence. Key invariant across assertions: a FAILED joblist query is an assertion failure (distinct message), NEVER silently "0 jobs"/"absent".wait-primary: monotonicclock_gettime(CLOCK_MONOTONIC), defaultWAIT_PRIMARY_DEFAULT_MS=30000, pollWAIT_PRIMARY_POLL_MS=200, bounded (no hang) — do NOT run no-argwait-primaryagainst a dead cluster in a test (blocks 30s); pass a small explicit timeout. - 7.1 (soak conversion): all
xce_testverdicts now come from the PROCESS EXIT CODE via a singlexce_test_scriptchokepoint (setsASAN_OPTIONS=detect_leaks=0).$jobidcapture usesecho ACKED_JOBID $jobid(space-separated — whole-token substitution requires it; a gluedMARK=$jobidwould NOT expand) parsed withsed. Multi-line script accumulation must use real$'\n', NOT$(...)(command-sub strips trailing newlines → glued tokens).current_primary/node_pid/node_rolelog helpers retained ONLY for lifecycle (victim selection, strace targeting), never as a verdict. Newrun_cold_restart_recoveryphase:!pkillall →!bash start.shall →wait-primary→expect-jobs/expect-job. 7.2 (live run on/xce-deploy-ha) is the only remaining task — manual env prerequisite (needs a real 3-node cluster). - 7.1 SOAK ROBUSTNESS HARDENING (post-merge, prompted by a 10-round live run that produced 18 mostly-FALSE "DATA LOSS — bug-661" lines): the converted soak CONFLATED liveness with durability — once a round's
wait-primaryfailed (cluster transiently unserveable), it STILL ranassert_no_loss, so everyexpect-job"not found" was mislabeled DATA LOSS, cascading. Root cause of the unavailability: killing the next primary before the previous round's restarted node had REJOINED → drops the live set to 1 node (no quorum). Fix (all insrc/tools/test-commit-durability.sh): (1)wait_cluster_healthygate (all 3 node PIDs up + a primary serves + rejoin settle) BEFORE each kill and before each durability assertion — never SIGKILL into a degraded cluster; (2)assert_no_lossPROBES serveability first →inconc()INCONCLUSIVE (distinct verdict, not DATA LOSS) when the cluster isn't serving; (3) a confirm-settle re-check so a momentary post-failover catch-up lag is not mislabeled a permanent loss — DATA LOSS is reported ONLY for a job PERSISTENTLY absent from a SERVING cluster; (4) post-kill election SLAELECT_TIMEOUT_MSdefault 60s, liveness failures labeled distinctly; (5)INCONCtally in the final verdict. RESULT: zero false cascades; durability vs liveness cleanly separated. A durability soak must NEVER cry bug-661 when it merely cannot reach a primary — that conflation makes the whole soak untrustworthy. - 7.2 LIVE RUN OUTCOME (ran 3 live soaks on a sandbox 3-node cluster): the converted soak works end-to-end with the exit-code verdict; rolling-kill zero-loss PASSES every round. TWO findings: (A) IN-SCOPE/FIXED — bug-786's 4.1 gate was insufficient (stale non-zero reply id
63-15--1-1015808slipped pastjobid != 0); strengthened tojobid == jobsubmit.jobid(commit8609976), garbage now rejected loud. Root cause is upstream libxce (xcectx.c handle_sub_replyleavesctx->reply.u.jobidstale on a non-jobid reply;xceapi.c:309copies it when status>=0) — the test client defends itself per Req 7.3 without touching libxce (boundary). (B) UPSTREAM/OUT-OF-SCOPE — cold-restart-all (kill ALL 3 + restart) NON-DETERMINISTICALLY loses a committed job (passed run #2, FAILED run #3: baseline job0-0-...332-0survived all rolling rounds then vanished after full restart; recovery logs show WALtruncate). This is a controller commit-durability race under total-cluster-outage — owned byha-commit-durability, surfaced (correctly) by this spec's new soak step. Sandbox is overlayfs, NOT the task's required "real fsync-able fs"; confirm on real hw + route to ha-commit-durability owner. DO NOT patch this in the test spec.