forked from lei.chen/myxce
docs(ha-commit-barrier-rollout): exhaustive xce_reply audit — zero stragglers (task 7.1)
43-site classification (8 category-A barrier-gated, 6 category-B receipt-ACK, ~29 category-C) recorded in tasks.md. Reviewer-confirmed exhaustive from the producer side and the opcode dispatch: every WAIT-class WAL producer reaches its requester reply only through a commit barrier; no handler bypasses wal_classify. Zero production changes. Noted: WORKER_UPDATE has a reply-less second producer (mark_worker_back liveness sweep) — not a violation, future RETAIN refinement candidate.
This commit is contained in:
parent
5522d6f274
commit
5896dcc3e6
2 changed files with 6 additions and 1 deletions
|
|
@ -91,7 +91,7 @@
|
|||
- _Requirements: 5.1, 5.2, 5.3_
|
||||
|
||||
- [ ] 7. Exhaustive reply-site sweep and final validation
|
||||
- [ ] 7.1 Run the exhaustive xce_reply audit over all control paths
|
||||
- [x] 7.1 Run the exhaustive xce_reply audit over all control paths
|
||||
- Grep ALL `xce_reply` call sites under src/controller/ (excluding testsuite/) — research.md lists the design-time population (job.c, queue.c, worker.c, join.c, opupd.c, cluster.c, config_repl.c, ha.c, opget.c, reply.c, task.c); re-run the grep fresh since lines drift
|
||||
- Classify every site into exactly one category: post-commit confirmation (restructured in task 3 / job submit / config success), receipt-only ACK for fact-shaped delivery (verdicts from task 3.3), or read-only/replication/error reply (no state-changing entries produced)
|
||||
- Fix any site that confirms a request-shaped state change before commit (a missed handler silently violates umbrella 2.1); commit the audit table (site → category → evidence) in the task completion notes
|
||||
|
|
@ -112,6 +112,8 @@
|
|||
- (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).
|
||||
- (7.1) AUDIT RESULT (reviewer-confirmed exhaustive, producer-side + opcode-dispatch): **ZERO request-shaped stragglers** — every WAIT-class WAL producer reaches its requester reply only through a commit barrier. 8 category-A barrier-gated sites: handle_jobsubmit success+reject (in-hook sync-majority, job.c:600/609), do_jobcontrol_op (job.c:1023), do_queuecontrol_op (queue.c:152), do_hostcontrol_op (worker.c:721), handle_worker_join (join.c:125), do_upgrade_config success (config_repl.c:1141). 6 category-B receipt-ACKs (RETAIN facts, pinned by 3.3/4.3): jobdispatch, update-resvals, update-rusage, setpreason, clrpreason, repl_config_push standby-ack. Remaining ~29 sites category C (read-only/replication/error/control-plane). All state-changing WAL flows through wal_classify at the 3 hook seams — no handler bypasses with a bare persist_wal_async/sync.
|
||||
- **Reviewer precision correction**: the audit's "worker_ctrl single-caller" claim is inaccurate — WAL_WORKER_CTRL/WORKER_UPDATE (classified WAIT) is ALSO produced by mark_worker_back (worker.c:589) via the heartbeat liveness sweep check_worker_timeout_ (cluster.c:1288), UN-barriered. NOT a 7.1 violation: that path issues NO xce_reply (controller-detected liveness fact, not a request confirmation), so it cannot confirm-before-commit; an un-barriered WAIT entry there behaves identically to RETAIN (commits later via ACKs). **Future-refinement candidate (not blocking)**: WORKER_UPDATE's WAIT classification could narrow to the request-bearing detail since the sweep path wants RETAIN semantics — carry to ha-single-leader-role or a hardening pass.
|
||||
- (6.1) For 6.2 (reviewer-confirmed): the "UPDATE clears terminal bits" guard arm (Req 5.2) is UNREACHABLE in production — mark_job_update's 3 callers (job.c:400 SIGCONT clears only PSUSP|USUSP, :410 SIGSTOP ORs PSUSP, :876 postdone ORs PDONE/EXIT) never produce terminal→non-terminal. 6.2 will most naturally share ONE backward-guard predicate across START+UPDATE; the START-on-terminal RED test exercises it, and the forward-update PIN protects the one reachable terminal UPDATE (postdone PDONE-merge) from over-rejection. on_job_* wrappers return 0 unconditionally — 6.2's caller audit must keep them returning 0 (success) on guarded-out transitions; the pins lock this. Guard placement: top of each mark_job_*, under the caller-held WRITE lock, before BEFORE_EVENT. Live anchors drifted: mark_job_start:592, finish:647, fail:705 (PERR-merge :711-721), update:735.
|
||||
- (5.2) **REJECTED then remediated (r1)** — two CRITICAL findings the first pass missed, both fixed: (F1) the config-ACK signal was never produced in production (repl_on_config_ack had zero callers; the standby's replconfigpushreplymsg reply was dropped on the primary) so the new authoritative barrier would have BRICKED every multi-member config upgrade — this was the unlanded half of ha-config-bootstrap task 8.1. Fixed by a replconfigpushreplymsg decode branch in repl_ack_decode_and_dispatch (ha.c) → repl_on_config_ack on the repl_add_standby-registered promoted-vote ep (production-normal path), + a defensive cluster.c OPCode_reply ingestion for inbound standbys (no-ops against unregistered slots, forward-looking). Schema unchanged (ReplConfigPushReply already carried versionTag). (F2) the 500ms barrier wait reached the iomgr epoll thread via 5 ha.c push sites. Fixed by exporting push_config_transport_only (non-blocking fan-out) for all best-effort ha.c/join.c streaming sites; push_config_to_standbys (transport+barrier) now has exactly ONE caller — do_upgrade_config_with_bytes in vtask context. LESSON: the design's "handlers run on vtask threads" claim was only true for cluster.c-dispatched opcodes, not the ha.c peer-channel push sites the design never analyzed (ha.c was outside its modified-files list). Wire-path test drives real capnp ACK bytes through the production decode branch.
|
||||
- (3.3) **bug-451 DISCOVERED (reviewer-verified byte-precise, NOT fixed — boundary)**: update_job_rusages fires HOOK_DETAIL_JOB_RUSAGE_UPDATE with (cluster_t*, xce_job_rusages_t*) (job.c:1014-1015) but persist_job_hook_fun's default arm (persist.c:1355-1359) reads vararg 2 as job_t* and dereferences j->job.state at offset 56 — one past the end of the exactly-56-byte xce_job_rusages_t → 4-byte OOB read + garbage state field in every WAL_JOB_RUSAGE payload (jobid lines up by accident). Production fix candidate for task 7.1's sweep or a follow-up: persist arm needs a RUSAGE-specific va_list signature. Census now 488.
|
||||
|
|
|
|||
|
|
@ -1024,3 +1024,6 @@
|
|||
| 21:24 | Edited src/controller/job.c | added 2 condition(s) | ~294 |
|
||||
| 04:37 | task 6.2: mark_job_* transition guards (dup no-op / backward reject / forward merge preserved) | src/controller/job.c | 22 ctrl tests 0 fail, 5 guard tests GREEN | ~14k |
|
||||
| 21:54 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~11 |
|
||||
| 04:58 | task 7.1 exhaustive xce_reply audit: 43 emit sites classified A/B/C; WAIT-class producers (queue_ctrl/worker_ctrl single-caller, job submit, worker_join, upgrade config) all barrier-gated; NO straggler | src/controller/*.c (audit only) | zero violations | ~30k |
|
||||
| 22:05 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~18 |
|
||||
| 22:05 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified RESULT() | ~441 |
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue