test(worker-rejoin-resync): task 7.4 - large job-count pagination correctness
Add test_large_job_count_pagination_correctness, proving both OPCode_jobStateQuery and OPCode_jobStateReport genuinely span many wire pages (1042 jobs, 11 pages each direction, including a partial final page) with zero jobs dropped/duplicated/truncated - exact-count assertions throughout (page continuity, no-duplicate bitmap, exact WAL delta, exact hook-fire count), extending task 6.2's 2-page proof by an order of magnitude. Test-only, no production changes. Logs bug-1755: the underlying query-pagination mechanism this test proves correct is never exercised in production - worker_send_resync_ query() hardcodes pktlimit=0, so the controller's real query send always produces one unbounded page regardless of job count, asymmetric with the report direction's real XCECD_RESYNC_REPORT_PKTLIMIT bound. No hard transport-layer size cap exists, so this isn't a demonstrated failure mode, but it contradicts design.md's explicit "both paginated" claim in this feature's own headline mechanism. Adds task 7.5 to close it with a small, well-scoped fix before the feature-level gate, rather than leaving a silently-false design claim in the completed feature. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
165fc1586a
commit
613b80e991
3 changed files with 715 additions and 1 deletions
|
|
@ -147,12 +147,19 @@
|
|||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 7.1_
|
||||
|
||||
- [ ] 7.4 Large job-count pagination correctness
|
||||
- [x] 7.4 Large job-count pagination correctness
|
||||
- Drive the reconciliation exchange for a worker running enough jobs to require multiple pages of both the query and the report, confirming the exchange completes correctly rather than truncating or silently dropping jobs past a single page
|
||||
- Observable completion: a job count an order of magnitude larger than a single page holds is fully queried and fully reported, with every job accounted for
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 7.1_
|
||||
|
||||
- [ ] 7.5 Wire a real page-size bound into the controller's outbound reconciliation query
|
||||
- Close bug-1755, found and correctly left unfixed by task 7.4's own test-only boundary: `worker_send_resync_query()` (`src/controller/worker.c`) hardcodes `pktlimit=0` in its `xce_job_state_query_req()` call, which task 7.4 independently confirmed means "send every non-terminal job ID in one unbounded page" — so the query direction never actually paginates in production today, even though task 1.4's underlying pagination mechanism (proven correct by task 7.4's own test) fully supports it, and design.md's Boundary Commitments explicitly states both opcodes are paginated
|
||||
- Give the query send site a real, bounded page size, mirroring the worker's own already-wired `XCECD_RESYNC_REPORT_PKTLIMIT` precedent (`src/worker/src/controller_connector.c`) — same constant-driven approach, not a new configuration surface
|
||||
- Observable completion: a worker rejoin with a non-terminal job count exceeding the chosen page size produces a query genuinely sent across multiple wire pages (observably, not just theoretically supported), with the same job-count-independent completeness task 7.4 already proved at the mechanism level; a rejoin with a job count under one page still produces exactly one page, unchanged from today's observable behavior
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 6.1, 7.4_
|
||||
|
||||
## Implementation Notes
|
||||
- Task 1.3: this task's premise (`src/worker` has no `testsuite/`) is FALSE as of implementation time — real, tracked worker test infrastructure already exists (`src/worker/testsuite/{meson.build,test_*.c}`, 5 passing tests, `suite: 'worker'` registered in meson, `/xce-build worker` already runs it), built by `component-failover-adoption` (commits `ce9f734a`/`331fa024`/`619b119e`, 2026-08-11) weeks before this spec was written. `.kiro/steering/structure.md`, `testing.md:55`, and `build-deployment.md:16` are all stale on this point — worth a `/kiro-steering` sync, out of this feature's boundary to fix. Confirmed directly (fresh build + `meson test -C src/xce.build --suite worker`: 5/5 OK) rather than trusting the finding secondhand. No files changed for this task — closed as satisfied by pre-existing infrastructure.
|
||||
- **Task 4.2 must follow the EXISTING worker testsuite convention**, not the controller aggregator-runner pattern the original task 1.3 text pointed at: each test file is its own standalone Unity executable (own `UNITY_BEGIN()`/`RUN_TEST`/`UNITY_END()`, white-box `#include`s the production `.c` file under test — see `test_resource_sync.c`'s shape) with its own explicit `executable()` + `test(..., suite: 'worker', ...)` block added to `src/worker/testsuite/meson.build`, mirroring `test_controller_connector.c`'s registration exactly. Do not introduce a second, competing test-runner shape.
|
||||
|
|
@ -172,3 +179,4 @@
|
|||
- **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.
|
||||
- Task 7.3 (test-only): reproduced bug-1741's exact original shape (7 jobs, 6 lost) in one round/one wire page — 5 finished-success + 1 finished-failure recovered, plus one already-DONE job re-reported as an unqueried extra with a deliberately wrong finish time, proving reconciliation neither leaves any lost job stuck nor duplicates/clobbers an already-terminal one (exact `+6` WAL LSN delta, not `>=`, so a duplicate append is genuinely caught — confirmed via RED-phase reproduction against `record_job_finish_fact_at`'s terminal-state guard). Deliberately kept to a single wire page (page-boundary correctness is task 7.4's separate concern). Two reviewer/implementer subagents in this session's task 6.2/7.2 review rounds hit the "background command with no external wakeup" trap (ended their turn on a backgrounded build, never resumed) — both implementer and reviewer prompts for 7.3 onward now carry an explicit foreground-only warning up front; worth keeping in every future subagent dispatch in this codebase, not just implementer roles.
|
||||
- **Task 7.4 (test-only) found and correctly declined to fix bug-1755, but the coordinator escalated it to a new task 7.5 rather than leaving it passively logged.** Proved 1042 jobs correctly query and report across 11 wire pages each direction (exact-count assertions throughout — page continuity, no-duplicate bitmap, exact WAL delta, exact hook-fire count), extending task 6.2's 2-page proof by an order of magnitude, per the task's own "no truncation past a single page" mandate. While building it, discovered the underlying `xce_job_state_query_req()` pagination mechanism it was proving correct is never actually exercised in production: `worker_send_resync_query()` (task 6.1) hardcodes `pktlimit=0`, meaning the controller's real query send ALWAYS produces exactly one unbounded page regardless of job count — asymmetric with the report direction's real, wired `XCECD_RESYNC_REPORT_PKTLIMIT=100` bound. No hard transport-layer message-size cap was found (dynamic malloc, `uint64_t` length prefix), so this isn't a demonstrated crash/hang/data-loss risk and Requirement 8.1's literal "without... failing" text is technically satisfied — but design.md's Boundary Commitments explicitly states both opcodes are "paginated," which is not true of the shipped code, in this feature's own headline mechanism, discovered in this feature's own final validation task. Per the review's explicit recommendation, added task 7.5 (small, well-scoped: mirror the report direction's constant-driven precedent) rather than accepting a silently-false design claim into the completed feature — same "found a real gap, fold it into an explicit task rather than a passive note" pattern this spec has followed since tasks 1.5/6.1's config-delivery gaps.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue