feat(ha-commit-engine): repl-side wiring — ACK feed, membership hooks, adapter (task 5.2)
replicator_t gains ce pointer (set-once-before-traffic contract). repl_on_ack feeds commit_on_ack after releasing r->lock (iomgr-safe); four membership mutators feed commit_on_membership_change post-unlock, gated on actual mutation. walAck branch delegates to commit_submit(WAIT) when ce wired AND entry_lsn != 0 (LSN-0 interim guard until 5.3); legacy counting kept for ce==NULL, marked DEPRECATED. commit.c linked into production target (inert until 5.3 wiring). Tests 30-31 + all repl_majority walAck cases green; 33/34 remain RED for 5.3; baseline unchanged.
This commit is contained in:
parent
d1353360cc
commit
6600fd5d1f
6 changed files with 104 additions and 15 deletions
|
|
@ -59,7 +59,7 @@
|
|||
- Observable completion: new/updated cases FAIL (red) against unwired production code
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 4.2, 4.4, 6.3_
|
||||
|
||||
- [ ] 5.2 Implement the repl-side wiring: ACK feed, membership hooks, adapter
|
||||
- [x] 5.2 Implement the repl-side wiring: ACK feed, membership hooks, adapter
|
||||
- repl.h: forward-declare `struct commit_engine`, add `ce` pointer to `replicator_t`, declare `repl_set_commit_engine()`; add `uint64_t entry_lsn` parameter to `repl_wait_majority_ack` (update the config_repl.c:916 caller to pass 0 — configAck branch behavior untouched)
|
||||
- repl.c: `repl_on_ack` calls `commit_on_ack(r->ce, slot_idx, acked_lsn)` after the `acked_lsn` store when wired; `repl_add_standby`/`repl_remove_standby`/`repl_start_catchup`/`repl_complete_catchup` call `commit_on_membership_change` when wired; walAck branch of `repl_wait_majority_ack` delegates to `commit_submit(ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms)` when wired, with the legacy counting kept only for `ce == NULL` test contexts and marked deprecated (retired by ha-commit-barrier-rollout)
|
||||
- Observable completion: ACK-feed and membership-hook tests from 5.1 PASS; configAck tests unaffected
|
||||
|
|
@ -92,3 +92,7 @@
|
|||
- (5.1) persist.c currently passes entry_lsn=0 to the adapter — if 5.2 lands walAck delegation before 5.3 passes written_lsn, the wired sync path returns -EINVAL. Land 5.2's delegation gated on ce!=NULL AND entry_lsn!=0 falling back to legacy, OR land 5.2+5.3 wiring in lockstep.
|
||||
- (5.1) Baseline failure root causes identified: test_committed_lsn_does_not_advance_on_timeout (test_repl_unit.c:294) expects the OLD contract (rc==0 + sync_rc out-param) but persist_wal_sync_append now rejects-on-timeout — STALE TEST EXPECTATION, fix the test at 6.1, not the code. test_p2_integration.c:test_single_node_committed_lsn_unchanged pins the -ENODEV degraded path that 5.3 replaces — update it at 5.3/6.1.
|
||||
- (5.1) Repo hygiene (pre-existing, out of scope): untracked src/lib/testsuite/test_wal_wire.c is referenced by committed src/lib/meson.build:176 — should be added to git eventually.
|
||||
- (5.2) commit.c linked into production controller_src ONE TASK EARLY (meson.build:3) — necessary (repl.c references engine symbols) and reviewer-verified inert (zero production callers of commit_engine_init/repl_set_commit_engine until 5.3; repl_init memsets ce=NULL → all new paths dormant).
|
||||
- (5.2) repl_broadcast's over-limit eviction (in_catch_up=1, repl.c:268-277) deliberately does NOT feed commit_on_membership_change — reviewer-proven no-op for waiter correctness (shrink can never raise the majority candidate; monotonic store). Design lists exactly four mutators; eviction recovery fires the hook at repl_complete_catchup.
|
||||
- (5.2) r->ce is read unlocked under a set-once-before-traffic contract (repl.h:47-52). 5.3 must wire it at cluster init BEFORE replication traffic; never NULL it concurrent with traffic (fini ordering is 5.3's responsibility).
|
||||
- (5.2) Remaining RED going into 5.3: test 33 (Expected -110 Was 0 — persist passes entry_lsn=0 → legacy fallback) and test 34 (Expected 0 Was -19 — -ENODEV special case). These plus test 32's honest-delegation green are 5.3's completion evidence.
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
<!-- How the user likes things done. Code style, tools, patterns, communication. -->
|
||||
|
||||
## Key Learnings
|
||||
- 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`.
|
||||
- Logging macros in utils.h: TRACE (= tlog_debug3, verbose), INFO, WARN. There is NO DEBUG3 macro — use TRACE for debug-level logging in repl.c and other controller files. (2026-06-10)
|
||||
|
|
|
|||
|
|
@ -927,3 +927,16 @@
|
|||
| 5.1-RED-done | RED captured: 9 new failures (4 commit_engine #30/31/33/34, 5 repl_majority walAck) + 3 baselines exact; build green; test 32 pins positive contract (green in RED by design) | census /tmp/census_final.log | ok | ~90k |
|
||||
| 16:08 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~22 |
|
||||
| 16:08 | Edited .kiro/specs/ha-commit-engine/tasks.md | modified hygiene() | ~517 |
|
||||
| 16:14 | Edited src/include/repl.h | modified struct() | ~202 |
|
||||
| 16:14 | Edited src/controller/repl.c | 3→4 lines | ~52 |
|
||||
| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~298 |
|
||||
| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~115 |
|
||||
| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~211 |
|
||||
| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~208 |
|
||||
| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~241 |
|
||||
| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~368 |
|
||||
| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~136 |
|
||||
| 16:16 | Edited src/controller/meson.build | 3→4 lines | ~18 |
|
||||
| 23:35 | ha-commit-engine 5.2: wired ce into replicator_t, ACK feed (repl_on_ack→commit_on_ack), 4 membership hooks, walAck delegation to commit_submit (gated ce!=NULL && entry_lsn!=0); added commit.c to controller_src for prod link | src/include/repl.h, src/controller/repl.c, src/controller/meson.build | tests 30-32 + 5 repl_majority walAck GREEN; 33/34 stay RED until 5.3; 3 baselines unchanged | ~45k |
|
||||
| 16:32 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~21 |
|
||||
| 16:32 | Edited .kiro/specs/ha-commit-engine/tasks.md | added error handling | ~345 |
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
controller_src = [
|
||||
'cluster.c',
|
||||
'commit.c',
|
||||
'config_repl.c',
|
||||
'dataex.c',
|
||||
'ha.c',
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
|
||||
#include "controller.capnp.h" /* OPCode_walAck, OPCode_upgradeConfigAck */
|
||||
#include "repl.h"
|
||||
#include "commit.h" /* commit engine feed + adapter (ha-commit-engine 5.2) */
|
||||
#include "mylock.h"
|
||||
#include "utils.h"
|
||||
#include "request.h" /* xce_wal_batch_req_raw */
|
||||
|
|
@ -120,11 +121,21 @@ int repl_add_standby(replicator_t *r, repl_ep_t ep, const char *host, uint32_t p
|
|||
r->nstandbys++;
|
||||
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Quorum membership changed: re-evaluate pending commits. Called AFTER
|
||||
* releasing r->lock — engine lock order is ce->lock then r->lock
|
||||
* (ha-commit-engine task 5.2, Req 4.2). The idempotent early-return
|
||||
* above mutates nothing, so it deliberately skips this hook. */
|
||||
if (r->ce != NULL) {
|
||||
commit_on_membership_change(r->ce);
|
||||
}
|
||||
return idx;
|
||||
}
|
||||
|
||||
void repl_remove_standby(replicator_t *r, repl_ep_t ep)
|
||||
{
|
||||
int removed = 0;
|
||||
|
||||
if (r == NULL || ep == NULL) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -138,11 +149,18 @@ void repl_remove_standby(replicator_t *r, repl_ep_t ep)
|
|||
s->last_disconnect_time = time(NULL);
|
||||
s->ep = NULL;
|
||||
r->nstandbys--;
|
||||
removed = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Membership shrink: wake pending waiters to re-check their predicates
|
||||
* (after r->lock release — engine lock order; ha-commit-engine Req 4.2). */
|
||||
if (removed && r->ce != NULL) {
|
||||
commit_on_membership_change(r->ce);
|
||||
}
|
||||
}
|
||||
|
||||
void repl_expire_disconnect_slots(replicator_t *r, time_t now_s, uint32_t snap_interval_s)
|
||||
|
|
@ -175,6 +193,8 @@ void repl_expire_disconnect_slots(replicator_t *r, time_t now_s, uint32_t snap_i
|
|||
|
||||
void repl_on_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_lsn)
|
||||
{
|
||||
int slot_idx = -1;
|
||||
|
||||
if (r == NULL || ep == NULL) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -187,12 +207,21 @@ void repl_on_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_lsn)
|
|||
atomic_store(&s->acked_lsn, acked_lsn);
|
||||
if (s->pending_ack_count > 0)
|
||||
s->pending_ack_count--;
|
||||
slot_idx = i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Feed the commit engine AFTER the acked_lsn store and AFTER releasing
|
||||
* r->lock — commit_on_ack takes ce->lock then r->lock (majority scan),
|
||||
* so calling it under r->lock would invert the engine's lock order.
|
||||
* Never blocks (iomgr-safe; ha-commit-engine task 5.2, design §ACK Feed). */
|
||||
if (r->ce != NULL && slot_idx >= 0) {
|
||||
commit_on_ack(r->ce, slot_idx, acked_lsn);
|
||||
}
|
||||
|
||||
/* Signal any waiter (repl_wait_ack / repl_wait_majority_ack) */
|
||||
pthread_mutex_lock(&r->ack_lock);
|
||||
pthread_cond_signal(&r->ack_cond);
|
||||
|
|
@ -386,17 +415,31 @@ void repl_on_config_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_config_lsn
|
|||
int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn,
|
||||
uint32_t timeout_ms, uint8_t ack_op)
|
||||
{
|
||||
/* entry_lsn is accepted but unused by this legacy counting body
|
||||
* (ha-commit-engine task 5.1 signature change): task 5.2 delegates the
|
||||
* walAck branch to commit_submit(ce, entry_lsn, COMMIT_MODE_WAIT, ...)
|
||||
* when the engine is wired. Behavior here stays byte-identical. */
|
||||
(void)entry_lsn;
|
||||
|
||||
if (r == NULL)
|
||||
return -EINVAL;
|
||||
|
||||
int is_wal_ack = (ack_op == (uint8_t)OPCode_walAck);
|
||||
|
||||
/* 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) {
|
||||
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. */
|
||||
|
||||
struct timespec deadline;
|
||||
clock_gettime(CLOCK_REALTIME, &deadline);
|
||||
deadline.tv_sec += (long)(timeout_ms / 1000);
|
||||
|
|
@ -513,30 +556,50 @@ uint64_t repl_trim_lsn(replicator_t *r, uint64_t checkpoint_lsn,
|
|||
|
||||
void repl_start_catchup(replicator_t *r, repl_ep_t ep, uint64_t join_applied_lsn)
|
||||
{
|
||||
int changed = 0;
|
||||
|
||||
mylock_lock(&r->lock);
|
||||
for (int i = 0; i < MAX_STANDBYS; i++) {
|
||||
if (r->standbys[i].ep == ep) {
|
||||
r->standbys[i].join_applied_lsn = join_applied_lsn;
|
||||
atomic_store_explicit(&r->standbys[i].in_catch_up, 1,
|
||||
memory_order_release);
|
||||
changed = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Catch-up entry excludes the standby from the evaluation count:
|
||||
* re-evaluate pending commits (after r->lock release — engine lock
|
||||
* order; ha-commit-engine task 5.2, Req 4.2). */
|
||||
if (changed && r->ce != NULL) {
|
||||
commit_on_membership_change(r->ce);
|
||||
}
|
||||
}
|
||||
|
||||
void repl_complete_catchup(replicator_t *r, repl_ep_t ep)
|
||||
{
|
||||
int changed = 0;
|
||||
|
||||
mylock_lock(&r->lock);
|
||||
for (int i = 0; i < MAX_STANDBYS; i++) {
|
||||
if (r->standbys[i].ep == ep) {
|
||||
atomic_store_explicit(&r->standbys[i].in_catch_up, 0,
|
||||
memory_order_release);
|
||||
changed = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
mylock_unlock(&r->lock);
|
||||
|
||||
/* Standby rejoins the evaluation count: its watermark may newly
|
||||
* satisfy a pending majority — re-evaluate (after r->lock release —
|
||||
* engine lock order; ha-commit-engine task 5.2, Req 4.2). */
|
||||
if (changed && r->ce != NULL) {
|
||||
commit_on_membership_change(r->ce);
|
||||
}
|
||||
|
||||
/* Broadcast so any blocked repl_wait_ack() re-checks the ACK condition */
|
||||
pthread_mutex_lock(&r->ack_lock);
|
||||
pthread_cond_broadcast(&r->ack_cond);
|
||||
|
|
@ -563,16 +626,17 @@ int repl_needs_snapshot(replicator_t *r, uint64_t standby_lsn,
|
|||
* (ha.c) for write admission. Takes ONLY r->lock — never the commit-engine
|
||||
* mutex (lock-ordering rule: gate path holds no locks when calling in).
|
||||
* (ha-commit-engine Req 3.1, 3.2, 3.3) */
|
||||
/* repl_set_commit_engine — INERT stub (ha-commit-engine task 5.1).
|
||||
* Stores nothing, so the engine is never wired and the task-5.1 RED tests
|
||||
* fail on behavior (ACK and membership events never reach the engine),
|
||||
* not on linking. Task 5.2 adds the replicator_t.ce field, stores the
|
||||
* pointer here, and feeds commit_on_ack / commit_on_membership_change
|
||||
* from repl_on_ack and the membership mutators. */
|
||||
/* repl_set_commit_engine — wire the commit engine into the replicator
|
||||
* (ha-commit-engine task 5.2). Once set, repl_on_ack feeds commit_on_ack,
|
||||
* the membership mutators feed commit_on_membership_change, and the walAck
|
||||
* branch of repl_wait_majority_ack delegates to commit_submit. Called once
|
||||
* at cluster init before replication traffic (task 5.3); NULL un-wires. */
|
||||
void repl_set_commit_engine(replicator_t *r, struct commit_engine *ce)
|
||||
{
|
||||
(void)r;
|
||||
(void)ce;
|
||||
if (r == NULL) {
|
||||
return;
|
||||
}
|
||||
r->ce = ce;
|
||||
}
|
||||
|
||||
int repl_active_standby_count(replicator_t *r)
|
||||
|
|
|
|||
|
|
@ -44,6 +44,12 @@ typedef struct replicator_s {
|
|||
mylock_t lock; /* Protects standbys[], nstandbys */
|
||||
pthread_cond_t ack_cond; /* Signaled when any ACK arrives */
|
||||
pthread_mutex_t ack_lock; /* Guards ack_cond */
|
||||
/* Commit engine feed (ha-commit-engine task 5.2). NULL = not wired
|
||||
* (legacy counting contexts). Set once via repl_set_commit_engine at
|
||||
* cluster init, before replication traffic — read without r->lock.
|
||||
* Lock ordering: engine paths take ce->lock then r->lock, so repl.c
|
||||
* must NEVER call into the engine while holding r->lock. */
|
||||
struct commit_engine *ce;
|
||||
} replicator_t;
|
||||
|
||||
/* Lifecycle */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue