forked from lei.chen/myxce
feat(ha-commit-barrier-rollout): persist_commit_barrier + legacy walAck retirement (task 2.2)
Barrier: watermark = current_lsn - 1 under p->lock (released before blocking), covered/nothing-appended/LSN-0 short-circuits, sync_timeout_ms fallback, engine code unmodified, ce==NULL vacuous. repl.c walAck track now engine-only (legacy acked_lsn>0 counting and walAck-reachable n_active==0 success deleted); configAck track byte-equivalent for task 5. 15/15 group; census 472/472.
This commit is contained in:
parent
611fbc8d7b
commit
8f688eace1
5 changed files with 106 additions and 54 deletions
|
|
@ -25,7 +25,7 @@
|
|||
- Observable completion: new cases FAIL (red) — the helper does not exist yet
|
||||
- _Requirements: 1.1, 1.2, 1.3_
|
||||
|
||||
- [ ] 2.2 Implement the barrier helper and retire the legacy walAck counting
|
||||
- [x] 2.2 Implement the barrier helper and retire the legacy walAck counting
|
||||
- Implement `persist_commit_barrier` in persist.c: snapshot `current_lsn` under `p->lock` (mirror persist.c:1019-1021), short-circuit when covered, else `commit_submit(ce, lsn, COMMIT_MODE_WAIT, timeout)`; return engine code unmodified
|
||||
- repl.c `repl_wait_majority_ack` (repl.c:386-462 anchor): remove the legacy walAck counting branch (upstream marked it "retired by ha-commit-barrier-rollout") — the walAck path delegates to the engine unconditionally; the function's remaining body is the configAck track (hardened in task 5)
|
||||
- Observable completion: all task 2.1 tests PASS; grep shows no remaining `acked_lsn > 0` counting and no `n_active == 0 → success` short-circuit reachable from any walAck call path
|
||||
|
|
@ -112,4 +112,5 @@
|
|||
- (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).
|
||||
- (2.2) Legacy walAck counting RETIRED: walAck track = entry_lsn==0 → 0 (LSN-0 interim) else unconditional commit_submit (ce==NULL → -EINVAL, fail-loud). configAck track byte-equivalent (task 5's surface intact, incl. its n_active==0 shortcut and acked_config_lsn>0 counting — both die at task 5). persist_commit_barrier: ce==NULL → 0 (vacuous, documented). test_repl_unit's two lsn=0 walAck callers are now vacuous for quorum counting (INFO for task 5/7.2). CENSUS = 472 (465 at 1.2 + 7 from 2.1).
|
||||
- (2.1) BARRIER WATERMARK RESOLVED (reviewer-judged): design's literal "Snapshot lsn = p->current_lsn" is loose phrasing contradicted by its own anchors — the coherent contract is watermark = current_lsn - 1 (last appended; persist.h:71 invariant committed_lsn <= current_lsn - 1 makes the literal reading unsatisfiable). Tests pin current_lsn - 1 with off-by-one killing power in BOTH directions; task 2.2 must implement that. Resolved contract recorded in persist.h doc comment.
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
<!-- How the user likes things done. Code style, tools, patterns, communication. -->
|
||||
|
||||
## Key Learnings
|
||||
- repl_wait_majority_ack walAck track is engine-only since barrier-rollout 2.2: entry_lsn==0 returns 0 immediately (documented once-per-lifetime LSN-0 first entry), everything else delegates to commit_submit unconditionally — ce==NULL surfaces as -EINVAL from commit_submit (unreachable in production; cluster_init_phase2 always wires the engine). The counting loop that remains in the function is configAck-ONLY (no self-count, n_active==0 shortcut is Req 8.5 config behavior). persist_wal_sync_append's residual non-engine branch can only store committed_lsn for the LSN-0 no-op case. (2026-06-12, task 2.2)
|
||||
- `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.
|
||||
|
||||
|
|
|
|||
|
|
@ -994,3 +994,12 @@
|
|||
| 05:05 | ha-commit-barrier-rollout task 2.1: RED tests for persist_commit_barrier (7 cases) + decl + inert -ENOSYS stub | test_commit_barrier_rollout.c, persist.h, persist.c | 7 FAIL (RED) on -38 sentinel, prior 8 PASS, suite census clean | ~60k |
|
||||
| 22:08 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~17 |
|
||||
| 22:08 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified RESOLVED() | ~256 |
|
||||
| 22:14 | Edited src/controller/persist.c | added 4 condition(s) | ~640 |
|
||||
| 22:15 | Edited src/controller/repl.c | added 1 condition(s) | ~657 |
|
||||
| 22:15 | Edited src/controller/repl.c | modified if() | ~180 |
|
||||
| 22:16 | Edited src/controller/persist.c | modified if() | ~342 |
|
||||
| 22:20 | Edited src/controller/repl.c | 5→5 lines | ~96 |
|
||||
| 05:25 | barrier-rollout 2.2: persist_commit_barrier implemented (watermark=current_lsn-1, covered short-circuit, ce==NULL→0, timeout fallback) + legacy walAck counting removed from repl_wait_majority_ack (LSN-0→0, unconditional commit_submit; configAck track untouched) | src/controller/persist.c, src/controller/repl.c | 15/15 group, 472 Unity / 0 FAIL | ~35k |
|
||||
| 05:45 | Adversarial review ha-commit-barrier-rollout task 2.2 (barrier helper + walAck retirement): boundary clean (persist.c+repl.c only), 15/15 group, full census 472 Unity PASS / 0 FAIL (= 465 prior + 7 new), configAck byte-equivalent, LSN-0 absorption commented, ce==NULL asymmetry judged defensible — APPROVED | src/controller/persist.c, src/controller/repl.c | APPROVED | ~30k |
|
||||
| 22:36 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~21 |
|
||||
| 22:36 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified RESOLVED() | ~148 |
|
||||
|
|
|
|||
|
|
@ -1042,10 +1042,13 @@ int persist_wal_sync_append(persist_t* p, uint32_t type,
|
|||
if (sync_err != 0)
|
||||
CTRL_METRIC_INC(ctrl_repl_degraded_count);
|
||||
} else {
|
||||
/* DEPRECATED legacy branch: engine-less (ce == NULL) unit-test
|
||||
* contexts plus the LSN-0 first entry. Connectivity-based counting;
|
||||
* committed_lsn is stored here only because no engine owns it.
|
||||
* Retired by ha-commit-barrier-rollout. */
|
||||
/* Residual non-engine branch: engine-less (ce == NULL) unit-test
|
||||
* contexts plus the LSN-0 first entry. The legacy connectivity-
|
||||
* based counting was retired by ha-commit-barrier-rollout task 2.2:
|
||||
* repl_wait_majority_ack now confirms LSN 0 immediately (documented
|
||||
* once-per-lifetime edge) and surfaces engine-less walAck waits as
|
||||
* -EINVAL via commit_submit, so the committed_lsn store below only
|
||||
* fires for the LSN-0 case (a no-op store of 0). */
|
||||
mylock_lock(&p->repl.lock);
|
||||
int has_standbys = (p->repl.nstandbys > 0);
|
||||
mylock_unlock(&p->repl.lock);
|
||||
|
|
@ -1171,19 +1174,58 @@ wal_commit_mode_t wal_classify(uint32_t wal_type, int detail)
|
|||
}
|
||||
}
|
||||
|
||||
/* Commit barrier helper — INERT RED stub (ha-commit-barrier-rollout task
|
||||
* 2.1; implemented in task 2.2). Returns a wrong sentinel so the task-2.1
|
||||
* tests fail on assertions, not linking. Task-2.2 contract (design.md
|
||||
* §Commit Barrier Helper): snapshot the appended watermark (current_lsn - 1
|
||||
* under p->lock), return 0 when committed_lsn already covers it (or nothing
|
||||
* appended), else commit_submit(WAIT, timeout_ms ? timeout_ms :
|
||||
* p->config.sync_timeout_ms) with the engine code passed through
|
||||
* unmodified. */
|
||||
/* Commit barrier helper (ha-commit-barrier-rollout task 2.2, design.md
|
||||
* §Commit Barrier Helper; Req 1.1, 1.2, 1.3).
|
||||
*
|
||||
* Handler-callable WAIT completion on the appended watermark: snapshot the
|
||||
* last-appended LSN, return 0 when the commit index already covers it,
|
||||
* else block in the commit engine until coverage or deadline. The engine
|
||||
* result is returned UNMODIFIED (0 / -ETIMEDOUT / -XCE_ERR_NOT_READY /
|
||||
* -EINVAL) so callers map it to their own error replies.
|
||||
*
|
||||
* Lock discipline: p->lock is held only for the current_lsn snapshot and
|
||||
* is released BEFORE commit_submit(WAIT) — the engine blocks on ce->lock
|
||||
* + condvar, and p->lock must never be held across it (engine
|
||||
* lock-ordering rule). */
|
||||
int persist_commit_barrier(persist_t* p, uint32_t timeout_ms)
|
||||
{
|
||||
(void)p;
|
||||
(void)timeout_ms;
|
||||
return -ENOSYS;
|
||||
uint64_t watermark;
|
||||
|
||||
if (p == NULL)
|
||||
return -EINVAL;
|
||||
|
||||
/* Snapshot the last-appended watermark under p->lock. current_lsn is
|
||||
* the NEXT LSN to assign (persist.h invariant: committed_lsn <=
|
||||
* current_lsn - 1), so the highest appended entry is current_lsn - 1
|
||||
* (resolved contract, spec Implementation Notes 2.1). current_lsn ==
|
||||
* 0 means nothing was ever appended — vacuously satisfied. */
|
||||
pthread_mutex_lock(&p->lock);
|
||||
if (p->current_lsn == 0) {
|
||||
pthread_mutex_unlock(&p->lock);
|
||||
return 0;
|
||||
}
|
||||
watermark = p->current_lsn - 1;
|
||||
pthread_mutex_unlock(&p->lock);
|
||||
|
||||
/* Already covered — committed_lsn is monotone, so coverage observed
|
||||
* here can never regress. This check also absorbs the LSN-0 first
|
||||
* entry (watermark 0 vs committed_lsn 0): that once-per-lifetime entry
|
||||
* has no quorum contract (ha-commit-engine Implementation Notes) and
|
||||
* commit_submit would reject lsn == 0 anyway. */
|
||||
if (atomic_load_explicit(&p->committed_lsn, memory_order_acquire) >=
|
||||
watermark)
|
||||
return 0;
|
||||
|
||||
/* Engine not wired: legacy/bootstrap contexts have no engine commit
|
||||
* contract — there is nothing to wait on, so the barrier is a no-op.
|
||||
* Production always wires the engine (cluster_init_phase2). */
|
||||
if (p->repl.ce == NULL)
|
||||
return 0;
|
||||
|
||||
/* timeout_ms == 0 borrows the configured sync deadline (Req 1.2). */
|
||||
return commit_submit(p->repl.ce, watermark, COMMIT_MODE_WAIT,
|
||||
timeout_ms != 0 ? timeout_ms
|
||||
: p->config.sync_timeout_ms);
|
||||
}
|
||||
|
||||
/* Classified append at the hook seam (barrier-rollout task 1.2).
|
||||
|
|
|
|||
|
|
@ -399,17 +399,19 @@ void repl_on_config_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_config_lsn
|
|||
}
|
||||
|
||||
/* ============================================================
|
||||
* Majority-quorum ACK wait (task 7.3)
|
||||
* Majority-quorum ACK wait (task 7.3; walAck track rebased onto the
|
||||
* commit engine by ha-commit-barrier-rollout task 2.2)
|
||||
*
|
||||
* Quorum formula: quorum = (n_active + 2) / 2 [ceiling of (n_active+1)/2]
|
||||
* n_active = count of active (non-catch-up) standbys.
|
||||
*
|
||||
* walAck: acked_total = 1 (self) + standbys with acked_lsn > 0.
|
||||
* Primary counts itself — it persists WAL before broadcasting.
|
||||
* configAck: acked_total = standbys with acked_config_lsn > 0 (no self-count).
|
||||
* Config must be confirmed by standbys; primary does not self-count.
|
||||
*
|
||||
* Returns 0 on quorum, -ETIMEDOUT on timeout.
|
||||
* walAck: per-LSN commit evaluation, delegated to the commit engine
|
||||
* unconditionally (the legacy connectivity-based "ACKed
|
||||
* anything ever" counting with self-count and the zero-active
|
||||
* success short-circuit is retired). Returns the engine code
|
||||
* unmodified: 0 / -ETIMEDOUT / -XCE_ERR_NOT_READY / -EINVAL.
|
||||
* configAck: legacy standby counting (hardened by barrier-rollout task 5):
|
||||
* quorum = (n_active + 2) / 2 over active (non-catch-up)
|
||||
* standbys; acked_total = standbys with acked_config_lsn > 0,
|
||||
* NO self-count (config must be confirmed by standbys).
|
||||
* Returns 0 on quorum, -ETIMEDOUT on timeout.
|
||||
* ============================================================ */
|
||||
|
||||
int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn,
|
||||
|
|
@ -418,27 +420,28 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn,
|
|||
if (r == NULL)
|
||||
return -EINVAL;
|
||||
|
||||
int is_wal_ack = (ack_op == (uint8_t)OPCode_walAck);
|
||||
if (ack_op == (uint8_t)OPCode_walAck) {
|
||||
/* LSN-0 edge: the first-ever WAL entry of a fresh cluster is LSN 0
|
||||
* (current_lsn starts at 0; written_lsn = current_lsn - 1) and is
|
||||
* structurally un-commitable — commit_submit rejects lsn == 0.
|
||||
* Documented once-per-cluster-lifetime interim behavior
|
||||
* (ha-commit-engine Implementation Notes): confirm it immediately;
|
||||
* the promotion no-op entry (ha-single-leader-role) eliminates the
|
||||
* case in practice. configAck callers also pass entry_lsn = 0 but
|
||||
* are dispatched by ack_op above, never through this guard. */
|
||||
if (entry_lsn == 0)
|
||||
return 0;
|
||||
|
||||
/* Engine adapter (ha-commit-engine task 5.2, design §Job-Submit Adapter):
|
||||
* the walAck barrier is per-LSN commit evaluation owned by the commit
|
||||
* engine when wired. Caller holds no repl locks here, so the engine's
|
||||
* ce->lock -> r->lock order is preserved.
|
||||
*
|
||||
* entry_lsn == 0 falls through to legacy counting: persist.c's
|
||||
* persist_wal_sync_append still passes 0 until task 5.3 forwards
|
||||
* written_lsn (and commit_submit rejects lsn==0 with -EINVAL). Note the
|
||||
* first-ever WAL entry IS LSN 0 — structurally un-commitable under the
|
||||
* current numbering; task 5.3 documents the resolution, and the
|
||||
* promotion no-op entry (ha-single-leader-role) eliminates it in
|
||||
* practice. configAck callers pass entry_lsn=0 and never delegate. */
|
||||
if (is_wal_ack && r->ce != NULL && entry_lsn != 0) {
|
||||
/* Unconditional engine delegation (ha-commit-engine task 5.2,
|
||||
* design §Job-Submit Adapter). Caller holds no repl locks here, so
|
||||
* the engine's ce->lock -> r->lock order is preserved. ce == NULL
|
||||
* (engine never wired) is unreachable in production — the engine
|
||||
* is wired at cluster_init_phase2 — and commit_submit surfaces it
|
||||
* as -EINVAL rather than this adapter fabricating a quorum result. */
|
||||
return commit_submit(r->ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms);
|
||||
}
|
||||
|
||||
/* DEPRECATED legacy counting below (connectivity-based, not per-LSN):
|
||||
* survives only for engine-less (ce == NULL) unit-test contexts and the
|
||||
* configAck branch; retired by ha-commit-barrier-rollout. */
|
||||
/* ── configAck track (legacy standby counting; task 5 hardens it) ── */
|
||||
|
||||
struct timespec deadline;
|
||||
clock_gettime(CLOCK_REALTIME, &deadline);
|
||||
|
|
@ -465,27 +468,23 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn,
|
|||
if (atomic_load_explicit(&s->in_catch_up, memory_order_acquire))
|
||||
continue;
|
||||
n_active++;
|
||||
if (is_wal_ack) {
|
||||
if (atomic_load_explicit(&s->acked_lsn, memory_order_acquire) > 0)
|
||||
acked_count++;
|
||||
} else {
|
||||
if (atomic_load_explicit(&s->acked_config_lsn,
|
||||
memory_order_acquire) > 0)
|
||||
acked_count++;
|
||||
}
|
||||
if (atomic_load_explicit(&s->acked_config_lsn,
|
||||
memory_order_acquire) > 0)
|
||||
acked_count++;
|
||||
}
|
||||
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Single-node shortcircuit: no active standbys → local write is quorum.
|
||||
* Applies to both walAck (self=1, quorum=1) and configAck (Req 8.5). */
|
||||
/* Single-node shortcircuit: no active standbys → local write is
|
||||
* quorum (configAck only, Req 8.5 — walAck never reaches this
|
||||
* loop; its single-member case is the engine's self-quorum). */
|
||||
if (n_active == 0) {
|
||||
rc = 0;
|
||||
break;
|
||||
}
|
||||
|
||||
int quorum = (n_active + 2) / 2;
|
||||
int acked_total = is_wal_ack ? (1 + acked_count) : acked_count;
|
||||
int acked_total = acked_count; /* no self-count for configAck */
|
||||
|
||||
if (acked_total >= quorum) {
|
||||
rc = 0;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue