docs(component-failover-adoption): mark task 5.1 complete, full-cluster E2E validated
Live 4-process E2E run (3-node controller cluster + xcecd_virtual worker + xce_scheduler + xce_apiserver, all on the full 3-candidate list) with the primary killed mid-session. Worker reconnect is solidly proven with real log evidence and unchanged PIDs across all three components throughout. Scheduler's rejoin path was never naturally exercised (no job reached its picker/allocator) -- same partial-coverage shape already accepted for task 3.3. Surfaced a likely-real, structural gap in the already-approved task 4.3/4.4 apiserver rejoin mechanism (bug-039): XCECLI_FF_CALL can silently self-heal the transport within a single heartbeat tick, making the cross-tick failure->success transition invisible to heartbeat.go's detector on any failover fast enough to complete within one heartbeat interval -- which is most real single-election failovers in this codebase. Also found bug-040 (job pushes never reach apiserver's store), very likely a downstream symptom of bug-039's rejoin gap via on_apiserver_join()'s per-process, unreplicated apiserver registration list. Both logged with detailed, independently-verified root-cause analysis for a dedicated follow-up -- fixing them is outside this task's cross-component-validation boundary. No src/ changes -- live validation only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNB4zGPCsHyDUCDJyMbiQN
This commit is contained in:
parent
79273a7f82
commit
6cb8f22b08
1 changed files with 2 additions and 1 deletions
|
|
@ -132,7 +132,7 @@
|
|||
- _Depends: 4.5_
|
||||
|
||||
- [ ] 5. Integration and cross-component validation
|
||||
- [ ] 5.1 E2E: full-cluster reconnect after primary failover
|
||||
- [x] 5.1 E2E: full-cluster reconnect after primary failover
|
||||
- Extend the project's existing HA soak pattern to a scenario with worker, scheduler, and apiserver all configured with the full candidate list; kill the primary mid-session with a write request in flight
|
||||
- Assert: worker reconnects and resumes job-state reporting; scheduler reconnects and resumes dispatch; apiserver reconnects, resumes serving API requests, and resumes receiving push notifications — none of the three restarted
|
||||
- Observable completion: the soak scenario passes with all three components showing a successful re-join to the new primary in their logs and no manual intervention
|
||||
|
|
@ -175,4 +175,5 @@
|
|||
- Task 3.4: pure test-coverage task, zero production changes to `scheduler.c`/`driver.c` (correct outcome — task 3.2 already implemented the retry/shutdown/rejoin behavior correctly; 3.4 proves it holds at multi-decision QUEUE granularity through the real drain loop, which task 3.2's own tests never exercised, all 6 of which call `sched_publish_dispatch()`/`sched_publish_pend_reason()` directly on one decision). New `test_scheduler_decision_redelivery.c` links real `scheduler_srcs` with `-Wl,--wrap=xcecli_jobdispatch` (task 3.2's established technique) and drives the real `xce_sched_driver_on_decision()` (driver.c) over 3 queued, distinct-jobid decisions. Two load-bearing tests, both RED→GREEN-verified against the real code by two independent reviewer rounds (not just the implementer's claim): ordering (decision 1 stuck-then-succeeds fully completes before decision 2's call ever fires) and bounded shutdown latency (decisions queued behind one that triggers `core.stop` mid-retry get ZERO publish calls — the concrete, fast, deterministic proof that `sched_publish_retry_loop()`'s check-`stop`-before-each-attempt placement is correct, verified by reviewers moving the check to bottom-of-loop and confirming only this one test catches it). Test-fixture gotcha for any future test building a bare `xce_sched_core_t` via `xce_sched_core_init()` alone (not full `sched_init()`): do NOT call `xce_sched_core_fini()` on it — `fini()` calls `dataq_fini()` on `pend_updates[0/1]`, which only `sched_init()` (not `xce_sched_core_init()`) ever `dataq_init()`s, so `fini()` on a bare core SEGVs on a NULL dataq; omit the `fini()` call entirely, matching `test_core_events.c`'s established "fixtures aren't torn down" convention for this exact minimal-fixture pattern.
|
||||
- Task 4.5: subtle design question resolved and worth remembering for task 4.6/5.x — `callBackend()`'s (`clib.go`) degraded fast-path deliberately skips `checkAndPerformRejoin()` ENTIRELY (not just the underlying `C.xcecli_*` call), meaning write-path requests contribute NOTHING to rejoin-retry cadence while `IsDegraded()` is true; only task 4.4's heartbeat goroutine keeps retrying during that window. Reviewer independently confirmed this is correct, not a violation of "must never suppress the pending-rejoin flag's own retry-until-actually-joined behavior": that constraint is one-directional (the circuit breaker's own optimistic TTL-based state must never be READ BY the rejoin mechanism to gate it — confirmed `checkAndPerformRejoin()` never references `IsDegraded()` anywhere), not a requirement that every possible call site attempt a rejoin check regardless of cost. The alternative (calling `checkAndPerformRejoin()`, which can itself block ~100s inside `xcecli_join()`'s own retry budget, on every fast-failed request) would directly violate THIS task's own fail-fast acceptance criterion — the two constraints (fail-fast + never-suppress-retry) are jointly satisfiable only because `heartbeat.go`'s goroutine runs unconditionally for the process lifetime, independent of circuit-breaker state, and `backendDegradedTTL` (10s, chosen well below the ~100s `XCECLI_FAILOVER_CALL` budget) is shorter than the heartbeat interval (default 30s) so write traffic resumes contributing once the TTL clears anyway. `callBackend()` is a genuine refactor consolidating what tasks 4.3/4.4 left as duplicated inline `rc := C.xcecli_*(...); checkAndPerformRejoin()` pairs across 5 wrapper functions into one shared helper — `checkAndPerformRejoin()` now appears exactly once in `clib.go`, inside `callBackend`, matching design.md's explicit "not copy-pasted per wrapper" instruction. `E_XCE_NOT_READY`-triggered `MarkDegraded()` uses a plain-Go `notReadyRC` mirror var (`= -int(C.E_XCE_NOT_READY)`, computed live via cgo, not hardcoded) since `_test.go` files can't reference `C.*` symbols directly (task 4.1's established Go-toolchain restriction) — keep this pattern for any future test needing a C errno constant.
|
||||
- Task 4.6 (closes out apiserver group 4.1-4.6): audit task, same "zero new code is a valid outcome when genuinely true" precedent as scheduler's task 1.3 — 2 of 3 coverage areas (degraded-to-healthy transitions, candidate-list parity) were already fully satisfied by tasks 4.1/4.5's own tests, verified by reading the cited tests directly rather than trusting prior notes' claims. Found and closed exactly one genuine gap: `reconnect.go`'s `needsRejoin`/`checkAndPerformRejoin` (task 4.3) had zero concurrent-access test coverage despite being reached from 5 real concurrent HTTP-wrapper call sites plus the heartbeat goroutine — `health.go`'s own 50-goroutine stress test (task 4.5) only covered the degraded-until timestamp, not this separate atomic flag. One new test closes it. `gClusterCfg`/`gClientCfg` (task 4.2's CGO structs) correctly judged NOT a concurrency gap — written once at startup, single-threaded, before the HTTP server starts, never mutated again. Confirmed apiserver's Go-level `xce_parse_peers()` malformed-string coverage (`candidates_test.go`) is broader than any C-side component's own test — worker's/scheduler's C tests, and even `test_cluster.c`, only exercise `xce_cluster_config_validate()` against pre-built structs, never `xce_parse_peers()` against malformed raw strings at all. `go test -race` is confirmed, again, flatly unusable in this ASAN-linked CGO environment ("too many address space collisions") — this is now the third task (4.2, 4.5, 4.6) to independently hit and document this; don't waste time trying `-race` here, rely on atomic-correctness-by-construction plus deterministic concurrent-stress assertions instead. `xce-build.sh apiserver asan`'s binary-compile step can hit transient `goproxy.cn` network flakiness unrelated to any code change — when that happens, fall back to `cd src/frontend && make test` (CGO_ENABLED=0, whole apiserver module + xcecli) and `make test-backend` (CGO_ENABLED=1, `./backend/...`) directly, which is what the full pipeline's own `RUN_TESTS=1` branch invokes anyway once binaries are built. A pre-existing, unrelated `apiserver/controllers` build failure under a whole-module `CGO_ENABLED=1 go test ./...` (predates this spec entirely, `integration_test.go`'s `//go:build !cgo` tag vs. `output_test.go`'s unconditional references to its helpers) doesn't affect either authoritative Makefile target — don't chase it as a regression in future apiserver work.
|
||||
- Task 5.1 (first Group 5 task, live 4-process E2E — worker + scheduler + apiserver + 3-node controller cluster): `xcecd_virtual` (worker's own task 2.3 meson target, `-DVIRTUAL_CLUSTER -DDBUS_MANAGER`) is the correct non-root worker E2E vehicle in this sandbox — use it, NOT `unshare -r`, which hits the unresolved bug-010 ASAN double-join abort. Worker's reconnect proved cleanly (real `checking_connection: rejoin after transparent reconnect succeeded` log line, 16s after kill, PID unchanged). Scheduler's rejoin path was never naturally exercised (no job ever reached its picker/allocator) — clean initial join + PID stability shown instead, same partial-coverage shape task 3.3 already got approved with. **Found a likely-real, structural gap in the already-approved task 4.3/4.4 apiserver rejoin mechanism (bug-039, see buglog for full detail)**: `XCECLI_FF_CALL` (backing `xcecli_heartbeat`) can silently self-heal the transport within a SINGLE heartbeat tick (rotate+recover, then the send succeeds) without ever returning a failure to Go — so `heartbeat.go`'s cross-tick `onFailureToSuccess` transition detector structurally cannot observe it when election completes faster than one heartbeat interval (elections in this codebase complete in single-digit seconds; default heartbeat interval is 30s), and none of apiserver's post-kill write calls showed evidence of hitting a failed connection either (consistent with the transport having already healed via a prior heartbeat tick before any write needed to retry). Net: `needsRejoin` may never get set on a real single-election failover, so apiserver's application-level re-join with the new primary never fires even though transport-level requests keep succeeding — risks Req 2 AC4 in production. This is NOT a task-5.1 fix (out of its cross-component-validation boundary) — flagged for a dedicated follow-up spec/task, escalated above ordinary buglog cadence given it touches an already-approved mechanism. **Also found bug-040** (job pushes never reach apiserver's store, `GET /xce/v1/jobs` empty despite controller-confirmed commits) — very likely a downstream symptom of bug-039, not independent: `on_apiserver_join()` (`apiserver.c`) only bulk-resyncs jobs when a fresh join is actually processed, and `c->apiservers[]` (the push fan-out list) is per-process in-memory state with no replication — a new primary starts with an empty registration table until each apiserver instance re-joins against it. Fixing bug-039 should be verified against bug-040 clearing as confirmation they share a root cause. Controller-side `join.c` logs only rejected/failed joins, never successful ones at any verbosity — a real observability gap for anyone debugging this family of issues in the future, worth fixing alongside.
|
||||
- Task 3.3: fixed a real production gap found along the way — `sched_init()`'s two all-candidates-unreachable/join-failure log lines logged only a candidate COUNT, never host:port names (same gap worker's task 2.3 fixed for the worker). Added `sched_format_candidate_list()`, threading `sched->cluster_cfg` through as a parameter rather than a new file-scope global (unlike worker's `g_cluster_cfg`) since `test_scheduler_connect.c` links `scheduler_srcs` as separate objects, not `#include`, so the `static` helper isn't directly callable from tests — coverage requires a real `dup2()`-based stdout capture around a live `sched_init()` call hitting the failure path, not a white-box direct call (see `capture_stdout_during_sched_init()` in `test_scheduler_connect.c` — reusable pattern for any future scheduler.c log-content assertion). Round-1 review caught this diagnostic fix shipping with zero test coverage (proven by reverting the 59-line diff and rebuilding: all existing tests still passed) — a reminder that "the fix works, I verified it manually/via live logs" is not the same as a committed regression test; always add the assertion, don't just observe the correct behavior once. The worst-case-occupancy backpressure test (`test_scheduler_backpressure.c`) only proves Req 3 AC2 (buffering/block/resume-with-no-loss) — it does not exercise task 3.2's `-E_XCE_NOT_READY` retry path, so don't cite it for Req 7 coverage (that's task 3.4's job). E2E kill-primary/reconnect and all-unreachable-at-startup were validated manually against a real 3-node `/xce-deploy-ha` cluster rather than committed as an automated test, following worker's task 2.3 precedent — this repo has no automated live-multi-process-deploy E2E harness for any C daemon (the only one, `test_e2e_live_ha_push.py`, is Python/apiserver-only); reviewer judged this acceptable both rounds. Manual validation confirmed join, primary-death survival, and lazy socket-orphaning-on-idle (matches design), but could not observe a live rejoin firing against real sockets — blocked by bug-010 (`xcecd` ASAN "already joined thread" abort under `unshare -r`, needed to bypass the worker daemon's root-only check in this sandbox; unconfirmed whether it's a genuine bug or a userns artifact, never tried under real root). The exact rejoin-after-publish-success code path already has solid white-box mock coverage from task 3.2's `test_rejoin_composition_retries_across_publish_calls`, so this residual gap is narrowly "proven against mocks, not yet against a live socket" — worth closing opportunistically if bug-010 ever gets resolved, not a blocker for this spec.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue