lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(ha-commit-barrier-rollout): commit-first-then-reply in all four handlers (task 3.2)

Browse source 
Uniform pattern (ctrl fn -> persist_commit_barrier -> reply) in
do_jobcontrol_op/do_queuecontrol_op/do_hostcontrol_op; join barrier
between AFTER_EVENT and success reply (failure -> error reply, skip
on_worker_join). FIXES bug-447: key_find found-rc normalized inside
job_ctrl/queue_ctrl/worker_ctrl — queue/host control now actually
append WAL and replicate (was silently unpersisted); idempotent no-ops
now return 0. handle_jobsubmit untouched. 12/12 + 15/15; census zero
failures.
This commit is contained in:
Rongsong Shen 2026-06-12 10:28:25 +00:00
parent e1c3c173f4
commit facdb90917
7 changed files with 439 additions and 309 deletions
Download patch file Download diff file Expand all files Collapse all files

3
.kiro/specs/ha-commit-barrier-rollout/tasks.md
View file

@ -37,7 +37,7 @@
- Observable completion: new cases FAIL (red) against the current reply-before-commit handlers (anchors: job.c:932, queue.c:130, worker.c:700, join.c:107-109 — re-verify with grep)
- _Requirements: 1.1, 1.2, 1.3, 1.4_
- [ ] 3.2 Restructure the four handlers to green
- [x] 3.2 Restructure the four handlers to green (incl. bug-447 fix: queue/host control now actually persist+replicate)
- Apply the uniform pattern: `rc = <ctrl fn>; if (rc == 0) rc = persist_commit_barrier(&c->persist, 0); reply.status = rc; xce_reply(...)` — in `do_jobcontrol_op` (job.c:921-934), `do_queuecontrol_op` (queue.c:119-132), `do_hostcontrol_op` (worker.c:689-702); in `handle_worker_join` (join.c:33-119) insert the barrier between the AFTER_EVENT hook and the success reply, keep error replies immediate, keep `on_worker_join` after the reply
- Barrier runs after entity locks are released in every handler (the ctrl fns unlock before returning)
- Observable completion: all task 3.1 tests PASS; `handle_jobsubmit` untouched (reference implementation)
@ -112,6 +112,7 @@
- (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).
- (3.2) bug-447 FIXED via rc normalization inside the three ctrl fns (real errors/misses preserved; reviewer-verified per-fn). Semantics change shipped intentionally: idempotent no-op control actions now return 0 (was leaked 1); queue/host mutations now append WAL + replicate. Join barrier failure → error reply + on_worker_join SKIPPED (coherent reading of design; worker retries via idempotent rejoin; documented at join.c:108-113). PRE-EXISTING ref leak: job_ctrl takes reference(j) (job.c:317) but normal exit never dereferences (bug-448) — out of scope, future fix.
- (3.1) **bug-447 CONFIRMED (reviewer-verified to librtti disassembly)**: key_find returns 1 on found (unnormalized from xkvmgr_find/kvtab_find); queue_ctrl (queue.c:61, gate :106) and worker_ctrl (worker.c:644, gate :678) leak rc=1 → AFTER_EVENT never fires on the found path → **queue/host control silently unpersisted+unreplicated in production today**; job_ctrl appends via mark_job_* hooks but still replies status 1. Fix point = 3.2's handler rc normalization (key_find is in binary librtti, out of repo). RED tests pin it.
- (3.1) Test link gained job.c/queue.c/join.c (test-only meson change) + 8 inert link stubs in test_commit_barrier_reply.c (none on tested paths — reviewer-traced). Reply capture = zeroed stack endpoint ep->outq inspection (test_config_repl.c:2210 precedent; endpoint_update_epoll no-ops when iomgr NULL). RED-window orphaned fsync threads self-resolve at GREEN (reviewer finding 5).
- (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).

646
.wolf/buglog.json
View file

File diff suppressed because it is too large Load diff

12
.wolf/memory.md
View file

@ -925,3 +925,15 @@
| 09:35 | Task 3.1 ha-commit-barrier-rollout: created RED integration tests for commit-first-then-reply (4 opcodes x withheld/acked/validation) | src/controller/testsuite/test_commit_barrier_reply.c, meson.build, controller_test.c | 484 results, 8 RED fails (new file only), 472 baseline green; discovered key_find rc==1 leak in queue_ctrl/worker_ctrl/job_ctrl (bug-447) | ~90k |
| 03:00 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~21 |
| 03:00 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | 1→3 lines | ~255 |
| 03:05 | Edited src/controller/queue.c | 2→3 lines | ~16 |
| 03:05 | Edited src/controller/queue.c | modified if() | ~150 |
| 03:05 | Edited src/controller/queue.c | added 2 condition(s) | ~210 |
| 03:05 | Edited src/controller/worker.c | modified if() | ~157 |
| 03:06 | Edited src/controller/worker.c | added 2 condition(s) | ~210 |
| 03:06 | Edited src/controller/job.c | modified if() | ~175 |
| 03:06 | Edited src/controller/job.c | added 2 condition(s) | ~208 |
| 03:06 | Edited src/controller/join.c | 3→4 lines | ~22 |
| 03:07 | Edited src/controller/join.c | added 2 condition(s) | ~243 |
| 11:00 | Task 3.2 ha-commit-barrier-rollout: commit barrier in 4 handlers + bug-447 rc normalization in queue_ctrl/worker_ctrl/job_ctrl | src/controller/{job,queue,worker,join}.c | 12/12 barrier-reply tests PASS, full suite 0 FAIL, no sanitizer reports | ~60k |
| 03:28 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~32 |
| 03:28 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | 1→2 lines | ~172 |

24
src/controller/job.c
View file

@ -327,6 +327,15 @@ int job_ctrl(cluster_t* c, xce_jobctrl_t* ctrinfo)
return -ENOENT;
}
/* bug-447: key_find returns 1 on found (binary librtti, unnormalizable
* upstream). Normalize the found-rc to 0 so the not-started siginfo
* branch below (which appends via the mark_job_* hooks but never
* reassigns rc) returns 0 to the handler's commit-barrier pattern
* instead of leaking 1 as the reply status. Real errors are
* untouched: rc < 0 returned above, the j == NULL miss returned
* -ENOENT, and every other branch sets rc itself. */
rc = 0;
/* FIXME: eliminate held multiple locks */
lock(j, RTTI_LOCK_READ);
if (j->job.allocs != NULL) {
@ -925,7 +934,20 @@ int do_jobcontrol_op(cluster_t* c, endpoint_t* ep, xce_request_t* request)
init_reply_by_req(&reply, request);
rc = job_ctrl(c, request->body.jobctrlmsg);
rc = job_ctrl(c, request->body.jobctrlmsg);
/* Commit barrier (ha-commit-barrier-rollout Req 1.1-1.4): the success
* reply is sent only after the operation's WAL entry is majority-
* committed; barrier failures (-ETIMEDOUT, ...) become the error
* reply. Errors from job_ctrl reply immediately — no barrier wait.
* job_ctrl released all entity locks before returning, so blocking
* here holds no lock. timeout 0 = configured sync_timeout_ms. */
if (rc == 0) {
rc = persist_commit_barrier(&c->persist, 0);
if (rc != 0)
WARN("%s commit barrier failed, rc %d", __func__, rc);
}
reply.status = rc;
reply.data_which = RequestReply_data_nodata;

16
src/controller/join.c
View file

@ -7,6 +7,7 @@
#include "cluster.h"
#include "config_repl.h"
#include "ha.h"
#include "persist.h"
#include "xce/cluster_def.h"
#include "xce/errnox.h"
@ -104,6 +105,21 @@ static int handle_worker_join(cluster_t* c, endpoint_t* ep,
xce_run_hooks(&(c->hooksys), XCE_CONTROLLER_HOOK_WORKER, HOOK_AFTER_EVENT,
HOOK_DETAIL_WORKER_JOIN, c, from->name, w);
/* Commit barrier (ha-commit-barrier-rollout Req 1.1-1.4): the success
* reply is sent only after the join's WAL entry is majority-committed;
* barrier failures (-ETIMEDOUT, ...) become the error reply and skip
* on_worker_join below. Join errors (idx < 0) reply immediately — no
* barrier wait. No lock is held here (AFTER_EVENT runs outside any
* lock). timeout 0 = configured sync_timeout_ms. */
if (idx >= 0) {
rc = persist_commit_barrier(&c->persist, 0);
if (rc != 0) {
WARN("%s commit barrier failed for %s, rc %d", __func__,
from->name, rc);
idx = rc;
}
}
reply.status = idx;
reply.data_which = RequestReply_data_nodata;
xce_reply(ep, c->myhost, &reply);

24
src/controller/queue.c
View file

@ -3,6 +3,7 @@
#include <string.h>
#include "cluster.h"
#include "persist.h"
#include "qobj.h"
#include "xce/cluster_def.h"
#include "xce/types.h"
@ -73,6 +74,14 @@ int queue_ctrl(cluster_t* c, xce_queuectrl_t* ctrl)
return -ENOENT;
}
/* bug-447: key_find returns 1 on found (binary librtti, unnormalizable
* upstream). Normalize the found-rc to 0 here so the AFTER_EVENT hook
* gate below fires (WAL append + replication) and the handler's
* commit-barrier pattern sees 0. Real errors are untouched: rc < 0
* returned above, the q == NULL miss returned -ENOENT, and control-
* action failures set rc themselves. */
rc = 0;
lock(q, RTTI_LOCK_WRITE);
switch (ctrl->action) {
case QUEUE_OPEN:
@ -123,7 +132,20 @@ int do_queuecontrol_op(cluster_t* c, endpoint_t* ep, xce_request_t* request)
init_reply_by_req(&reply, request);
rc = queue_ctrl(c, request->body.queuectrlmsg);
rc = queue_ctrl(c, request->body.queuectrlmsg);
/* Commit barrier (ha-commit-barrier-rollout Req 1.1-1.4): the success
* reply is sent only after the operation's WAL entry is majority-
* committed; barrier failures (-ETIMEDOUT, ...) become the error
* reply. Errors from queue_ctrl reply immediately — no barrier wait.
* queue_ctrl released all entity locks before returning, so blocking
* here holds no lock. timeout 0 = configured sync_timeout_ms. */
if (rc == 0) {
rc = persist_commit_barrier(&c->persist, 0);
if (rc != 0)
WARN("%s commit barrier failed, rc %d", __func__, rc);
}
reply.status = rc;
reply.data_which = RequestReply_data_nodata;

23
src/controller/worker.c
View file

@ -657,6 +657,14 @@ int worker_ctrl(cluster_t* c, xce_hostctrl_t* ctrl)
return -ENOENT;
}
/* bug-447: key_find returns 1 on found (binary librtti, unnormalizable
* upstream). Normalize the found-rc to 0 here so the AFTER_EVENT hook
* gate below fires (WAL append + replication) and the handler's
* commit-barrier pattern sees 0. Real errors are untouched: rc < 0
* returned above, the w == NULL miss returned -ENOENT, and control-
* action failures set rc themselves. */
rc = 0;
lock(w, RTTI_LOCK_WRITE);
switch (ctrl->action) {
case HOST_OPEN:
@ -693,7 +701,20 @@ int do_hostcontrol_op(cluster_t* c, endpoint_t* ep, xce_request_t* request)
init_reply_by_req(&reply, request);
rc = worker_ctrl(c, request->body.hostctrlmsg);
rc = worker_ctrl(c, request->body.hostctrlmsg);
/* Commit barrier (ha-commit-barrier-rollout Req 1.1-1.4): the success
* reply is sent only after the operation's WAL entry is majority-
* committed; barrier failures (-ETIMEDOUT, ...) become the error
* reply. Errors from worker_ctrl reply immediately — no barrier wait.
* worker_ctrl released all entity locks before returning, so blocking
* here holds no lock. timeout 0 = configured sync_timeout_ms. */
if (rc == 0) {
rc = persist_commit_barrier(&c->persist, 0);
if (rc != 0)
WARN("%s commit barrier failed, rc %d", __func__, rc);
}
reply.status = rc;
reply.data_which = RequestReply_data_nodata;