docs(raft-apply-decoupling): reconcile design.md deviations, retroactive boundary tag

Closes the two non-blocking follow-ups from the 2026-07-07 validate-impl
audit:

- design.md had 3 sections describing the original plan rather than what
  landed, each already self-disclosed in tasks.md's Implementation Notes
  but never reconciled back: repl.c's planned repl_submit_majority_commit()
  was never built (task 3.2 calls commit_submit_async() directly instead);
  the apply pump is wired via commit_resolve_pending() in commit.c, not
  ha.c's on_commit_advanced; fact hooks fire one generic
  HOOK_DETAIL_JOB_UPDATE, not per-type hooks (verified safe -- sched.c's
  subscribers already consume the generic form).

- task 5.3 touched ha.c (two promotion/fence call sites) without a formal
  _Boundary:_ exception the way task 1.4 got one for its own out-of-file
  touch -- added retroactively, no behavior change.
This commit is contained in:
Rongsong Shen 2026-07-07 08:48:57 +00:00
parent 9f530f5742
commit 48b4dba700
2 changed files with 5 additions and 3 deletions

View file

@ -132,11 +132,11 @@ This design has a hard build order, not just a hard conceptual one:
### Modified Files
- `src/controller/commit.c` — Replace the `COMMIT_MODE_WAIT` blocking-wait branch with a continuation-registration + `commit_resolve()` mechanism; `commit_advance_locked()` walks newly-decided LSNs and calls `commit_resolve()` for each instead of `cond_broadcast`-ing a single waiter set. `COMMIT_MODE_RETAIN` registration is unchanged (already non-blocking). Also adds the timeout-arming mechanism for registered continuations: removing the blocking wait also removes the only thing that ever detected a commit timing out (the caller's own `pthread_cond_timedwait` deadline), so an explicit timeout must be armed at registration and swept/fired independently.
- `src/controller/repl.c` — `repl_wait_majority_ack()` becomes `repl_submit_majority_commit()`: registers a continuation instead of blocking; retains its current callers' error-code contract (same negative-rc vocabulary) for the continuation's resolved outcome.
- `src/controller/repl.c` — **NOT touched (implementation deviation, confirmed 2026-07-07 validate-impl audit).** `repl_wait_majority_ack()`/`repl_submit_majority_commit()` were never built; task 3.2 instead calls `commit_submit_async()` directly on `p->repl.ce`, bypassing `repl.c` entirely (see tasks.md task 3.2's Implementation Note). This narrows the reusable primitive this design originally wanted — a third parallel "append+track a commit" implementation now exists alongside `COMMIT_MODE_WAIT`'s and `COMMIT_MODE_RETAIN`'s. Left as a known follow-up, not fixed retroactively (out of a validation pass's boundary).
- `src/controller/persist.c` — Add `apply_committed_entry()` as the single dispatch seam (extracted from today's `apply_wal_entry()` switch body, persist.c:2452-2562, plus the fact-transition guard fix). `apply_wal_entry()` (replay) and the new apply pump both call it. Add the post-replay CLEAN/DIVERGENT verification at the end of `persist_recover()`/`replay_wal()`. `persist_job_hook_fun`'s dead `WAL_MODE_WAIT` branch is removed (no longer needed once `repl_submit_majority_commit` fully replaces the old call convention). `wal_hook_append_classified()` (fact receipt path) stops mutating job state — it becomes durable-append-only. Also adds a call from the existing WAL-truncation point into the commit engine's `commit_resolve()` so any pending registered continuation for a truncated LSN resolves as failed instead of hanging.
- `src/controller/ha.c` — Adds a call from the existing demotion/role-transition point into the commit engine's `commit_resolve()` so any pending registered continuation invalidated by losing leadership resolves as failed instead of hanging. This is in addition to `ha.c`'s other listed change (wiring the apply pump, below).
- `src/controller/job.c` — `handle_jobsubmit()`: defer `create_job()`/job-object construction to fire from `apply_committed_entry()` after commit succeeds, not before. `mark_job_start/finish/fail/update()`: split into a guard+mutation core (callable from apply) and a receipt-time validation-only entry point; their `AFTER_EVENT` hook-firing relocates to fire from the apply step. `restore_job_state()` is retired in favor of routing replay through the same guarded core.
- `src/controller/ha.c` — Wire the apply pump: on `on_commit_advanced` (already-existing callback, ha.c:1776) and on the existing durable-majority `committed_lsn` advance, walk `[last_applied_lsn+1 .. committed_lsn]` in order and invoke `apply_committed_entry()` for each, dispatched via `vtask_enq_classified`.
- **Apply pump wiring — implementation deviation (confirmed 2026-07-07 validate-impl audit).** Task 6.1 wired the pump's trigger into `commit_resolve_pending()` (`commit.c`), not `ha.c`'s `on_commit_advanced` as originally planned above — every one of `commit.c`'s 6 internal `committed_lsn`-advance sites kicks the pump directly, which covers `on_commit_advanced`'s own call path plus every other advance site `on_commit_advanced` alone would have missed. The walk itself (`[last_applied_lsn+1 .. committed_lsn]` in order, invoking `apply_committed_entry()` for each, dispatched via `vtask_enq_classified`) matches this section as designed; only the trigger's wiring point differs.
- `src/controller/egress.c` — No functional change; `job_submit_committed()` (egress.c:47) is left in place as a now-tautological defensive predicate (see research.md).
### New Test Files
@ -372,7 +372,8 @@ int apply_committed_entry(persist_t* p, uint32_t wal_type, uint64_t lsn,
**Contracts**: Event [x] / Batch [x]
##### Event Contract
- Published events: `AFTER_EVENT` for `HOOK_DETAIL_JOB_START`/`_EXIT`/etc., now fired from the apply step, not the receipt handler.
- **Published events — implementation deviation (confirmed 2026-07-07 validate-impl audit).** All nine job-state fact WAL types route through a single guard-only `apply_job_update_transition()` core in `apply_committed_entry()` and fire one generic `HOOK_DETAIL_JOB_UPDATE`, not per-type `HOOK_DETAIL_JOB_START`/`_EXIT`/`_FAIL`/etc. as stated below. Verified safe in practice: `sched_job_update_fun`/`apiserver_job_update_fun` (`src/controller/sched.c:446-465`) already exist and consume exactly the generic `(cluster_t*, job_t*)` form the apply path passes — no subscriber assumed per-type dispatch. Left as a documentation correction, not a functional gap.
- ~~Published events: `AFTER_EVENT` for `HOOK_DETAIL_JOB_START`/`_EXIT`/etc., now fired from the apply step, not the receipt handler.~~ (superseded by the note above)
- Subscribed events: none new.
- Ordering / delivery guarantees: strict commit order via the shared apply pump; at-least-once redelivery for facts lost before append remains `worker-rejoin-resync`'s responsibility (out of boundary).

View file

@ -127,6 +127,7 @@
- Observable: the test process remains running and retrying after a forced DIVERGENT verdict, never terminating
- _Requirements: 8.4_
- _Depends: 5.2_
- _Boundary: Restart Replay & Verification — retroactive exception (added 2026-07-07 validate-impl audit, mirroring task 1.4's precedent): review found the sticky `recovery_unrecoverable` flag has two pre-existing, trigger-agnostic live-clear call sites in `ha.c` (`promotion_fence_timeout_fn`, `raft_cb_become_leader`'s promotion gate) that could silently release an apply-divergence-triggered fence. The fix is confined to those two existing call sites only — an `apply_divergent` guard ANDed into each — never job.c or any command-handler code._
- [x] 6. Ordered apply pump integration
- [x] 6.1 Build the strict-order walk that applies every newly committed entry