Rongsong Shen
06135954ff
Pre-existing uncommitted work: ha-repl-hardening payload data plane (7.x), heartbeat liveness fix (Req 1.3), connection-reuse ASAN fixes, doc cleanup. New: ha-consistency-invariants umbrella (requirements/design/research) and its four implementation sub-specs (specs generated via batch, cross-spec review applied), steering roadmap + product.md Raft+CP decision.
86 lines
6.4 KiB
Markdown
86 lines
6.4 KiB
Markdown
# Implementation Plan
|
|
|
|
## Task Format
|
|
|
|
- Detail items
|
|
- Observable completion condition
|
|
- _Requirements: X.X, Y.Y_
|
|
|
|
---
|
|
|
|
- [x] 1. Foundation: write failing tests for all new worker join behaviors (TDD Red)
|
|
|
|
- [x] 1.1 Write failing tests for the shared status and resource update step
|
|
- Add a new test file (e.g., `test_worker_join.c`) to `src/controller/testsuite/` and register it in `controller_test.c`
|
|
- Tests assert: after the helper runs, `XCE_WORKER_UNAVAIL` is cleared on the worker's status, hosttype/model match the hconfig override values, slot count equals the hconfig mxj value, and host-attribute resources (type, model, cpuf, maxcpus, maxmem, maxswap, maxtmp, ndisks) hold the expected values
|
|
- Tests compile but fail because the helper does not yet exist
|
|
- _Requirements: 4.1, 4.2, 4.3_
|
|
|
|
- [x] 1.2 Write failing tests for the first-time worker creation path
|
|
- Tests assert: calling the creation function with a previously-unknown hostname produces a non-negative index; `key_find` on `workermgr` with that name succeeds after the call
|
|
- Tests assert: when the creation function returns an error, the error code is negative and no `on_worker_join` side effect occurs
|
|
- Tests compile but fail because the function does not yet exist
|
|
- _Requirements: 2.1, 2.2, 2.3_
|
|
|
|
- [x] 1.3 Write failing tests for the rejoin update path with admin-bit preservation
|
|
- Tests assert: after the update function runs, the worker's `host.attrs` reflects the new values and `w->ep` matches the new endpoint
|
|
- Tests assert: admin status bits (`XCE_WORKER_LOCKEDU`, `XCE_WORKER_CLOSED`, `XCE_WORKER_HIDDEN`, `XCE_WORKER_CLOSED_BY_ADMIN`) present before the call are still set after it
|
|
- Tests compile but fail because the function does not yet exist
|
|
- _Requirements: 3.1, 3.2, 3.3_
|
|
|
|
---
|
|
|
|
- [x] 2. Core: extract shared status and resource update into a standalone helper (TDD Green — 1.1)
|
|
- Move the status-clear + hconfig-override + resource-update block out of `add_worker` in `worker.c` into a new `static` helper function
|
|
- The helper covers: clearing `XCE_WORKER_UNAVAIL`, assigning `w->ep`, overriding hosttype/model strings from hconfig (with `dereference`/`makeString`), calling `add_hostattrs_resvals`, computing mxj slots via `set_host_slots`, replacing `dispatch_window_config` and `thresholds` from hconfig, and looping over `hconfig->nresources` with `mark_host_res`
|
|
- Must be called under the caller-held per-worker write lock; document this precondition with a comment
|
|
- Tests from 1.1 turn green; `add_worker` still compiles by delegating to the new helper
|
|
- _Requirements: 4.1, 4.2, 4.3_
|
|
|
|
---
|
|
|
|
- [x] 3. Core: implement the first-time creation and rejoin update helpers
|
|
|
|
- [x] 3.1 Implement the first-time worker creation function (TDD Green — 1.2)
|
|
- Build a temporary `xce_worker_t` from the incoming hostname and attrs, then call `makeWorker` to register it in `workermgr` (makeWorker manages its own write lock)
|
|
- On a `NULL` return from `makeWorker`, log the failure and return `-errno` to the caller without touching `on_worker_join`
|
|
- On success, acquire the per-worker write lock, call the shared helper from Task 2, release the lock, and return `w->idx`
|
|
- Tests from 1.2 turn green
|
|
- _Requirements: 2.1, 2.2, 2.3_
|
|
|
|
- [x] 3.2 Implement the rejoin worker update function (TDD Green — 1.3)
|
|
- Acquire the per-worker write lock at the start; save `admin_status = w->host.status & XCE_WORKER_ADMIN_MASK` before any modification
|
|
- Free and replace the existing `hosttype` and `hostmodel` strings; overwrite numeric attrs fields (`cpufactor`, `maxcpus`, `maxmem`, `maxswap`, `maxtmp`, `ndisks`) from the incoming attrs argument
|
|
- Call the shared helper from Task 2 (status/resource update, ep assignment), then restore admin bits via `w->host.status |= admin_status`; release the lock
|
|
- Return `w->idx`; tests from 1.3 turn green and admin bits are unchanged after a full round-trip
|
|
- _Requirements: 3.1, 3.2, 3.3_
|
|
|
|
---
|
|
|
|
- [x] 4. Integration: wire the explicit existence check and new helpers into the join handler
|
|
|
|
- [x] 4.1 Update exported worker function declarations
|
|
- Remove the `add_worker` declaration from `src/controller/include/worker.h`
|
|
- Add declarations for the first-time creation function and the rejoin update function with their full parameter lists (including the `xce_hostconfig_t*` parameter)
|
|
- Verify that the only direct caller of `add_worker` was `handle_worker_join`; if any other callers exist, update them to use the appropriate new function
|
|
- `worker.h` compiles cleanly with no remaining reference to `add_worker`
|
|
- _Requirements: 5.4_
|
|
|
|
- [x] 4.2 Rewrite the worker join handler with explicit existence check and branching
|
|
- Remove the outer `lock(c->workermgr, RTTI_LOCK_WRITE)` / `unlock` pair that currently wraps the `add_worker` call (eliminates the double write-lock deadlock)
|
|
- Move `find_matched_hostconfig` to the top of the handler, before any lock; reject the join with the appropriate error code and set `reply.status` if it returns `NULL`
|
|
- Acquire a brief workermgr read lock, call `key_find`, release the read lock immediately
|
|
- On `-ENOENT` call the first-time creation function; on a found entry call the rejoin update function; on any other error from `key_find` reject the join and return that error code
|
|
- Fire `BEFORE_EVENT / HOOK_DETAIL_WORKER_JOIN` before the `key_find` step; fire `AFTER_EVENT` after the create-or-update completes; call `xce_reply` and `on_worker_join` outside any lock
|
|
- Set `priv->u.idx` before calling `on_worker_join`; if create or update returned an error, set `reply.status`, call `xce_reply`, and return without calling `on_worker_join`
|
|
- The join handler compiles and the sequence check → create-or-update → status/resource-update is visible at the call site with no hidden branching inside `makeWorker`
|
|
- _Requirements: 1.1, 1.2, 1.3, 2.2, 2.3, 4.4, 5.1, 5.2, 5.3_
|
|
|
|
---
|
|
|
|
- [x] 5. Validation: run full test suite and confirm behavioral preservation
|
|
- Build the controller test binary and run all suites in `src/controller/testsuite/`
|
|
- Confirm `test_e2e_hooks` records `BEFORE_EVENT` before any workermgr mutation and `AFTER_EVENT` after it, matching the pre-refactor ordering
|
|
- If any test fixture in the suite constructed workers by calling `add_worker` directly, update the fixture to call the creation function with the added `xce_hostconfig_t*` argument
|
|
- `make test` exits 0; no previously-passing test is broken by the refactor
|
|
- _Requirements: 5.1, 5.2, 5.3, 5.4_
|