From 312bd6be34006bca54f4f1c9d6088d6bd0ce25cc Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 17 Jun 2026 04:48:01 +0000 Subject: [PATCH] chore(wolf): record libxce EINVAL-guard gotcha + sandbox live-cluster limits --- .wolf/cerebrum.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 6015c97..c376de8 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -75,6 +75,8 @@ - [2026-06-09] Do NOT make the duplicate-vote deny candidate-agnostic ("term-only deny"). Raft §5.2 grants iff votedFor ∈ {Nil, candidateId} — same-candidate re-grant must stay idempotent (network retransmission path). An uncommitted ha.c change did this and broke test_vote_granted_to_same_candidate_same_term; ha.c must match raft.c's Step 4 semantics (deny only a DIFFERENT candidate in the same term). See bug-338. - [2026-06-09] The "self" voted_for_name placeholder only exists in test mode; production raft_cb_persist records the node's REAL name for a self-vote. Any check for "did I vote for myself" must compare against BOTH "self" and c->myconf->name (see myapp.c leader_known). - [2026-06-15] SOLVED (bug-563): "jobsub ok but joblist/workerlist empty on the HA primary; reslist works". ROOT CAUSE: `cluster_start()` is the ONLY setter of `c->running = 1` and it is ORPHANED — zero callers (grep `cluster_start(`). The HA `myapp_run` path starts iomgr/vtaskmgr/delaymgr/persist itself but never calls `cluster_start`, so `c->running` stays 0 on the primary. `check_controller()` (the streaming gate `xce_reply_get` uses for joblist/workerlist) returns `-EINVAL` when `!c->running`; `xce_reply_get` then sets the chunk count `nc = 0`, shipping an EMPTY list even though `collect_jobs()` found the rows. `reslist` passes `checkf=NULL` so it bypasses the gate — that asymmetry is the tell. Single-node "works" because the legacy `cluster_init` path called `cluster_start`. FIX: `c.running = 1;` in `myapp_run()` after `persist_start()` (src/controller/myapp.c). RULE: when only SOME list ops are empty but the data is present server-side, check the `xce_reply_get` `check_*` predicate — a `<0` return silently zeroes the chunk (`nc=0`), not an error. To prove it: log `nc`/`checkstatus` in the `xce_reply_get` send loop. Reproduce HA bugs locally with `/xce-build` + a 3-node deploy-ha under ASAN rather than theorizing. (My earlier UAF/stale-binary guess here was WRONG — disproven by "crash gone, binary current".) Still-latent: `cluster_start` also armed `check_workers_timeout` (delaymgr) — that side-effect remains unrestored on the HA path. +- [2026-06-16] When adding a NEW opcode to the HA protocol, it MUST be wired into `ha_dispatch_gate()` (src/controller/ha.c) — the role-based admission gate runs in `myreq_handler` (cluster.c:251) BEFORE the per-opcode handler, and an unlisted opcode is rejected (`-XCE_ERR_STANDBY`/`-XCE_ERR_NOT_READY`) so the handler never runs. bug-605: ha-prevote added `OPCode_preVoteRequest` (opcode 38) + handler (cluster.c:440) but not the gate; STANDBY voters silently dropped every incoming PreVote, the candidate counted the failed exchange as a denial, and a cold cluster looped PreVote forever with NO primary. INVARIANT: a PreVote is strictly non-mutating, so it must be admitted in EXACTLY the roles a real `requestVote` is (CTRL_INIT/CTRL_ELECTING/CTRL_STANDBY). Diagnostic tell: voters log zero "PreVote DENIED" yet the candidate keeps "losing" — means the request never reached the handler (gate drop), not a policy denial. Confirm with gdb breakpoint on `ha_dispatch_gate` rc. +- [2026-06-16] HA election timeouts must be RANDOMIZED/STAGGERED on the FAILOVER path, not just at startup. bug-606: when the primary dies, the surviving standbys lose it at the same instant and (before the fix) campaigned in perfect lock-step — both win PreVote (neither sees a primary), both vote self at the same term, split the vote (quorum unreachable with 2 survivors), time out on a FIXED watchdog (raft.c:105) and retry on the FIXED heartbeat_timeout liveness cadence → term climbs unbounded, no leader. The jitter machinery (election_priority_step_ms=500, election_jitter_max_ms=200, xorshift64 on jitter_seed) existed but was wired ONLY into the startup heartbeat-wait (ha_wait_for_heartbeat_or_timeout / myapp.c stagger); the PreVote failover path (liveness_poll_fn→ha_on_primary_ctrl_fd_rdhup→ha_begin_failover→ha_start_prevote_async) bypassed it, and raft's own staggered-start (raft_on_leader_lost→on_start_election_after_delay, raft.c:421) is also bypassed by the PreVote path. FIX: ha_election_stagger_ms() + a delaymgr-scheduled, guarded PreVote start in ha_begin_failover (skip if a primary appears during the stagger). RULE: any new re-election/failover trigger must apply per-node stagger (priority*step + fresh random jitter, re-randomized each attempt) AND a "primary alive?" guard before campaigning. Diagnostic tell: identical ms-aligned election timestamps across nodes + term climbing by 1 each cycle = lock-step split vote, not a vote-policy bug. step MUST exceed jitter_max so adjacent priorities never overlap. - [2026-06-14] A partially-initialized `xce_reply_t` is a UAF hazard. The Cap'n-C generated `encode_RequestReply` (controller.capnp.c) reads the COMMON TRAILER fields (`redirectHost`, `redirectPort`) on EVERY reply regardless of `data_which` — `redirect_host` is `strlen`'d whenever `!= NULL`. Handlers declare `xce_reply_t reply;` on the stack and only set status/data, so those trailer fields were stack garbage; a garbage `redirect_host` aliasing a freed capnp segment = heap-use-after-free (seen on the OPCode_nop/state-report path, cluster.c). FIX (bug-562): `init_reply_by_req()` now `memset`s the whole struct first. RULE: any xce_reply_t built WITHOUT init_reply_by_req (e.g. ha_send_wal_ack_reply) MUST memset it. Generated union encoders are not safe to feed a partially-init struct — common/trailer fields are always serialized. - [2026-06-04] capnpc-c 1.0.1 has a bug with `:group` inside a `union` when `$C.codecgen` is active: the encode/decode generator emits `d->FIELDNAME` (flat) instead of `d->data.GROUPNAME.FIELDNAME`, AND places the case as dead code after the prior `break`. Do NOT use inline `:group` in a union that has `$C.codecgen`. Instead, define a named outer struct (`struct Foo $C.mapname("foo_t")`) and reference it as a pointer union member. The named struct generates correct `encode_Foo_ptr`/`decode_Foo_ptr` calls. The `read_RequestReply`/`write_RequestReply` wire layer works correctly with groups but the codec layer does not. @@ -118,3 +120,13 @@ - persist_commit_barrier watermark = current_lsn - 1 (matching persist.c:1021 persist_wal_sync_append read); covered ⇔ current_lsn == 0 OR committed_lsn >= current_lsn - 1. Design §Commit Barrier Helper "lsn = p->current_lsn" is shorthand — the parenthetical persist.c:1019-1021 reference pins it to current_lsn - 1 (engine clamps committed to highest appended, so waiting on current_lsn itself would never complete). (2026-06-12, task 2.1) - (2026-06-13, single-leader-role 4.2) ha_dispatch_gate's OUTER `default: return 0` ADMITS — so after deleting the `case CTRL_PRIMARY_COMMITTING:` block, a role manually set to COMMITTING falls through to admit (not NOT_READY). That is exactly why test_ha_startup.c:test_dispatch_gate_committing_denies_job_ops flips from PASS→FAIL (Expected -10013 Was 0) — category-(a), 5.1-owned. The 6 PRIMARY||COMMITTING compound sites in ha.c: raft_cb_vote_eligible(~215), handle_request_vote term-stepdown(~1984)+role-deny(~2004), peer_ctrl_msg_handler case label(~2820), vote_ep_err_handler(~3088), primary_heartbeat_fn re-arm(~3883). Collapsing all to PRIMARY-only makes a manually-COMMITTING test role a NON-leader. Census after 4.2: exactly 5 FAIL, ALL COMMITTING-state category-(a) (test_ha_startup ×2, test_ha_rdhup_integration, test_ha_repl_push, test_ha_connection_reuse), zero category-(b). enum value CTRL_PRIMARY_COMMITTING=4 stays in ha.h (5.2 deletes it); no ha.c CODE branches on it post-4.2 (only comments in 2.2/3.1-owned functions remain). Unity attributes vote-gate cases to test_ha_failover.c by group UNITY_BEGIN — locate by NAME. FAIL bytes: `name:\x1b[41mFAIL\x1b[0m` (color BETWEEN ':' and FAIL) — grep raw `FAIL`, count was exactly 5. + +### libxce async failover (2026-06-16) +- `xce_request(ep,req)` is fire-and-forget enqueue; reply status is only available at the `cbuf_get` round trip in xceapi.c. `xce_request_with_retry()` production branch is a no-op stub (retries only under the test hook). Any client-side retry/failover MUST wrap the send+wait round trip at the `api_ctx_t` level, not `xce_request`. Controller redirect (`E_XCE_REDIRECT`, join.c) sends status-only; redirect_host/port schema fields exist (controller.capnp:568) but are never populated. See spec libxce-failover, bug-608. +- xceapi.c per-op guard rejects ctx->state != XCE_CTX_RUN with -EINVAL; the transport-error path (xcectx.c:148) drives ctx to XCE_CTX_ERR -> this is the exact source of the failover "Invalid argument" symptom. Any failover must restore XCE_CTX_RUN after reconnect. +- ctx->wbuf is a single-slot status-only channel (cbuf_init ...,1,sizeof(int), xcectx.c:185): no seq correlation, so client is single-in-flight per ctx. Retries must drain/seq-match to avoid consuming a stale reply. +- Job submit uses a CLIENT-generated jobid (job.c:56) + server EEXIST/E_XCE_DUP_JOB replay guard (job.c:95) -> writes are idempotent on re-send; DUP on a retried submit means success. +- libxce layering: xcecli_* (client.c) is the UNIVERSAL client surface — worker (controller_connector.c), scheduler (scheduler.c), apiserver cgo (clib.go), xce_test all call xcecli_*, never xceapi xce_* directly. Only api_ctx_t (client.c) holds the master list; xce_ctx_t (xceapi/xcectx.h) does not. => client-side cross-cutting concerns (failover/retry) belong in client.c wrapping xce_*, keeping xce_* one-shot/fail-fast and xceapi unchanged. + +### Decision: full-state rejoin on controller failover (2026-06-16) +- On failover, stateful daemons re-join the new primary and apiserver/scheduler receive FULL state (full job list), not a delta. Accepted because failover is rare, some handshake is unavoidable (new primary has no live connection to the daemon until rejoin), and full-state-on-rejoin is simpler/correct and still better than IBM LSF (full-history replace, no snapshot). Lightweight re-attach / delta transfer deferred (YAGNI) — roadmap backlog. Revisit only if measured as a scale problem.