feat(component-failover-adoption): validate worker thread isolation, fix rejoin bugs found by E2E
Confirm connector thread is isolated from worker_process_update() (main thread) and resource_sync_thread() (own thread) via code read plus a real pthread concurrency test. Add unit and live 3-node E2E coverage for candidate/reconnect behavior, all-candidates-unreachable diagnostics, and liveness cadence under concurrent reconnect. The E2E run surfaced two production bugs the 2.1/2.2 unit tests missed: xcecli_start() resets the reconnect callback to NULL after it runs, so registering it before xcecli_start() silently no-ops the whole rejoin mechanism; and a successful worker join returns its assigned slot index as reply.status (any rc >= 0), not always exactly 0, so checking_connection()'s rc == 0 success check never fired on a real non-zero-slot join. Both fixed, plus per-candidate names added to the all-unreachable failure log. Root-check bypass needed for the E2E run is gated at compile time via the existing VIRTUAL_CLUSTER flag, not a runtime env var, so the real xcecd binary's root requirement has zero bypass surface.
This commit is contained in:
parent
331fa02423
commit
619b119ef5
5 changed files with 455 additions and 8 deletions
|
|
@ -41,7 +41,7 @@
|
|||
- _Requirements: 1.4, 2.5_
|
||||
- _Depends: 2.1_
|
||||
|
||||
- [ ] 2.3 Validate worker thread isolation and add worker candidate/reconnect test coverage
|
||||
- [x] 2.3 Validate worker thread isolation and add worker candidate/reconnect test coverage
|
||||
- Confirm the connector thread is isolated from the worker's other recurring liveness duties, and that a blocked connector thread during retry/backoff does not stall them
|
||||
- Add end-to-end coverage: worker starts and joins across a multi-candidate config; killing the joined primary results in reconnect and re-join to a surviving candidate without a worker restart
|
||||
- Add coverage for the case where every configured candidate is unreachable or rejects the join at startup: assert the worker reports a clear failure identifying every candidate it attempted, not a generic error
|
||||
|
|
@ -166,3 +166,4 @@
|
|||
- Task 1.3: closed with zero new code. Verified fresh (source read + a clean `xce-build.sh lib asan` run) that every item in its checklist was already satisfied by prior tasks: the `_Static_assert(N_MAX_MASTERS == MAX_HA_CANDIDATES, ...)` landed in task 1.1 (`test_libxce_retry.c:44`), and the four behavioral cases (order/no-branch/truncation/NULL-empty) landed in task 1.2's 8 tests (`test_cluster.c`). No implementer/reviewer cycle was dispatched since there was no diff to review — dispatching one would have either duplicated existing test names (build conflict) or been a no-op review of an empty diff.
|
||||
- Task 2.1: `xce_cluster_config_to_client_config()` (task 1.2) aliases `out->masters[i]` directly to `cfg->candidates[i].hostname` — no `strdup`. Any caller building the source `xce_cluster_config_t` on the stack and passing it through the helper then using the resulting client config beyond that stack frame's lifetime has a use-after-scope bug. Worker's fix keeps the source config in a file-scope `static` for this reason; tasks 3.1 (scheduler) and 4.2 (apiserver) hit the identical hazard and must do the same (or otherwise guarantee the source config outlives the client config's use).
|
||||
- Task 2.2: fixed the startup-ordering bug by threading `cluster_cfg` by value through `xcecd_config_t` → `xcecd_context_t` → `cc_ctx` (set in `main.c` before `xcecd_init()`/`xcecd_start()` run, instead of after). Safe as a plain struct copy because `xce_candidate_t.hostname` is a fixed `char[]` array, not a pointer — no aliasing hazard here (unlike the `xce_cluster_config_to_client_config()` output struct noted above). Scheduler (3.1) likely has the same ordering gap (`sched_init` loops candidates correctly already per design.md, so may not need this fix — verify against current source, don't assume). Rejoin-retry pattern established: `checking_connection()` checks `needs_rejoin` after every probe return, calls `xcecli_join()` directly (not full re-entry into the join-cluster function), clears the flag only on success — same pattern scheduler/apiserver rejoin tasks (3.1, 4.3) should follow.
|
||||
- Task 2.3 (real E2E caught what unit tests missed — 2 production bugs slipped through tasks 2.1/2.2's approved reviews): (1) `xcecli_start()` (`src/lib/client.c`) unconditionally resets `ctx->failover.rejoin_cb = NULL` as part of its own init — any `xcecli_set_reconnect_handler()` call before `xcecli_start()` is silently wiped. Register the reconnect handler AFTER `xcecli_start()` succeeds, not before. Scheduler/apiserver's own reconnect-handler registration (3.1, 4.3) must follow this same ordering or they'll hit the identical silent-wipe bug. (2) A successful `xcecli_join()`/worker join returns the assigned slot index as `reply.status` (any `rc >= 0`, not always exactly `0`) — checking `rc == 0` for success is wrong; use `rc < 0` as the only failure test (matches `xcecd_join_cluster()`'s own established convention). Any new code checking a join/rejoin call's success must use this same `rc < 0` convention, not `rc == 0`. (3) Security lesson: an env-var runtime bypass of a security check (`XCECD_ALLOW_NONROOT`) added for test/E2E convenience was rejected on review — the correct pattern for skipping a production-only check (root privilege, etc.) in a test/virtual build is a compile-time guard against the project's existing `VIRTUAL_CLUSTER` or `XCE_TESTING` flags, never a runtime environment variable reachable in the shipped production binary.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue