forked from lei.chen/myxce
docs(job-array-single-wal-record): record fresh /kiro-validate-impl GO, close stale notes
spec.json: phase -> implementation-complete, new GO decision (2026-07-17) covering the full feature plus phase 7's residual-gap closure; prior decision preserved as prior_decision; bug-1443 known_limitations entry retired (closed by task 7.1) and replaced with the one limitation that is still genuinely open (task 7.2's first-boot promotion-ordering gap). tasks.md: task 3.1's implementation note updated from "open, unowned gap" to "closed by task 7.1" -- it predated that fix and had gone stale. Validation evidence: full C suite 9/9 green (incl. libxce test post rtti-fix), Go suite green (88.1% backend coverage), clean TBD/secrets grep, live array-soak smoke PASS (O(1) replay time confirmed at 2-element vs 10,000-element scale), cross-task integration verified at every seam phase 7 touched, 10/10 requirement groups covered, zero design drift, zero blocked tasks.
This commit is contained in:
parent
ccee18b0c9
commit
9cf61c9fc1
2 changed files with 12 additions and 7 deletions
|
|
@ -1,9 +1,9 @@
|
|||
{
|
||||
"feature_name": "job-array-single-wal-record",
|
||||
"created_at": "2026-07-09T17:20:00Z",
|
||||
"updated_at": "2026-07-16T06:15:00Z",
|
||||
"updated_at": "2026-07-17T09:47:00Z",
|
||||
"language": "en",
|
||||
"phase": "tasks-generated",
|
||||
"phase": "implementation-complete",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
|
|
@ -22,11 +22,16 @@
|
|||
"blocked_on": [],
|
||||
"validate_impl": {
|
||||
"decision": "GO",
|
||||
"validated_at": "2026-07-16T02:10:00Z",
|
||||
"validated_at": "2026-07-17T09:47:00Z",
|
||||
"known_limitations": [
|
||||
"bug-1443: restore_job_array()'s bulk-insert-phase failure can permanently orphan array elements (violates Req 2.3/9.4 literally); tracked in .wolf/buglog.json, not fixed in this spec"
|
||||
"First-boot promotion-ordering gap (documented in tasks.md task 7.2's Implementation Notes): a node's very first promotion evaluation in a fresh process runs before that node's own first persist_recover()/replay_wal() call, so the Live Apply Resource Fence is invisible to that one specific check. Every promotion evaluation after a process's first one is protected. Out of this spec's persist.c-only boundary; flagged for a dedicated ha.c follow-up, not tracked as a numbered bug."
|
||||
],
|
||||
"reopened_at": "2026-07-16T06:15:00Z",
|
||||
"reopen_reason": "User directed all of this feature's own open residual gaps be fixed (2026-07-16): bug-1443 (bulk-insert partial-visibility), task 4.2's boot-replay fence gap, and task 5.3b's bounded leak. Added as Phase 7 (tasks 7.1-7.3) in tasks.md. Unrelated pre-existing bugs 1437/1438 explicitly excluded from this reopening per user's scope decision."
|
||||
"prior_decision": {
|
||||
"decision": "GO",
|
||||
"validated_at": "2026-07-16T02:10:00Z",
|
||||
"reopened_at": "2026-07-16T06:15:00Z",
|
||||
"reopen_reason": "User directed all of this feature's own open residual gaps be fixed (2026-07-16): bug-1443 (bulk-insert partial-visibility), task 4.2's boot-replay fence gap, and task 5.3b's bounded leak. Added as Phase 7 (tasks 7.1-7.3) in tasks.md. Unrelated pre-existing bugs 1437/1438 explicitly excluded from this reopening per user's scope decision.",
|
||||
"note": "bug-1443 was closed by task 7.1 (commit dcf9f39); superseded by this validation's fresh GO."
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -222,7 +222,7 @@
|
|||
- **Task 1.3**: `job_obj_init()`/`job_obj_fini()` are now idempotent (atomic_flag-guarded) — safe to call more than once without an intervening `fini()`. Any future test construction can rely on this instead of coordinating a single call site.
|
||||
- **Task 1.3**: `makeJobsub()`/`makeJobstartinfo()` now return `-ENODEV` (not a generic `-EINVAL`) when the interning table isn't initialized — if a later task's error handling branches on `job_init()`'s return value, `-ENODEV` is a distinct, checkable signal for "table not ready" vs. other construction failures.
|
||||
- **Task 3.3**: `apply_wal_entry()` (persist.c:2574) is dead code in production — confirmed via exhaustive grep (zero call sites outside its own declaration/definition/comments) and via `ha.c:8024`'s `ha_standby_advance_committed_and_apply` (dated 2026-07-10), whose own doc comment explicitly says standby entity materialization now goes through the SAME `apply_committed_entry()`/`persist_apply_pump_kick` pump the primary uses, not `apply_wal_entry()`. The persist.c:3722 comment claiming otherwise predates that refactor and was never updated (design.md corrected accordingly). Requirement 3.1/3.2/4.1 are fully delivered by task 3.2 alone. Task 3.3's code was kept (not discarded) as defensive parity with `WAL_JOB_SUBMIT`'s existing, also-dead case in the same switch — it is tested and correct, just not currently reachable in production. If `apply_wal_entry()` is ever revived or removed entirely in a future spec, this task's case should move or be deleted with it.
|
||||
- **Task 3.1**: `restore_job_array()`'s atomicity has one narrow, documented exception (design.md updated to match): the final bulk-insert-into-`c->jobmgr` pass can itself fail on allocation/growth exhaustion (`c->jobmgr` grows via `realloc()`; no capacity-reservation primitive exists anywhere in `kv.c`/`kvmgr.c`/`zarray.c` to pre-check this before mutating). On this specific failure, the function never silently claims success — it returns a negative errno — but an already-inserted prefix `[0, inserted)` can be left genuinely, permanently visible in `c->jobmgr` (no `key_del` exists to undo it), and that prefix is NEVER included in `out_new_jobs`, so `HOOK_DETAIL_JOB_NEW` never fires for it, not on this attempt or any later successful retry of the same LSN. **Tasks 3.2/4.x must account for this**: those elements are individually addressable (status/cancel/completion work normally) but permanently un-pushed to the scheduler/API server unless something later re-derives and re-notifies them — this is a real gap in the current design, not yet solved, and task 4.x's Live Apply Resource Fence design should explicitly decide whether/how to handle it (e.g. detecting this residual state and re-notifying on self-heal) rather than only handling the construction-phase failure case.
|
||||
- **Task 3.1 (superseded by task 7.1, see that entry below)**: `restore_job_array()`'s atomicity originally had one narrow, documented exception: the final bulk-insert-into-`c->jobmgr` pass could itself fail on allocation/growth exhaustion (`c->jobmgr` grows via `realloc()`; no capacity-reservation primitive existed anywhere in `kv.c`/`kvmgr.c`/`zarray.c` to pre-check this before mutating), and an already-inserted prefix `[0, inserted)` could be left permanently visible in `c->jobmgr` but never included in `out_new_jobs`, so `HOOK_DETAIL_JOB_NEW` never fired for it — tracked as bug-1443. **This gap is now closed** (task 7.1, commit dcf9f39, 2026-07-16): `kvmgr_reserve()`/`xzarray_reserve()` pre-grow `c->jobmgr` before the bulk-insert pass starts, structurally closing the dominant N-scaling trigger; for the narrower residual case that remains theoretically possible (a tiny per-entry hash-chain-node allocation or a content collision, non-N-scaling), `restore_job_array()` now populates `out_new_jobs` with the genuinely-inserted prefix even on failure, and `apply_committed_entry()` fires `HOOK_DETAIL_JOB_NEW` for that prefix while still propagating the negative rc so the LSN halts and the Live Apply Resource Fence engages for the remainder. See task 7.1 below for full detail.
|
||||
**UPDATE (post-4.4): this gap was NOT closed by Phase 4.** Tasks 4.1-4.4 correctly deliver the generic negative-rc halt/fence/gate/self-heal machinery — a bulk-insert-phase failure DOES still engage the fence and block promotion, same as any other failure — but none of them add the "no partial/divergent set" guarantee specifically for this seam, because that guarantee is structurally false on this path with current APIs (no `key_del`). Task 4.4's own capstone test deliberately used the (fully atomic) construction-phase seam instead, precisely because asserting "zero partial elements" against the bulk-insert seam would be dishonest. **This residual gap remains open and unowned by any task in this spec's current scope** — flag at `/kiro-validate-impl` time; closing it would require either a `key_del`/rollback primitive in `kv.c`/`kvmgr.c`, or a re-derive-and-re-notify recovery path for the orphaned prefix, either of which is design-level work beyond this spec's approved task list.
|
||||
- **Task 4.1**: `persist_apply_pump_run()` now halts the walk (no `last_applied_lsn` advance) on ANY negative return from `apply_committed_entry()`, not just the new array type — this is a general fix. Investigated whether this newly exposes a hazard for the pre-existing job-state fact entries (`WAL_JOB_START/FINISH/FAIL/UPDATE`), which have their own real `-ENOENT` branch when the target jobid isn't in `c->jobmgr`: confirmed NOT a hazard, because `c->jobmgr` (`xkvmgr_type`) has no `key_del` (jobs are never removed for the process lifetime, corroborated by `job.c:428,455,645`), and this task's own strict sequential-no-skip apply means a fact's LSN is only reached after its causally-prior submit already applied. So a live `-ENOENT` on a fact can now only mean genuine corruption/divergence — halting is correct, not a new footgun. Independent reviewer confirmed this via `test_term_safety_adapter.c:2039-2043`'s synthetic (non-production-representative) trigger. design.md's "Existing Architecture Analysis" table row overclaims ("every existing WAL type... returns 0 or positive in practice") — technically inaccurate re: the fact-ENOENT branch — left as-is since the persist.c code comment at the site already documents the exception correctly; low-severity, non-blocking.
|
||||
- **Task 4.2**: New `_Atomic uint64_t apply_resource_fenced_lsn` field on `persist_t` (0 = clear, nonzero = the stuck LSN) plus `persist_apply_resource_fenced()`/`persist_mark_apply_resource_fenced()`/`persist_clear_apply_resource_fence()`, wired into `persist_apply_pump_run()`'s existing halt branch (engage) and unconditionally after every successful advance (clear). The unconditional clear is provably equivalent to "clear only on the fenced LSN's own successful retry" because `last_applied_lsn` has exactly 3 writers codebase-wide, the other two (`replay_wal`, `persist_recover`) are boot-only and temporally disjoint from the live pump, and a failed walk never advances past the stuck LSN — so the next success by construction IS that LSN's retry. **Known residual gap (out of this task's boundary, flagged by review, NOT yet fixed)**: `replay_wal()` (boot/restart replay, persist.c:3875-3888) still discards `apply_committed_entry()`'s failure return the same way the live pump did before task 4.1, and `persist_init()` unconditionally zeroes the fence field on every restart. A node that crashes while fenced and re-hits the identical resource-exhaustion condition during boot replay will silently pass it and look fully promotable while actually divergent — task 4.1/4.2's protection does not yet extend to the boot-replay path. This is a pre-existing gap (not introduced by this feature) but is now inconsistent with the live path's new guarantee; worth a dedicated follow-up task before anything downstream places full trust in this fence across a restart.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue