From e8ecc87551ce76932960bbfd4f28cbea5b93b21f Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 12 Jun 2026 04:43:50 +0000 Subject: [PATCH] feat(ha-commit-barrier-rollout): wal_classify + classified hook dispatch (task 1.2) Verified table implemented (STOP/RESUME/UPDATE/FAIL RETAIN per supersession; WORKER detail switch; CHECKPOINT LOCAL); unknown type = ERROR + XCE_TESTING abort + production RETAIN fallback. All three hooks route through wal_hook_append_classified (zero bare async appends); RETAIN registers with the engine lock-free; facts propagate local append rc only. 8/8 group tests green; full suite 465/465. --- .../specs/ha-commit-barrier-rollout/tasks.md | 3 +- .wolf/cerebrum.md | 1 + .wolf/memory.md | 10 ++ src/controller/persist.c | 144 +++++++++++++++--- 4 files changed, 138 insertions(+), 20 deletions(-) diff --git a/.kiro/specs/ha-commit-barrier-rollout/tasks.md b/.kiro/specs/ha-commit-barrier-rollout/tasks.md index 6a0b86a1..d1046b2c 100644 --- a/.kiro/specs/ha-commit-barrier-rollout/tasks.md +++ b/.kiro/specs/ha-commit-barrier-rollout/tasks.md @@ -11,7 +11,7 @@ - Observable completion: build succeeds with the new header declarations; all new test cases FAIL (red) against the unmodified persist.c - _Requirements: 2.1, 2.2, 3.1_ -- [ ] 1.2 Implement wal_classify and the hook-seam dispatch to green +- [x] 1.2 Implement wal_classify and the hook-seam dispatch to green - Implement `wal_classify(wal_type, detail)` in persist.c per the design's verified classification table; consult it in `persist_job_hook_fun` / `persist_queue_hook_fun` / `persist_worker_hook_fun` (persist.c:1093/1172/1204 anchors): RETAIN types → `persist_wal_async` + `commit_submit(ce, appended_lsn, COMMIT_MODE_RETAIN, 0)` registration; WAIT types → WAL_JOB_SUBMIT keeps the existing sync path, queue/worker appends stay async (handler barrier owes completion, task 3); LOCAL → unchanged - Unclassified guard: `assert` under XCE_TESTING with the offending type logged; production ERROR + RETAIN fallback - Fact appends never propagate commit-progress errors to hook callers (local append rc only) @@ -111,3 +111,4 @@ - (1.1) The unknown-type fail-fast test forks and asserts WTERMSIG==SIGABRT — task 1.2 must implement the XCE_TESTING guard as assert/abort (or the test needs a matching tweak). - (1.1) wal_entry_type has 13 values (wal_types.h:44-58), no WAL_NOOP yet (lands in ha-single-leader-role, which must add a classification row + sweep entry — the totality sweep will catch it). - (1.1) Suite baseline is ZERO failures since ha-commit-engine 6.1. detect_leaks=0 remains the harness norm (6 inventoried pre-existing leak groups). +- (1.2) CENSUS METHOD: only ~9 of 48 groups emit Unity summary blocks; the rest report per-test PASS lines only — count distinct Unity results (~465) and grep FAIL lines, never sum summary blocks. commit_submit(RETAIN) early-returns BEFORE taking ce->lock (commit.c:233-239) — registration is lock-free, safe with entity write locks held. wal_hook_append_classified is the single classified-append seam; appended_lsn read = current_lsn - 1 under p->lock (benign race, commented). diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index f813a2e4..6d2c4dc4 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -9,6 +9,7 @@ ## Key Learnings +- `commit_submit(COMMIT_MODE_RETAIN)` is fully lock-free: it returns 0 BEFORE `pthread_mutex_lock(&ce->lock)` (commit.c:233-239), so it is safe to call from hook functions even while entity write locks are held — only WAIT mode takes ce->lock. (barrier-rollout task 1.2, 2026-06-12) - ha-commit-engine: once repl.c references commit_* symbols (task 5.2), commit.c MUST be in controller_src (meson.build) or the production xce_controller link fails — the "linkage lands with 5.3" note in tasks.md only held while repl.c had no engine references. Lock order ce->lock → r->lock: repl.c calls engine hooks only AFTER releasing r->lock. - Use `clang-format` for C/C++ formatting; shared config lives at `xce/.clang-format`. diff --git a/.wolf/memory.md b/.wolf/memory.md index 04313e91..de339623 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -975,3 +975,13 @@ | 21:05 | Task 1.1 ha-commit-barrier-rollout RED complete: build green, 8/8 new cases FAIL on assertions, 0 pre-existing failures (901 PASS) | test_commit_barrier_rollout.c, persist.h, persist.c, meson.build, controller_test.c | RED verified | ~60k | | 21:12 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~21 | | 21:13 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | added error handling | ~350 | +| 21:17 | Edited src/controller/persist.c | 2→3 lines | ~33 | +| 21:18 | Edited src/controller/persist.c | added 4 condition(s) | ~1200 | +| 21:18 | Edited src/controller/persist.c | modified if() | ~120 | +| 21:18 | Edited src/controller/persist.c | modified persist_queue_hook_fun() | ~147 | +| 21:19 | Edited src/controller/persist.c | modified persist_worker_hook_fun() | ~188 | +| 21:19 | Edited src/controller/persist.c | 4→6 lines | ~69 | +| 04:30 | Task 1.2 ha-commit-barrier-rollout: wal_classify table + wal_hook_append_classified dispatch in 3 hooks | src/controller/persist.c | 8/8 PASS, full controller suite 0 FAIL | ~30k | +| 04:42 | Adversarial review of ha-commit-barrier-rollout task 1.2 (wal_classify + hook dispatch): APPROVED — full suite census 465 Unity PASS / 0 FAIL (implementer's 9/109 was a Unity-summary-block misread, not a partial run) | src/controller/persist.c | APPROVED | ~30k | +| 21:43 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~19 | +| 21:43 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | 1→2 lines | ~168 | diff --git a/src/controller/persist.c b/src/controller/persist.c index 6416a3ff..61042a12 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -9,6 +9,7 @@ #include "controller.capnp.h" #include "xlist.h" #include "cluster.h" +#include "commit.h" /* commit_submit RETAIN registration (barrier-rollout 1.2) */ #include #include @@ -1118,16 +1119,104 @@ static uint32_t detail_to_wal_type(int detail) } } -/* Write-path commit classification — INERT RED-phase stub - * (ha-commit-barrier-rollout task 1.1; sanctioned scaffolding so the RED - * tests fail on assertions, not at link time). Returns an out-of-enum - * sentinel: classifies nothing yet. The verified classification table - * (design.md §Write-Path Classification) is implemented in task 1.2. */ +/* Write-path commit classification (ha-commit-barrier-rollout task 1.2). + * Pure lookup over the VERIFIED table (design.md §Write-Path + * Classification); `detail` disambiguates WAL_WORKER_CTRL only. + * Unclassified types fail fast under XCE_TESTING (abort) and fall back to + * RETAIN in production — never dropped, never falsely confirmed (Req 2.2). */ wal_commit_mode_t wal_classify(uint32_t wal_type, int detail) { - (void)wal_type; - (void)detail; - return (wal_commit_mode_t)-1; /* invalid sentinel — task 1.2 replaces */ + switch (wal_type) { + case WAL_JOB_SUBMIT: + /* client job submit — request-shaped; keeps the in-hook sync path + * (reference implementation, handle_jobsubmit) */ + return WAL_MODE_WAIT; + case WAL_QUEUE_CTRL: + /* client queue control — request-shaped; the handler barrier owes + * commit completion before the requester reply (tasks 2-3) */ + return WAL_MODE_WAIT; + case WAL_WORKER_CTRL: + /* the only detail-disambiguated type: JOIN/UPDATE have a requester + * awaiting confirmation (WAIT); DEAD is a controller-detected fact. + * Unexpected details take the fact-safe RETAIN side. */ + if (detail == HOOK_DETAIL_WORKER_JOIN || + detail == HOOK_DETAIL_WORKER_UPDATE) + return WAL_MODE_WAIT; + return WAL_MODE_RETAIN; + case WAL_JOB_START: + case WAL_JOB_FINISH: + case WAL_JOB_FAIL: + case WAL_JOB_STOP: + case WAL_JOB_RESUME: + case WAL_JOB_UPDATE: + case WAL_JOB_RUSAGE: + case WAL_JOB_CKPT: + case WAL_JOB_PREASON: + /* worker/scheduler-reported facts — retained until committed. + * FAIL/UPDATE are dual-origin; their request-shaped side is + * realized by the handler barrier in do_jobcontrol_op + * (research.md §2 call-site verification). */ + return WAL_MODE_RETAIN; + case WAL_CHECKPOINT: + /* local segment marker — never replicated, by design */ + return WAL_MODE_LOCAL; + default: + ERROR("wal_classify: unclassified WAL type %u (detail %d) — " + "RETAIN fallback; add it to the classification table", + wal_type, detail); +#ifdef XCE_TESTING + assert(0 && "unclassified WAL type — add it to wal_classify"); +#endif + return WAL_MODE_RETAIN; + } +} + +/* Classified append at the hook seam (barrier-rollout task 1.2). + * + * WAIT (queue/worker control): the append stays async here — the handler + * barrier (tasks 2-3) owes commit completion before the requester reply. + * WAL_JOB_SUBMIT never reaches this helper (it keeps the in-hook sync + * path in persist_job_hook_fun). + * + * RETAIN (facts): append + broadcast, then register the appended LSN with + * the commit engine. commit_submit(COMMIT_MODE_RETAIN) is registration- + * free bookkeeping (commit.c — returns 0 immediately; commit state is + * derived from registry watermarks at ACK time), so the call is contract + * documentation: the entry stays pending purely by lsn > committed_lsn + * and commits later via covering ACKs. Commit-progress results are NEVER + * propagated to hook callers — only the local append rc is (Req 3.1). + * + * Lock context: persist_wal_async takes/releases p->lock internally; the + * appended-LSN read below re-takes p->lock briefly (same discipline as + * persist_wal_sync_append) and releases it BEFORE commit_submit, so no + * p->lock is ever held across ce->lock (engine lock-ordering rule). */ +static int wal_hook_append_classified(persist_t* p, uint32_t wal_type, + wal_commit_mode_t mode, + const void* payload, uint32_t len) +{ + uint64_t appended_lsn; + int rc; + + rc = persist_wal_async(p, wal_type, payload, len); + if (rc != 0 || mode != WAL_MODE_RETAIN) + return rc; + + if (p->repl.ce == NULL) + return rc; /* engine not wired (bootstrap / engine-less contexts) */ + + /* current_lsn - 1 is the LSN persist_wal_async just assigned (it + * increments under p->lock). A concurrent append may bump it further; + * that is benign — contiguous advancement means the higher watermark + * still covers this entry. LSN-0 edge: commit_submit rejects lsn == 0 + * (first-ever entry of a fresh cluster — accepted interim behavior, + * spec Implementation Notes). */ + pthread_mutex_lock(&p->lock); + appended_lsn = p->current_lsn - 1; + pthread_mutex_unlock(&p->lock); + if (appended_lsn != 0) + (void)commit_submit(p->repl.ce, appended_lsn, COMMIT_MODE_RETAIN, 0); + + return rc; /* local append rc only — facts have no commit timeout */ } /* P0 payload: just the detail value (4 bytes). @@ -1142,13 +1231,18 @@ int persist_job_hook_fun(int type, int situation, int detail, if (situation != HOOK_AFTER_EVENT) return 0; - persist_t* p = cls; - uint32_t wal_type = detail_to_wal_type(detail); + persist_t* p = cls; + uint32_t wal_type = detail_to_wal_type(detail); + wal_commit_mode_t mode; + if (wal_type == 0) return 0; + mode = wal_classify(wal_type, detail); - if (wal_type == WAL_JOB_SUBMIT) { - /* AFTER_EVENT hook args: cluster_t* c, xlist_t* jlist */ + if (mode == WAL_MODE_WAIT) { + /* WAL_JOB_SUBMIT — the only WAIT type in the job hook; reference + * implementation keeps the existing in-hook sync path. + * AFTER_EVENT hook args: cluster_t* c, xlist_t* jlist */ (void)va_arg(ap, cluster_t*); xlist_t* jlist = va_arg(ap, xlist_t*); int rc = 0; @@ -1209,7 +1303,8 @@ int persist_job_hook_fun(int type, int situation, int detail, } } - return persist_wal_async(p, wal_type, &js, sizeof(js)); + /* mode == WAL_MODE_RETAIN — fact-shaped job state transition */ + return wal_hook_append_classified(p, wal_type, mode, &js, sizeof(js)); } int persist_queue_hook_fun(int type, int situation, int detail, @@ -1219,10 +1314,13 @@ int persist_queue_hook_fun(int type, int situation, int detail, if (situation != HOOK_AFTER_EVENT) return 0; - persist_t* p = cls; - uint32_t wal_type = detail_to_wal_type(detail); + persist_t* p = cls; + uint32_t wal_type = detail_to_wal_type(detail); + wal_commit_mode_t mode; + if (wal_type == 0) return 0; + mode = wal_classify(wal_type, detail); /* args: cluster_t*, queue_t* */ (void)va_arg(ap, cluster_t*); @@ -1239,7 +1337,10 @@ int persist_queue_hook_fun(int type, int situation, int detail, memcpy(pkt, &hdr, sizeof(hdr)); memcpy(pkt + sizeof(hdr), name, name_len); - int rc = persist_wal_async(p, wal_type, pkt, (uint32_t)total); + /* mode == WAL_MODE_WAIT — the append stays async; the handler barrier + * owes commit completion before the requester reply (task 3) */ + int rc = wal_hook_append_classified(p, wal_type, mode, pkt, + (uint32_t)total); free(pkt); return rc; } @@ -1251,10 +1352,13 @@ int persist_worker_hook_fun(int type, int situation, int detail, if (situation != HOOK_AFTER_EVENT) return 0; - persist_t* p = cls; - uint32_t wal_type = detail_to_wal_type(detail); + persist_t* p = cls; + uint32_t wal_type = detail_to_wal_type(detail); + wal_commit_mode_t mode; + if (wal_type == 0) return 0; + mode = wal_classify(wal_type, detail); /* WAL_WORKER_CTRL payload layout: [uint32_t detail][capnp worker OR name] */ uint32_t disc = (uint32_t)detail; @@ -1281,7 +1385,9 @@ int persist_worker_hook_fun(int type, int situation, int detail, memcpy(pkt + WORKER_WAL_HDR_SIZE, buf, capnp_sz); free(buf); - rc = persist_wal_async(p, wal_type, pkt, (uint32_t)total); + /* JOIN/UPDATE → WAIT (handler barrier owes completion, task 3); + * DEAD → RETAIN (controller-detected fact, registered with the engine) */ + rc = wal_hook_append_classified(p, wal_type, mode, pkt, (uint32_t)total); free(pkt); return rc; }