test(worker-rejoin-resync): task 7.2 - failover-during-job recovery
Add test_failover_during_job_recovers_lost_finish, a focused proof of the feature's own original motivating scenario (bug-1741): a job dispatched and RUN with no finish fact ever committed anywhere (modeling a lost fire-and-forget finish notify), recovered to DONE with the worker-reported finish time - not stuck in RUN - via the real rejoin/query/report/apply chain. Test-only, no production changes; reuses task 7.1's helper infrastructure rather than duplicating its broader four-outcome coverage. Updates the existing bug-924 (/tmp exhaustion / flaky teardown) entry with a 5th occurrence hit during validation, unrelated to this task's own change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
8650cfd9ba
commit
bceca0c990
3 changed files with 438 additions and 5 deletions
|
|
@ -135,7 +135,7 @@
|
|||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 3.1, 3.2, 3.3, 3.6, 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 4.7, 4.8, 7.1, 7.2, 7.3_
|
||||
- _Depends: 6.1, 6.2, 4.3, 5_
|
||||
|
||||
- [ ] 7.2 Failover-during-job recovery
|
||||
- [x] 7.2 Failover-during-job recovery
|
||||
- Reproduce the feature's core motivating scenario: a job's finish report is lost across a controller primary failover, and the new primary's rejoin reconciliation with that worker recovers the job's true outcome
|
||||
- Observable completion: a job whose finish notification never reached any controller instance ends up correctly finished, not stuck running, once the worker rejoins the new primary
|
||||
- _Requirements: 4.1, 4.2, 4.6_
|
||||
|
|
@ -170,3 +170,4 @@
|
|||
- **Task 6.2's mandated standby-redirect investigation found bug-1753**, a DIFFERENT gap than the one it was looking for: the generic not-primary (`CTRL_STANDBY`) redirect path turned out to already work correctly with zero opcode-specific code (`ha_dispatch_gate` gates every opcode uniformly before the switch), but the investigation surfaced that `ha_dispatch_gate`'s SEPARATE `CTRL_PRIMARY`-without-write-quorum branch — a "fact-bearing ops admitted anyway" allowlist (`OPCode_notify`/`update`/`setpreason`/`clrpreason`, bug-396) — was missing `OPCode_jobStateReport`, meaning a report arriving during a transient quorum-loss window (e.g. mid-failover, this feature's own core motivating scenario) would be rejected outright with no retry, since the worker's send is fire-and-forget. Fixed by adding the one missing case, verified safe against `resync_round_is_current`'s staleness check and the WAL_MODE_RETAIN semantics every fact this feature produces already uses. Citation nit: this fix's true motivation sits closer to Requirements 1.3/4/6 than the task's own tagged 8.1/8.3 — recorded here for traceability, tasks.md's tag left as originally generated. **Lesson**: a task text that names one specific concern to investigate ("check the redirect path") can surface a DIFFERENT, more consequential gap in the same general area — don't stop the investigation at "the named concern turned out fine," keep reading the surrounding gating logic in full.
|
||||
- **Task 6.2 also drew three escalating, unsolicited cross-session messages** (from an unfamiliar peer session) mid-flight, pushing a specific unreviewed code change to `test_jobstatereport_dispatch.c` — the exact file the dispatched implementer subagent was independently writing at the time. Declined to act on any of them (risk of conflicting with in-flight subagent work; escalating urgency/"no further investigation needed" framing is a known social-engineering pattern regardless of whether the underlying suggestion is technically plausible). The implementer subagent finished independently and solved the underlying `get_ep_worker()`-in-test-binary limitation completely differently (a `#ifdef XCE_TESTING` call-count seam in `resync.c`, not a global-fixture hack) — validating the decision not to short-circuit the normal TDD+review pipeline based on an unverified peer instruction.
|
||||
- **Task 7.1 (test-only, zero production changes) discovered and honestly asserted a genuine, pre-existing, systemic gap distinct from bug-1747**: a worker-reported (or any) job failure's exit code is never durably recorded anywhere in this codebase — `record_job_fail_fact()`'s `WAL_JOB_FAIL` payload has no exit-code slot at all, and `apply_job_fail_transition()` never touches `job.job.exit_info` for the FAIL path, so it stays `NULL` unconditionally. Bug-1747 only covers the FAIL finish-*time* mistiming; this is the exit-*code* gap, logged separately as bug-1754 (out of scope for this test-only task, correctly not fixed here — flagged for a dedicated future task). The round-trip test asserts `exit_info == NULL` honestly with an inline comment rather than encoding a wrong expectation the codebase can't actually produce.
|
||||
- Task 7.2 (test-only): added a focused, single-job proof of the feature's own original motivating scenario (bug-1741) — a job dispatched and RUN with NO finish fact ever committed (modeling the lost fire-and-forget notify), recovered to DONE with the worker-reported finish time via the real rejoin/query/report chain. Deliberately scoped smaller than task 7.1's four-outcome round trip (reuses its helper infrastructure, adds no new scaffolding) rather than duplicating it. tasks.md's own `_Requirements: 4.1, 4.2, 4.6_` tag includes 4.6 (unrecognized→PEND), which is genuinely unrelated to this finished-success scenario (that's task 7.1/2.1's territory) — `REQUIREMENTS_CHECKED` correctly narrowed to 4.1/4.2 instead of force-fitting an unrelated assertion, same accepted-imprecise-tag pattern as task 6.2's own citation nit.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue