Pre-existing uncommitted work: ha-repl-hardening payload data plane (7.x), heartbeat liveness fix (Req 1.3), connection-reuse ASAN fixes, doc cleanup. New: ha-consistency-invariants umbrella (requirements/design/research) and its four implementation sub-specs (specs generated via batch, cross-spec review applied), steering roadmap + product.md Raft+CP decision.
332 lines
30 KiB
Markdown
332 lines
30 KiB
Markdown
# Design: ha-commit-engine
|
||
|
||
> Sub-spec 1 of 4 of the **ha-consistency-invariants** umbrella. This design slices the umbrella design's **Commit Engine** and **Quorum Accounting** components (`.kiro/specs/ha-consistency-invariants/design.md`) and reuses the umbrella's `commit.h` service interface verbatim. Discovery is complete via the umbrella's `research.md` (Option C phased hybrid, locked decisions); this spec implements umbrella Migration Strategy phase 1. Requirement IDs below are THIS spec's IDs (see `requirements.md` for the umbrella mapping).
|
||
|
||
## Overview
|
||
|
||
**Purpose**: This feature gives cluster operators an honest commit barrier for job submission: a new commit engine (`commit.c`) becomes the single authority for `committed_lsn` advancement, evaluating majority per entry (`acked_lsn >= N`, self counted) instead of "ACKed anything ever", and the dispatch-gate quorum count stops counting catch-up standbys.
|
||
|
||
**Users**: Cluster operators gain the guarantee that a confirmed job submission is durable on a majority of members; downstream sub-specs (ha-commit-barrier-rollout, ha-single-leader-role, ha-convergence-recovery) gain the engine interface they build on.
|
||
|
||
**Impact**: Replaces the counting logic of `repl_wait_majority_ack` (repl.c:386-461) for WAL ACKs with per-LSN engine evaluation; removes the `n_active == 0 → success` short-circuit for multi-member clusters; moves the `committed_lsn` store out of `persist_wal_sync_append` (persist.c:1035) into the engine; fixes `ha_has_write_quorum` (ha.c:1507) to exclude catch-up standbys. Behavior of every other write path is unchanged in this spec.
|
||
|
||
### Goals
|
||
- Entry N commits only when `1 (self) + count(active standbys with acked_lsn >= N) >= n_peers/2 + 1`
|
||
- `committed_lsn` advancement: monotonic, contiguous, never beyond the appended log — owned by the engine alone
|
||
- Full `commit.h` service interface shipped (WAIT + RETAIN modes, `commit_set_writable_lsn`, membership re-evaluation) so downstream specs consume without interface churn
|
||
- Admission cap on concurrent WAIT blockers, expressed relative to `n_vcpus` (never absolute)
|
||
- `n_peers <= 1` self-quorum preserved; `n_peers > 1` zero-peer-ACK never commits
|
||
|
||
### Non-Goals
|
||
- Wiring any path other than job submit (WAL_JOB_SUBMIT sync append) — barrier-rollout owns the classification sweep
|
||
- Setting `writable_lsn` anywhere (own-term gate consumers are ha-single-leader-role; unset = no gate)
|
||
- RETAIN-mode production consumers (facts roll out in ha-commit-barrier-rollout; the mode itself ships and is tested here)
|
||
- Config-change barrier (`acked_config_lsn` track untouched), demotion/egress/catch-up changes, COMMITTING removal
|
||
- Async continuation of commit waits (umbrella-documented future work; blocking condvar + admission cap is the accepted mechanism)
|
||
|
||
## Boundary Commitments
|
||
|
||
### This Spec Owns
|
||
- `src/controller/commit.c` + `src/controller/include/commit.h` — the commit engine: per-LSN ACK evaluation, waiter registry, admission cap, `writable_lsn` storage, `committed_lsn` advancement
|
||
- The walAck branch semantics of `repl_wait_majority_ack` (becomes the engine adapter) and the `repl_on_ack → commit_on_ack` feed
|
||
- `ha_has_write_quorum` catch-up exclusion and the `repl_active_standby_count` helper
|
||
- The `committed_lsn` advancement rule on the job-submit sync path (`persist_wal_sync_append`)
|
||
- `testsuite/test_commit_engine.c` and updates to existing tests whose expectations encode the dishonest counting
|
||
|
||
### Out of Boundary
|
||
- Which WAL types use WAIT vs RETAIN (classification table — ha-commit-barrier-rollout)
|
||
- Calling `commit_set_writable_lsn` from promotion, WAL_NOOP, COMMITTING deletion (ha-single-leader-role)
|
||
- Demotion truncate+restart, rejoin reconciliation, egress watermark filter, ACK-driven catch-up, quorum-transition logging (ha-convergence-recovery)
|
||
- `acked_config_lsn` / configAck counting in `repl_wait_majority_ack` (config barrier — ha-commit-barrier-rollout); the configAck branch is left byte-for-byte as-is
|
||
- WalAckReply wire format, standby apply mechanics, `acked_lsn` update rules (ha-wal-replication — complete upstream)
|
||
|
||
### Allowed Dependencies
|
||
- `standby_conn_t.acked_lsn` / `in_catch_up` per-standby tracking in `replicator_t` (src/include/repl.h) — read-only consumption; the engine never manages connections
|
||
- `persist_t.committed_lsn` atomic (src/include/persist.h:71) — the engine is the only production writer after init/truncation
|
||
- iomgr/vtaskmgr threading rules per `ha-async-io.md`: `commit_on_ack` and `commit_on_membership_change` are called on the iomgr thread and must never block on I/O or long waits (bounded mutex hold only); `commit_submit(WAIT)` blocks only on vtask threads
|
||
- Dependency direction (umbrella contract): `include/commit.h` depends only on libc/stdatomic types plus forward-declared `persist_t`; `commit.c` is called BY `persist.c` (append/sync path), `repl.c` (ACK + membership events), and `ha.c`/`cluster.c` (init, future promotion); it calls into none of them
|
||
|
||
### Revalidation Triggers
|
||
- Any change to `commit.h` signatures or return-code contract → revalidate ha-commit-barrier-rollout, ha-single-leader-role, ha-convergence-recovery
|
||
- Any change to `acked_lsn` semantics or WalAckReply payload (ha-wal-replication) → re-verify engine evaluation
|
||
- Changing the admission-cap formula or `COMMIT_WAIT_RESERVED` default → re-check operator tuning guidance and barrier-rollout sizing assumptions
|
||
- Moving `committed_lsn` ownership (e.g., persisting it) → revalidate ha-convergence-recovery's rejoin reconciliation
|
||
|
||
## Architecture
|
||
|
||
### Existing Architecture Analysis
|
||
- Single-writer model: only the primary appends WAL entries; per-standby `acked_lsn` (Raft matchIndex equivalent) already exists and is updated by `repl_on_ack` (repl.c:176-196) on the iomgr thread (`peer_ctrl_msg_handler` → `repl_ack_decode_and_dispatch` → `repl_on_ack`, ha.c:2739-2748)
|
||
- ACKs are cumulative: a standby ACKs its highest contiguously applied LSN (ha-wal-replication contract), so per-standby `acked_lsn` is a watermark — engine evaluation needs no per-entry bitmap
|
||
- Defects replaced: `repl_wait_majority_ack` counts `acked_lsn > 0` (repl.c:419) — any ACK ever; `n_active == 0` short-circuits to success (repl.c:432); quorum denominator is `n_active` (connected standbys) rather than `n_peers` (configured members); `ha_has_write_quorum` (ha.c:1507-1526) counts `standby_endpoints->count`, which includes catch-up standbys
|
||
- `persist_wal_sync_append` (persist.c:1003-1049) is the only sync (blocking) write path — job submit via `persist_job_hook_fun` (persist.c:1125); it blocks a vtask pool thread (default 8, `--vcpus` override, cluster.c:1082); no liveness risk since wakeups come from iomgr
|
||
- `replicator_t` is embedded in `persist_t` (`p->repl`), and `ha_state_t`/`cluster_t` reach it via `c->persist.repl` — the engine can reach the standby registry through its `persist_t*`
|
||
|
||
### Architecture Pattern & Boundary Map
|
||
|
||
```mermaid
|
||
graph TB
|
||
subgraph vtask_thread
|
||
JS[job submit hook<br/>persist_wal_sync_append]
|
||
end
|
||
subgraph iomgr_thread
|
||
PCM[peer_ctrl_msg_handler<br/>repl_ack_decode_and_dispatch]
|
||
ROA[repl_on_ack]
|
||
end
|
||
subgraph commit_engine[commit engine - commit.c]
|
||
SUB[commit_submit WAIT/RETAIN]
|
||
OA[commit_on_ack]
|
||
MC[commit_on_membership_change]
|
||
CL[(committed_lsn<br/>in persist_t)]
|
||
end
|
||
GATE[ha_dispatch_gate<br/>ha_has_write_quorum]
|
||
REG[(replicator_t standbys<br/>acked_lsn, in_catch_up)]
|
||
|
||
GATE -->|fast reject 6.1| JS
|
||
JS -->|entry lsn N| SUB
|
||
PCM --> ROA
|
||
ROA -->|standby_idx, acked_lsn| OA
|
||
ROA -->|updates| REG
|
||
OA -->|reads| REG
|
||
OA -->|advance + broadcast| CL
|
||
SUB -->|wait predicate on| CL
|
||
MC -->|wake + re-evaluate| SUB
|
||
GATE -->|repl_active_standby_count| REG
|
||
```
|
||
|
||
**Architecture Integration** (sliced from umbrella):
|
||
- Selected pattern: single commit engine as the authority for `committed_lsn`; the dispatch gate stays a fast-rejection filter (6.1), the engine is the authoritative ACK-time verifier (1.2, 4.1) — closing the TOCTOU by construction: gate-time connectivity never substitutes for acknowledgment (1.3)
|
||
- Existing patterns preserved: hook-based persistence dispatch, full fan-out broadcast, `errnox` early-return style, `ack_cond` signaling (kept for the untouched configAck track)
|
||
- New component rationale (umbrella): requirements families 2.x/4.x are one capability; placing them in `repl.c` would bloat connection management with consensus logic
|
||
- Steering compliance: iomgr callbacks never block (bounded mutex only); blocking waits on vtask threads with `n_vcpus`-relative admission cap; TDD with testsuite/ tests first
|
||
|
||
### Technology Stack
|
||
|
||
| Layer | Choice / Version | Role in Feature | Notes |
|
||
|-------|------------------|-----------------|-------|
|
||
| Backend / Services | C (gnu17), pthreads, stdatomic | commit engine | no new dependencies, no .capnp changes |
|
||
| Data / Storage | existing WAL (persist.c) | `committed_lsn` atomic in `persist_t` | no schema change; WAL_NOOP is NOT in this slice |
|
||
| Build / Test | meson, testsuite/ pattern | `test_commit_engine.c` registered in `controller_unit_test_src` | XCE_TESTING seams |
|
||
|
||
## File Structure Plan
|
||
|
||
### Directory Structure
|
||
```
|
||
src/controller/
|
||
├── commit.c # NEW — commit engine: per-LSN evaluation, waiter registry,
|
||
│ # admission cap, writable_lsn, committed_lsn advancement
|
||
├── include/commit.h # NEW — engine public API (controller-internal; umbrella-verbatim)
|
||
└── testsuite/
|
||
└── test_commit_engine.c # NEW — unit + integration tests for the engine and wiring
|
||
```
|
||
|
||
### Modified Files
|
||
- `src/include/repl.h` — `replicator_t` gains `struct commit_engine *ce` (forward-declared, NULL until wired); declares `repl_set_commit_engine()`, `repl_active_standby_count()`; `repl_wait_majority_ack()` gains an `entry_lsn` parameter
|
||
- `src/controller/repl.c` — `repl_on_ack` forwards `(standby_idx, acked_lsn)` to `commit_on_ack` after updating `acked_lsn`; `repl_add_standby` / `repl_remove_standby` / `repl_start_catchup` / `repl_complete_catchup` call `commit_on_membership_change`; walAck branch of `repl_wait_majority_ack` delegates to `commit_submit(WAIT)` when the engine is wired (legacy counting survives only for engine-less unit-test contexts and is retired by ha-commit-barrier-rollout); `repl_active_standby_count` helper added
|
||
- `src/controller/persist.c` — `persist_wal_sync_append` passes `written_lsn` to the adapter; deletes its own `committed_lsn` store (persist.c:1035) and the `has_standbys → -ENODEV` special case on the engine-wired path (the engine decides: self-quorum vs timeout)
|
||
- `src/controller/ha.c` — `ha_has_write_quorum` counts via `repl_active_standby_count` (connected AND not `in_catch_up`); `ha_state_t` gains a `replicator_t *repl` back-pointer set at cluster init
|
||
- `src/controller/ha.h` — `ha_state_t` field addition (`replicator_t *repl`)
|
||
- `src/controller/include/cluster.h` / `cluster.c` — `cluster_t` gains `commit_engine_t commit`; `commit_engine_init(&c->commit, &c->persist, ha->n_peers, c->vtaskmgr.n_vcpus)` + `repl_set_commit_engine` called at init phase after vtask pool sizing (cluster.c:1082) and peer count are known; `commit_engine_fini` on teardown
|
||
- `src/controller/meson.build` — add `commit.c` to controller sources and `testsuite/test_commit_engine.c` to `controller_unit_test_src`
|
||
- `src/controller/testsuite/test_repl_majority.c` — walAck cases updated to engine-wired per-LSN semantics (configAck cases untouched)
|
||
|
||
## System Flows
|
||
|
||
### Job submit through the engine (umbrella "request-shaped write", sliced)
|
||
|
||
```mermaid
|
||
sequenceDiagram
|
||
participant C as client
|
||
participant V as vtask thread
|
||
participant CE as commit engine
|
||
participant R as repl broadcast
|
||
participant S as standby
|
||
participant IO as iomgr thread
|
||
C->>V: job submit
|
||
V->>V: ha_dispatch_gate (incl. ha_has_write_quorum, catch-up excluded)
|
||
V->>V: insert job (provisional, unchanged order)
|
||
V->>R: persist_wal_async: append lsn=N + broadcast
|
||
V->>CE: commit_submit(N, WAIT, sync_timeout_ms)
|
||
CE->>CE: admission cap check (n_vcpus - reserved)
|
||
S->>IO: WalAck acked_lsn >= N
|
||
IO->>CE: repl_on_ack → commit_on_ack
|
||
CE->>CE: majority for N? advance committed_lsn, broadcast cond
|
||
CE-->>V: 0 (committed) | -ETIMEDOUT | -XCE_ERR_NOT_READY
|
||
alt committed
|
||
V->>C: success
|
||
else timeout / not ready
|
||
V->>C: error (operation failed)
|
||
end
|
||
```
|
||
|
||
Flow decisions: the admission cap rejects before blocking when `inflight_waits >= max(1, n_vcpus - COMMIT_WAIT_RESERVED)` (7.1, 7.2); majority is evaluated against the entry's LSN at ACK time — gate-time connectivity never substitutes (1.2, 1.3, 4.1); a timeout leaves `committed_lsn` unmoved past the entry (2.1) and the entry in the log (outcome converges per umbrella 2.5, owned downstream).
|
||
|
||
### Commit evaluation (inside `commit_on_ack`)
|
||
|
||
Under the engine mutex: collect `acked_lsn` of all standbys with `ep != NULL && !in_catch_up` into a scratch array, sort descending. With self counted, entry N has majority when at least `n_peers/2` active standbys have `acked_lsn >= N`; therefore the new commit candidate is `arr[n_peers/2 - 1]` (0-indexed; 0 candidates → no advance) clamped to the highest appended LSN. Store `committed_lsn = max(committed_lsn, candidate)` (2.2); cumulative ACK watermarks make the result contiguous by construction (2.3). Broadcast the condvar; each waiter re-checks its own predicate. Catch-up standbys are excluded from the count (umbrella-locked, conservative — their ACKs count after `repl_complete_catchup` triggers `commit_on_membership_change` re-evaluation).
|
||
|
||
## Requirements Traceability
|
||
|
||
| Requirement | Summary | Components | Flows |
|
||
|-------------|---------|------------|-------|
|
||
| 1.1 | submit confirmed only after per-entry majority | Commit Engine, Job-Submit Adapter | job submit |
|
||
| 1.2 | ACK counts toward N only if acked_lsn >= N | Commit Engine | commit evaluation |
|
||
| 1.3 | connectivity never substitutes for ACKs | Commit Engine (ACK-time eval) | job submit |
|
||
| 2.1 | no commit-index advance without majority | Commit Engine | commit evaluation |
|
||
| 2.2 | monotonic committed_lsn | Commit Engine (max() store) | commit evaluation |
|
||
| 2.3 | contiguous, bounded by appended log | Commit Engine (watermark median, clamp) | commit evaluation |
|
||
| 3.1, 3.2, 3.3 | admission quorum: only members able to receive | Quorum Accounting (`ha_has_write_quorum`, `repl_active_standby_count`) | job submit (gate) |
|
||
| 4.1 | post-acceptance shrink → fail, not confirm | Commit Engine (WAIT timeout, no false success) | job submit |
|
||
| 4.2 | membership change re-evaluates pending waits | Commit Engine (`commit_on_membership_change`), ACK Feed | — |
|
||
| 4.3, 4.4 | zero peer ACKs never majority (n_peers>1) | Commit Engine (short-circuit removal) | commit evaluation |
|
||
| 5.1 | lag never blocks majority confirmation | Commit Engine (majority threshold only) | commit evaluation |
|
||
| 6.1 | reject without quorum | Quorum Accounting (gate fast-reject) + Commit Engine | job submit |
|
||
| 6.2 | auto-resume on quorum restore | Quorum Accounting (stateless re-evaluation per request) | — |
|
||
| 6.3 | standalone self-quorum | Commit Engine (`n_peers <= 1` immediate commit) | — |
|
||
| 7.1, 7.2 | n_vcpus-relative admission cap, NOT_READY beyond | Commit Engine (admission counter) | job submit |
|
||
| 7.3 | ACK processing unaffected at cap | ACK Feed (iomgr, non-blocking) | — |
|
||
|
||
## Components and Interfaces
|
||
|
||
| Component | Layer | Intent | Req Coverage | Key Dependencies | Contracts |
|
||
|-----------|-------|--------|--------------|------------------|-----------|
|
||
| Commit Engine | controller/commit.c | authority for commit-index advancement and waiter completion | 1.1–1.3, 2.1–2.3, 4.1–4.4, 5.1, 6.3, 7.1–7.2 | repl ACK events (P0), persist_t committed_lsn (P0) | Service, State |
|
||
| ACK Feed | controller/repl.c | forward ACK + membership events to the engine from iomgr | 4.2, 7.3 | commit engine (P0), iomgr handler chain (P0) | Event |
|
||
| Quorum Accounting | controller/ha.c, repl.c helper | honest gate-time quorum, catch-up exclusion | 3.1–3.3, 6.1, 6.2 | repl registry (P0) | Service |
|
||
| Job-Submit Adapter | controller/persist.c, repl.c | route the sync append wait through the engine | 1.1, 6.3 | commit engine (P0) | Service |
|
||
|
||
### Commit Engine (`commit.c` / `include/commit.h`)
|
||
|
||
| Field | Detail |
|
||
|-------|--------|
|
||
| Intent | Single authority for per-entry commit evaluation, `committed_lsn` advancement, and waiter completion |
|
||
| Requirements | 1.1–1.3, 2.1–2.3, 4.1–4.4, 5.1, 6.3, 7.1–7.2 |
|
||
|
||
**Responsibilities & Constraints**
|
||
- Owns advancement of `persist->committed_lsn` (the atomic stays in `persist_t`; only the engine stores to it in production after init/truncation paths — the store at persist.c:1035 is deleted; the truncation store at persist.c:1438 remains)
|
||
- Commit rule (Raft §5.3, umbrella-verbatim): entry N is committed when `count(self + active standbys with acked_lsn >= N) > n_peers / 2`; advancement is contiguous and monotonic (2.1–2.3)
|
||
- Own-term gate storage: `writable_lsn` defaults to 0 = no gate (this spec never sets it); when set (by ha-single-leader-role), no waiter completes successfully until `committed_lsn >= writable_lsn`
|
||
- Admission cap: at most `max(1, n_vcpus - COMMIT_WAIT_RESERVED)` (reserved default = 2) concurrent WAIT blockers; excess fails fast with `-XCE_ERR_NOT_READY`. Expressed relative to `n_vcpus` only — never absolute (operator tunes `--vcpus`)
|
||
- `n_peers <= 1`: self-ACK constitutes majority; WAIT returns 0 immediately and advances `committed_lsn` to the entry (6.3 — intentional change: standalone nodes now advance `committed_lsn` on sync appends, Raft-correct). For `n_peers > 1`, zero peer ACKs never commit (4.3, 4.4)
|
||
|
||
**Dependencies**
|
||
- Inbound: `repl_on_ack` (iomgr) — ACK events (P0); `persist_wal_sync_append` — WAIT registration (P0); `cluster.c` init — lifecycle (P0); membership events from repl registry mutations (P0)
|
||
- Outbound: none (reads `p->repl` registry and stores `p->committed_lsn`); signals waiters via condvar broadcast with per-LSN predicates
|
||
|
||
**Contracts**: Service [x] / State [x] / Event [x]
|
||
|
||
##### Service Interface (umbrella-verbatim, `src/controller/include/commit.h`)
|
||
```c
|
||
/* include/commit.h — all functions thread-safe; _on_ack callable from iomgr (non-blocking) */
|
||
typedef enum { COMMIT_MODE_WAIT, COMMIT_MODE_RETAIN } commit_mode_t;
|
||
|
||
int commit_engine_init(commit_engine_t *ce, persist_t *p, int n_peers, int n_vcpus);
|
||
void commit_engine_fini(commit_engine_t *ce);
|
||
|
||
/* Register entry at append time. RETAIN returns 0 immediately.
|
||
* WAIT blocks up to timeout_ms: 0 = committed; -ETIMEDOUT = not committed
|
||
* (entry remains in log; outcome resolves per umbrella 2.5); -XCE_ERR_NOT_READY =
|
||
* admission cap exhausted (only cause). An unmet own-term gate does NOT fail
|
||
* fast: the gate is part of the completion predicate, so WAIT blocks within
|
||
* timeout_ms until the gate opens, surfacing -ETIMEDOUT if it never does. */
|
||
int commit_submit(commit_engine_t *ce, uint64_t lsn, commit_mode_t mode,
|
||
uint32_t timeout_ms);
|
||
|
||
/* iomgr path: record standby ACK, evaluate majority, advance committed_lsn
|
||
* contiguously, wake satisfied waiters. Never blocks. */
|
||
void commit_on_ack(commit_engine_t *ce, int standby_idx, uint64_t acked_lsn);
|
||
|
||
/* Promotion: set own-term gate to the no-op entry's LSN.
|
||
* (API ships here; no caller in this spec — unset = no gate.) */
|
||
void commit_set_writable_lsn(commit_engine_t *ce, uint64_t noop_lsn);
|
||
|
||
/* Quorum-membership change (standby add/remove/catch-up transition):
|
||
* re-evaluate pending entries; standbys in catch-up are excluded. */
|
||
void commit_on_membership_change(commit_engine_t *ce);
|
||
```
|
||
- Preconditions: entries submitted in LSN order (single-writer guarantees this); `commit_engine_init` called after `n_peers` and `n_vcpus` are final
|
||
- Postconditions: `committed_lsn` never decreases (2.2); never exceeds the highest majority-acked contiguous LSN nor the highest appended LSN (2.1, 2.3)
|
||
- Invariants: no waiter completes successfully while `committed_lsn < its lsn` or (gate set) `committed_lsn < writable_lsn`; `inflight_waits <= max(1, n_vcpus - COMMIT_WAIT_RESERVED)`
|
||
|
||
##### State Management
|
||
- State: per-standby view delegated to `standby_conn_t.acked_lsn` / `in_catch_up` (no duplication); internal: sorted matchIndex scratch array (`MAX_STANDBYS`), waiter accounting (per-LSN predicate over shared condvar), `writable_lsn`, `inflight_waits` counter, `n_peers`, `n_vcpus`, `persist_t *p` back-reference
|
||
- Concurrency: one mutex + one condvar (broadcast on `committed_lsn` advance and on membership change; waiters re-check per-LSN predicate — umbrella-accepted broadcast cost); `committed_lsn` read lock-free via the existing atomic; mutex hold in `commit_on_ack` is bounded (scan + sort of ≤ MAX_STANDBYS entries, no I/O) — safe on iomgr
|
||
- Re-evaluation semantics (4.2): membership change broadcasts the condvar so pending waiters re-check against updated membership; waiters whose entries gained majority complete with 0, others keep waiting until their deadline and fail with `-ETIMEDOUT` (4.1 — no confirmation ever happens without actual majority ACK; early-fail on shrink is permitted but not required by the umbrella, and is deliberately not implemented to allow reconnection within the timeout)
|
||
|
||
**Implementation Notes**
|
||
- Integration: instance embedded in `cluster_t`; wired to repl via `repl_set_commit_engine(&c->persist.repl, &c->commit)`; XCE_TESTING tests construct the engine standalone against a stub `persist_t` + `replicator_t`
|
||
- Validation: assert monotonicity and the inflight invariant under `XCE_TESTING`; log a counter of committed-vs-appended lag (retained-uncommitted depth) for observability
|
||
- Risks: condvar broadcast storms under high write concurrency — accepted at current scale (umbrella); revisit with async continuation (out of scope)
|
||
|
||
### ACK Feed (`repl.c`)
|
||
|
||
| Field | Detail |
|
||
|-------|--------|
|
||
| Intent | Deliver ACK and membership events from the iomgr thread to the engine without blocking |
|
||
| Requirements | 4.2, 7.3 |
|
||
|
||
Summary: `repl_on_ack` (repl.c:176) — after `atomic_store(&s->acked_lsn, acked_lsn)` and the existing `ack_cond` signal (kept for configAck) — calls `commit_on_ack(r->ce, slot_idx, acked_lsn)` when `r->ce != NULL`. `repl_add_standby`, `repl_remove_standby`, `repl_start_catchup`, `repl_complete_catchup` call `commit_on_membership_change(r->ce)` after mutating the registry. All call sites are iomgr-driven; `commit_on_*` never block (7.3). `replicator_t` gains the forward-declared `struct commit_engine *ce` pointer (NULL = not wired) so `src/include/repl.h` keeps zero controller-header dependencies.
|
||
|
||
### Quorum Accounting (`ha.c` + `repl.c` helper)
|
||
|
||
| Field | Detail |
|
||
|-------|--------|
|
||
| Intent | Honest gate-time write-admission quorum: count only members able to receive new entries |
|
||
| Requirements | 3.1–3.3, 6.1, 6.2 |
|
||
|
||
Summary (umbrella §Quorum Accounting, sliced): new `int repl_active_standby_count(replicator_t *r)` counts slots with `ep != NULL && !in_catch_up` under `r->lock`. `ha_has_write_quorum` (ha.c:1507) replaces `standby_endpoints->count` with this helper via a `replicator_t *repl` back-pointer on `ha_state_t` (set during cluster init; when NULL — legacy test contexts — fall back to the current `standby_endpoints` count). Quorum condition unchanged: `n_peers <= 1 → true`; else `active >= n_peers / 2` (3.1–3.3). The gate stays a fast-rejection filter (6.1, `-XCE_ERR_NOT_READY` per existing `ha_dispatch_gate`); it is stateless per request, so quorum restoration auto-resumes acceptance with no operator action (6.2); the engine re-verifies authoritatively at ACK time (1.3, 4.1). Quorum-loss transition logging stays out (ha-convergence-recovery).
|
||
|
||
### Job-Submit Adapter (`persist.c` + `repl.c`)
|
||
|
||
| Field | Detail |
|
||
|-------|--------|
|
||
| Intent | Route the existing job-submit sync wait through the engine; preserve external return-code contract |
|
||
| Requirements | 1.1, 4.3, 4.4, 6.3 |
|
||
|
||
Summary (umbrella migration note: "`repl_wait_majority_ack` becomes a thin adapter"): `repl_wait_majority_ack(r, timeout_ms, ack_op)` gains an `entry_lsn` parameter. walAck branch: when `r->ce` is wired, `return commit_submit(r->ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms)`; legacy counting survives only for engine-less unit-test contexts (deprecated comment; retired by ha-commit-barrier-rollout when call sites move to the engine directly). configAck branch: unchanged, `entry_lsn` ignored. `persist_wal_sync_append` (persist.c:1003): passes `written_lsn`; on the engine path, deletes the `has_standbys → -ENODEV` special case and its own `committed_lsn` store — the engine returns 0 (and has advanced `committed_lsn`) for `n_peers <= 1` (6.3), and `-ETIMEDOUT` for `n_peers > 1` with no covering ACKs (4.3, 4.4 — the `n_active == 0 → success` short-circuit is thereby removed from production). Return contract to `persist_job_hook_fun` callers is unchanged (0 / `-ETIMEDOUT` / now also `-XCE_ERR_NOT_READY`), and the existing error propagation to the job-submit client (job.c reference path) carries it (1.1).
|
||
|
||
## Data Models
|
||
|
||
### Domain Model (changes only)
|
||
- **commit_engine_t** (in-memory only; commit state volatile, Raft-conformant — umbrella-locked): `{ pthread_mutex_t lock; pthread_cond_t cond; persist_t *p; int n_peers; int n_vcpus; int inflight_waits; uint64_t writable_lsn; uint64_t scratch[MAX_STANDBYS]; }`
|
||
- **replicator_t**: + `struct commit_engine *ce` (NULL until wired)
|
||
- **ha_state_t**: + `replicator_t *repl` (NULL in legacy test contexts)
|
||
- **cluster_t**: + `commit_engine_t commit`
|
||
- Invariants: `committed_lsn` monotonic, contiguous, ≤ highest appended LSN; `inflight_waits` within cap; engine never duplicates per-standby state
|
||
|
||
## Error Handling
|
||
|
||
### Error Strategy
|
||
- `commit_submit(WAIT)` returns `0` (committed), `-ETIMEDOUT` (not committed within timeout — entry remains in log; cluster-wide outcome convergence is umbrella 2.5, owned by ha-convergence-recovery), `-XCE_ERR_NOT_READY` (admission-cap exhaustion only — an unmet own-term gate is not a fail-fast cause: it is part of the WAIT completion predicate, so the waiter blocks within its timeout until the gate opens and surfaces `-ETIMEDOUT` otherwise) — distinct log messages per cause, both retriable
|
||
- Errors propagate via the existing `errnox` early-return pattern: `persist_wal_sync_append` → `persist_job_hook_fun` → job-submit reply; no in-memory rollback (provisional-state model, umbrella-locked)
|
||
- Gate rejection (no write quorum) keeps the existing `-XCE_ERR_NOT_READY` dispatch-gate behavior (6.1)
|
||
- `commit_engine_init` validates `n_peers >= 0`, `n_vcpus >= 1`, non-NULL `persist_t`; returns `-EINVAL` otherwise
|
||
|
||
### Monitoring
|
||
- Log counters: inflight WAIT count vs cap; committed-vs-appended lag; per-cause rejection WARNs (rate-limited to transitions where applicable). Metrics export out of scope.
|
||
|
||
## Testing Strategy
|
||
|
||
### Unit Tests (`testsuite/test_commit_engine.c`, XCE_TESTING, stub persist/replicator)
|
||
- Per-LSN majority: with n_peers=3, ACK for LSN M does not commit entry N>M (`acked_lsn > 0` regression dies) (1.2); ACK `>= N` from one standby + self commits N (1.1)
|
||
- Contiguity + monotonicity: out-of-order/cumulative ACK watermarks advance `committed_lsn` to the median watermark, never past appended log, never backwards (2.1–2.3)
|
||
- Zero-peer rule: n_peers=3, no standby ACKs → WAIT returns `-ETIMEDOUT`, `committed_lsn` unmoved (4.3, 4.4); n_peers=1 → WAIT returns 0 immediately, `committed_lsn` advanced (6.3)
|
||
- Admission cap: with n_vcpus=8 reserved=2, the 7th concurrent WAIT gets `-XCE_ERR_NOT_READY`; cap floor `max(1, ...)` at tiny n_vcpus; ACKs still advance commits for admitted waiters while cap is saturated (7.1–7.3)
|
||
- Own-term gate storage: `writable_lsn` unset → no gating; set above entry → waiter held until `committed_lsn >= writable_lsn` (interface contract for downstream; no production caller here)
|
||
- Membership re-evaluation: catch-up standby's ACK excluded; after `commit_on_membership_change` (catch-up complete) its watermark counts and the pending waiter completes (4.2); waiter never falsely completes on shrink (4.1)
|
||
- Lag tolerance: n_peers=3, one standby far behind — majority ACK still commits (5.1)
|
||
|
||
### Integration Tests (testsuite/)
|
||
- Quorum accounting: `repl_active_standby_count` excludes `in_catch_up`; `ha_has_write_quorum` false when the only connected standby is in catch-up (n_peers=3), true again after `repl_complete_catchup`; gate re-accepts after reconnection without any reset call (3.1–3.3, 6.1, 6.2)
|
||
- ACK feed: `repl_on_ack` with wired engine updates `acked_lsn` AND advances `committed_lsn` when majority reached (4.2 wiring)
|
||
- Sync-append path: `persist_wal_sync_append` with wired engine — success only on per-entry majority; `-ETIMEDOUT` with stale `acked_lsn > 0` from an earlier entry (the dishonest counting regression test); `committed_lsn` unmoved on timeout (1.1, 1.2, 2.1)
|
||
- `test_repl_majority.c` updated: walAck cases wire an engine and assert per-LSN semantics; configAck cases byte-identical
|
||
|
||
### Regression
|
||
- Full controller testsuite + ASAN (`/xce-build controller`) green; no production walAck path reaches the legacy `acked_lsn > 0` counting
|
||
|
||
## Performance & Scalability
|
||
- Commit latency unchanged from the ha-wal-replication baseline (majority RTT + standby fsync); `commit_on_ack` adds an O(MAX_STANDBYS log MAX_STANDBYS) scan per ACK under a short mutex — negligible
|
||
- Throughput under degradation bounded by the admission cap — deliberate CP rejection, `n_vcpus`-relative; sizing guidance is operator-doc material (umbrella research 3(e)), not spec constants
|
||
|
||
## Migration Strategy
|
||
This spec is phase 1 of the umbrella's hard-ordered landing ("barrier before state removal"): commit engine + per-LSN evaluation behind the existing job-submit path — a behavioral fix only, no new write paths. The adapter shell (`repl_wait_majority_ack`) and the engine-less legacy fallback are explicitly transitional; ha-commit-barrier-rollout moves call sites onto the engine directly and retires the fallback. Rollback trigger: controller testsuite regression or job-submit E2E failure — reverting this spec restores the prior (dishonest but functional) barrier, leaving the system no less consistent than before.
|