commit_submit_async() lets a WAIT-mode caller register a continuation against an appended LSN instead of blocking on pthread_cond_timedwait, reusing the existing admission-cap discipline (wait_admit_locked/ wait_release_locked) so the bounded-capacity guarantee carries over unchanged. Purely additive: no existing call site (repl_wait_majority_ack) is touched yet -- that switch lands in task 3.2 once commit_resolve() (task 1.2) exists to drive it.
14 KiB
Implementation Plan
-
1. Commit engine event-driven resolve (keystone prerequisite)
-
1.1 (P) Add per-LSN continuation registration to the commit engine for the synchronous (WAIT) commit mode, replacing the blocking wait
- Register a continuation (callback + context) against an appended LSN instead of parking the calling thread
- Preserve the existing per-LSN bookkeeping and admission-cap discipline already used by the commit engine
- Observable: a unit test can register a continuation for an LSN and the calling thread returns immediately, before the LSN commits
- Requirements: 1.3, 1.4, 9.1, 9.2
- Boundary: Commit Engine Resolve — commit.c per-LSN registry only
-
1.2 Wire majority-commit and failure-path resolution to invoke the registered continuation exactly once
- On majority ack, invoke the continuation with a committed outcome
- On timeout, demotion, or log-tail truncation of a pending LSN, invoke the continuation with a failed outcome and the existing error-code vocabulary
- Dispatch every continuation invocation off the calling thread using the existing non-blocking task-submission primitive, never inline on the replication-ACK or iomgr path
- Observable: forcing each of {majority ack, timeout, demotion, truncation} in a test resolves the registered continuation exactly once, with no case left unresolved
- Requirements: 1.3, 9.3, 9.4
-
1.3 Unit test the commit engine's resolve behavior under concurrent in-flight commits
- Cover exactly-once resolution across majority ack, timeout, demotion, and truncation
- Cover that a burst of concurrently pending commits does not exhaust the bounded request-handling capacity
- Observable: a test suite run demonstrates bounded worker usage and exactly-once resolution under a concurrent-commit burst
- Requirements: 9.1, 9.2, 9.3, 9.4
-
2. Shared apply seam for committed entries
-
2.1 (P) Extract the guarded state-transition logic used by fact handling into a form usable outside the receipt-time call site
- Keep the existing monotonic-rank checks (reject backward transitions, no-op duplicate terminal transitions) intact and unchanged in behavior
- Decouple the transition logic from firing its own hook inline, so hook-firing can be relocated by a later task without touching this logic again
- Observable: the extracted transition logic can be invoked directly by a test without going through the worker-report receipt path, and produces the same guard behavior as before extraction
- Requirements: 3.3, 7.1, 7.2
- Boundary: Apply Seam — persist.c / job.c apply-dispatch and replay call sites only, no dependency on the commit-engine change in task 1
-
2.2 Build the single dispatch function that applies one committed entry to in-memory state, covering every classified entry type
- Route job submission through the existing idempotent, jobid-addressable construction primitive instead of a raw uncoordinated insert
- Route job state transitions through the extracted guarded logic from 2.1
- Route queue control and worker control entries through their existing restore primitives
- Fire the entry type's post-mutation notification hook from inside this function, after the mutation completes
- Observable: calling the dispatch function directly with a sample entry of each classified type produces the correct in-memory mutation and fires exactly one post-mutation notification
- Requirements: 2.2, 3.1, 3.2, 4.1, 4.2, 5.2
- Depends: 2.1
-
2.3 Point restart replay's per-entry application at the new dispatch function
- Replace replay's direct calls to the individual restore primitives with calls to the dispatch function from 2.2
- Observable: restart replay of a log containing every classified entry type produces identical materialized state to before this change, verified by an existing or updated replay test
- Requirements: 3.1, 3.2
- Depends: 2.2
-
2.4 Unit test idempotent and ordered re-application of the dispatch function
- Cover applying the same committed entry twice is a no-op with respect to observable state
- Cover that a duplicate or out-of-order fact entry does not regress a job's lifecycle state
- Observable: a test applies a FINISH entry followed by a stale, previously-unapplied START entry for the same job and confirms the job remains in its terminal state
- Requirements: 3.4, 7.1, 7.2, 7.3
- Depends: 2.2
-
3. Command apply-after-commit ordering
-
3.1 (P) Stop constructing the in-memory job object before the commit barrier
- Change job submission to carry forward only the data needed to build the WAL entry payload, without inserting anything into the in-memory job table until the (still-synchronous, at this point) commit call returns success
- Remove the now-unreachable code path that previously ran the durability step from inside the notification hook for job submission
- Observable: a job submission whose commit fails or times out leaves no job object in the in-memory job table; a successful submission still replies synchronously exactly as before this task
- Requirements: 1.2, 2.1, 2.3
- Boundary: Raft Step (Command) — job.c handle_jobsubmit and its call chain only, no dependency on task 1
-
3.2 Convert job submission to register its commit outcome through the new continuation mechanism instead of blocking
- Replace the synchronous commit call from 3.1 with a registration against the task 1 continuation API
- On a failed or timed-out outcome, reply failure directly from the continuation — no apply is needed on this path
- On a committed outcome, hand the entry off to the ordered apply pump (task 6) rather than applying or replying from this continuation directly, so this entry's apply and reply happen in strict commit order relative to every other entry
- Observable: the calling thread returns before the commit resolves; a forced failure/timeout outcome still replies failure immediately with no job ever appearing (the committed-outcome path has no independently observable behavior until task 6 completes it)
- Requirements: 1.1, 1.4, 2.1, 2.3
- Depends: 1.1, 1.2
-
3.3 Unit test the command failure path leaves no observable trace
- Force repeated commit failures and timeouts for job submission and confirm no job object is ever created
- Observable: the test suite demonstrates zero phantom job objects across repeated forced commit failures
- Requirements: 2.1, 2.3
- Depends: 3.2
-
4. Fact apply-after-commit split
-
4.1 (P) Change worker-fact receipt handling to validate and durably append only
- Remove the state mutation and the post-mutation notification hook firing from the receipt-time handlers for job start, finish, fail, and update reports
- Keep the existing pre-mutation validation hook firing at receipt time, unchanged
- Observable: after a worker reports a job event, the job's in-memory state is unchanged immediately after the receipt handler returns
- Requirements: 5.1
- Depends: 2.1
- Boundary: Fact Ingestion — job.c on_job_start/on_job_finish/on_job_fail/on_job_update receipt handlers only, disjoint from handle_jobsubmit
-
4.2 Unit test durable retention of facts submitted while majority is unreachable
- Submit facts while majority is unreachable and confirm they are durably appended and that job state remains unchanged for the duration of the outage
- Confirm no local-only apply code path exists by inspection of the receipt handler from 4.1 (no runtime check needed — the guarantee is structural)
- Observable: the test demonstrates zero state mutation during a simulated quorum-loss window
- Requirements: 6.1, 6.2
- Depends: 4.1
-
5. Restart apply-correctness verification
-
5.1 (P) Compute a post-replay expectation from the committed log tip and compare it against materialized state
- Derive a lightweight expectation (e.g., last-applied position and a count or checksum of materialized entities) from the recovered log
- Compare the expectation against the actual state produced by replay through the shared apply dispatch function
- Observable: a clean replay produces a CLEAN verdict alongside the existing structural (CRC) verdict
- Requirements: 8.1, 8.2
- Depends: 2.3
- Boundary: Restart Replay & Verification — persist.c replay/recovery path only
-
5.2 Route a verification mismatch through the existing non-promotable fence mechanism
- Reuse the existing loud-fence function and sticky fence flag rather than introducing a new terminal or fencing path
- Observable: an injected materialization mismatch produces a DIVERGENT verdict, a loud operator-visible log entry, and a non-promotable fenced state
- Requirements: 8.3, 8.4
- Depends: 5.1
-
5.3 Unit test the DIVERGENT verdict path stays alive and keeps retrying
- Force a DIVERGENT verdict and confirm the process does not exit or abort
- Confirm the fence clears once a subsequent recovery reaches a consistent state
- Observable: the test process remains running and retrying after a forced DIVERGENT verdict, never terminating
- Requirements: 8.4
- Depends: 5.2
-
6. Ordered apply pump integration
-
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
- Observable: a burst of interleaved command and fact entries committing out of submission order still applies in commit order
- Requirements: 3.3, 7.3
- Depends: 1.2, 2.2
-
6.2 Fire a command's registered continuation immediately after its entry applies
- After the walk in 6.1 applies an entry that has a registered commit continuation (from task 3.2), invoke that continuation now, after the mutation, not before
- Leave fact entries with no further action beyond what the apply dispatch function itself performs
- Observable: a successful job submission's client reply carries the job id only after the job is visible in the in-memory job table; the reply always occurs strictly after the mutation
- Requirements: 1.1, 2.1, 2.2, 2.3
- Depends: 6.1, 3.2
-
6.3 Integration test command and fact apply/notify timing through the full pump
- Cover a successful job submission becoming visible and notified only after commit, exercised through the full pump (superseding the earlier synchronous-path check from task 3.1)
- Cover that worker slot release, scheduler dispatch triggering, and client/apiserver notification for a fact do not occur until that fact's entry is committed
- Observable: the test suite shows zero fact-reaction side effects during a delayed-commit window, and a submitted job's reply always follows its visibility in the job table
- Confirm a consumer acting on the post-apply notification never needs to separately re-check a commit watermark, since the notification's occurrence already implies the entry is committed and applied
- 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
- 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
- Depends: 6.1, 4.2
-
7. Conformance, regression, and existing-behavior preservation
-
7.1 Extend the deterministic conformance harness with apply-ordering assertions
- Add an assertion that no entry's state mutation or reactions occur before that entry commits
- Add an assertion that re-applying an already-committed entry is idempotent
- Observable: the conformance harness run reports both new assertions passing alongside its existing safety checks
- Requirements: 10.1, 10.3
- Depends: 6.3, 6.4
-
7.2 Add a live-cluster regression scenario that kills the primary mid-fact-stream
- Drive a continuous worker-fact stream, kill the primary partway through, and let a new primary take over
- Assert no phantom slot-frees, no committed-state divergence, and no notification for a fact that never committed
- Observable: the scenario completes with zero divergence and zero phantom reactions across repeated runs
- Requirements: 10.2, 10.4
- Depends: 6.3, 6.4
-
7.3 Re-run the full existing automated test suite and update only the tests tied to the old ordering
- Run the existing unit, integration, and HA soak suites
- Update only the specific tests that asserted the old apply-before-commit ordering to assert the new ordering instead
- Observable: the full suite passes with zero committed-job loss and zero state divergence across the existing failover soak
- Requirements: 11.1, 11.2, 11.3
- Depends: 7.1, 7.2
-
7.4 Measure sustained fact-stream throughput against the pre-feature baseline
- Record a throughput baseline on the pre-feature code, then drive the same sustained high-volume worker-fact stream against the completed feature
- Confirm no per-fact synchronous commit barrier was introduced and throughput does not regress
- Observable: a recorded throughput comparison shows no regression relative to the pre-feature baseline
- Requirements: 11.4
- Depends: 7.3
Implementation Notes
- 1.1:
commit_submit_async()reuses the exactwait_admit_locked/wait_release_locked/inflight_waitsadmission-cap discipline asCOMMIT_MODE_WAIT— task 1.2'scommit_resolve()MUST callwait_release_locked()when resolving an async-registered entry, or the shared admission cap leaks. Continuation registry iscalloc(n_vcpus, ...), sized son_continuations >= wait_cap(ce)always holds;lsn == 0is the free-slot sentinel. - 1.1 review: bare
controller_unit_test(no--runfilter) segfaults deterministically in this environment (SSL_CTX_free/iomgr_fini crash,test_ha_vep_teardown.c) — reproduced identically on the pre-task base commit, confirmed pre-existing and unrelated. Logged as bug-1406. Alsotest_commit_advance_retry_clears_push_before_commit_fence(test_config_repl.c) fails independently of this feature, also reproduced on base — logged as bug-1407, flagged because it's adjacent tocommit_advance_locked, which task 1.2 modifies. Always validate with a filtered--run <group>invocation or the tightly-coupled commit-engine cluster, not the bare unfiltered binary, until 1406/1407 are separately fixed.