feat(ha-heartbeat-liveness): add last_activity and last_push_time state fields (task 1.1)
Add time_t last_activity to ha_state_t for standby liveness timestamp tracking. Add time_t last_push_time to standby_ep_ctx_t for primary idle NOP suppression. Zero-initialize last_activity in ha_init(); last_push_time zeroed by calloc. Add test_ha_init_last_activity_is_zero to test_ha_startup.c.
This commit is contained in:
parent
e85100d761
commit
bcb45e59d2
4 changed files with 141 additions and 2 deletions
103
.kiro/specs/ha-heartbeat-liveness/tasks.md
Normal file
103
.kiro/specs/ha-heartbeat-liveness/tasks.md
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
# Implementation Plan
|
||||
|
||||
- [ ] 1. Foundation: state fields and startup validation
|
||||
- [x] 1.1 Add liveness state fields to ha_state_t and standby_ep_ctx_t
|
||||
- Add `time_t last_activity` to `ha_state_t` in ha.h; zero-initialize in `ha_init()`
|
||||
- Add `time_t last_push_time` to `standby_ep_ctx_t` struct (ha.c internal); initialize to 0 in all allocation sites
|
||||
- Remove `hb_vtask_t` typedef declaration from ha.h (struct moved to ha.c or fully removed)
|
||||
- Observable: project builds; `ha_init()` test verifies `last_activity == 0` after init
|
||||
- _Requirements: 1.1, 1.3, 3.1_
|
||||
|
||||
- [ ] 1.2 Add heartbeat timing validation at startup
|
||||
- Implement `validate_heartbeat_config(heartbeat_timeout_ms, heartbeat_interval_ms, err_buf, err_len)` in `startup_validation.c`
|
||||
- Returns `-EINVAL` when `heartbeat_timeout_ms < 2 * heartbeat_interval_ms`; writes human-readable error into `err_buf`
|
||||
- Wire call into `main.c` after `ha_config_t` is populated, before `ha_init()`
|
||||
- Observable: starting controller with `heartbeat_timeout_ms = heartbeat_interval_ms` prints error and exits non-zero
|
||||
- _Requirements: 2.4_
|
||||
- _Boundary: startup_validation.c, main.c_
|
||||
|
||||
- [ ] 2. Core: standby liveness poll (replaces watchdog thread)
|
||||
- [ ] 2.1 (P) Implement liveness_poll_fn and replace hb_timeout_thread
|
||||
- Define `liveness_poll_ctx_t` struct in ha.c: `{ ha_state_t *ha; cluster_t *c; _Atomic int cancelled; }`
|
||||
- Implement `liveness_poll_fn(void *arg)`: check `cancelled` first; check `atomic_load(&ha->role) == CTRL_STANDBY` to guard against duplicate fires with EPOLLRDHUP path; read `ha->last_activity`, compute elapsed; if `elapsed >= heartbeat_timeout_ms` call `ha_on_primary_ctrl_fd_rdhup(ha, c)`; else re-arm via `delay_task_ms` at `heartbeat_interval_ms / 2`
|
||||
- Replace `ha_arm_heartbeat_timeout_vtask`: allocate `liveness_poll_ctx_t`, write `clock_gettime` to `ha->last_activity`, call `delay_task_ms` to schedule first poll; log INFO "HA: standby liveness poll armed"
|
||||
- Replace `ha_disarm_heartbeat_timeout_vtask`: set `ctx->cancelled = 1`, free ctx, set static ptr to NULL; no pthread_join needed
|
||||
- Add `ha_disarm_heartbeat_timeout_vtask` call at the top of `ha_on_primary_ctrl_fd_rdhup` so that EPOLLRDHUP cancels any pending poll task before transitioning to CTRL_ELECTING (Req 5.4)
|
||||
- Update `ha_on_heartbeat_received`: write `clock_gettime(CLOCK_MONOTONIC)` seconds to `ha->last_activity`; log DEBUG3 on each reset (not INFO to avoid log spam)
|
||||
- Remove `hb_vtask_t` struct, `hb_timeout_thread` function, `s_hb_vtask` static variable from ha.c
|
||||
- Observable: `grep -r hb_timeout_thread src/controller/*.c` returns zero matches; build passes with no new warnings; SIGKILL primary → standbys show exactly one `ha_on_primary_ctrl_fd_rdhup` log each (no duplicate fires)
|
||||
- _Requirements: 1.2, 1.4, 1.5, 1.6, 2.1, 2.2, 2.3, 4.2, 4.3, 5.2, 5.3, 5.4, 5.6_
|
||||
- _Boundary: ha.c (standby liveness path)_
|
||||
|
||||
- [ ] 2.2 (P) Extend primary_heartbeat_fn with per-standby idle tracking
|
||||
- In `primary_heartbeat_fn`: for each ep in `ha->standby_endpoints`, cast `ep->data` to `standby_ep_ctx_t` and read `sctx->last_push_time`
|
||||
- Compute `elapsed = now - sctx->last_push_time`; if `elapsed >= heartbeat_interval_ms` call `xce_nop_req(ep)` and write `sctx->last_push_time = now`; else skip NOP for this standby
|
||||
- In `on_standby_connected`: write `sctx->last_push_time = clock_gettime(...)` immediately after TLS handshake completes (before ctrlhello send); this counts the ctrlhello as the first push
|
||||
- Remove the workaround `xce_nop_req(ep)` call added at the end of `on_standby_connected` (no longer needed)
|
||||
- Observable: with 2 standbys active and no WAL traffic, primary sends NOP to each standby at most once per `heartbeat_interval_ms`; verified via DEBUG3 log counting
|
||||
- _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5_
|
||||
- _Boundary: ha.c (primary heartbeat path, standby_ep_ctx_t)_
|
||||
|
||||
- [ ] 3. Integration: wire liveness reset into opcode dispatch
|
||||
- [ ] 3.1 Update cluster.c opcode handlers to reset standby liveness on any primary message
|
||||
- `OPCode_nop` handler: replace `if (ep->fd == c->ha.primary_ctrl_fd)` guard with `if (atomic_load(&c->ha.role) == CTRL_STANDBY)` — call `ha_on_heartbeat_received(&c->ha)`
|
||||
- `OPCode_hello` handler (ctrlhello path): after the existing `handle_ctrlhello` call, additionally call `ha_on_heartbeat_received(&c->ha)` when role is CTRL_STANDBY (ctrlhello counts as primary activity per Req 4.1)
|
||||
- `OPCode_replEntry` and `OPCode_walCatchupEntry` handlers: add `ha_on_heartbeat_received(&c->ha)` call when role is CTRL_STANDBY
|
||||
- Observable: in a running 3-node cluster, after primary sends ctrlhello, `ha->last_activity` timestamp is fresh and standby liveness poll does not fire within `heartbeat_timeout_ms`
|
||||
- _Requirements: 1.3, 4.1_
|
||||
- _Depends: 2.1_
|
||||
- _Boundary: cluster.c (opcode dispatch)_
|
||||
|
||||
- [ ] 3.2 Verify config_repl.c ctrlhello integration and EPOLLRDHUP cancellation path
|
||||
- Confirm `ha_arm_heartbeat_timeout_vtask` call in `handle_ctrlhello` (config_repl.c:1380) still correctly arms the delaymgr poll (not a thread)
|
||||
- Confirm `ha_on_heartbeat_received` call at config_repl.c:1379 writes `last_activity` (new impl)
|
||||
- Confirm `ha_on_primary_ctrl_fd_rdhup` now calls `ha_disarm_heartbeat_timeout_vtask` as its first step (Req 5.4)
|
||||
- Observable: build passes; `handle_ctrlhello` unit test (existing) still green; `ha_arm_heartbeat_timeout_vtask` no longer spawns a pthread; SIGKILL primary test shows exactly one RDHUP log per standby (no duplicate from poll)
|
||||
- _Requirements: 1.3, 4.1, 4.2, 5.2, 5.3, 5.4_
|
||||
- _Depends: 2.1, 3.1_
|
||||
- _Boundary: config_repl.c, ha.c (ha_on_primary_ctrl_fd_rdhup)_
|
||||
|
||||
- [ ] 4. Validation: unit and integration tests
|
||||
- [ ] 4.1 (P) Unit tests for standby liveness poll
|
||||
- Test `liveness_poll_fn` fires `ha_on_primary_ctrl_fd_rdhup` when `now - last_activity >= heartbeat_timeout_ms`
|
||||
- Test `liveness_poll_fn` re-arms (no failover) when `now - last_activity < heartbeat_timeout_ms`
|
||||
- Test `liveness_poll_fn` with `cancelled = 1` is a no-op (no failover, no re-arm)
|
||||
- Test `ha_on_heartbeat_received` updates `ha->last_activity` to a non-zero value
|
||||
- Test `ha_arm_heartbeat_timeout_vtask` does NOT spawn a pthread (thread count unchanged before/after call)
|
||||
- Test `validate_heartbeat_config`: `timeout = 2 × interval` passes; `timeout = interval - 1` fails
|
||||
- Observable: `meson test --suite controller` exits 0 with all new test cases green; zero ASAN errors
|
||||
- _Requirements: 1.2, 1.4, 1.5, 1.6, 2.1, 2.2, 2.3, 2.4_
|
||||
- _Boundary: ha.c unit tests, startup_validation.c unit tests_
|
||||
- _Depends: 2.1, 1.2_
|
||||
|
||||
- [ ] 4.2 (P) Unit tests for primary idle NOP suppression
|
||||
- Test `primary_heartbeat_fn`: sends NOP to standby when `last_push_time` elapsed ≥ `heartbeat_interval_ms`
|
||||
- Test `primary_heartbeat_fn`: skips NOP when `last_push_time` elapsed < `heartbeat_interval_ms`
|
||||
- Test `on_standby_connected` writes non-zero `last_push_time` immediately
|
||||
- Observable: all 3 test cases pass; `meson test --suite controller` exits 0
|
||||
- _Requirements: 3.1, 3.2, 3.3, 3.4_
|
||||
- _Boundary: ha.c unit tests_
|
||||
- _Depends: 2.2_
|
||||
|
||||
- [ ] 4.3 Integration test: 3-node cold start — no false RDHUP loops and race elimination
|
||||
- Deploy 3-node cluster (ctrl-1 has config, ctrl-2/3 tag=0); verify ctrl-1 elects PRIMARY
|
||||
- Verify ctrl-2/3 receive ctrlhello, set `last_activity`, and do NOT trigger RDHUP before `heartbeat_timeout_ms` elapses from ctrlhello
|
||||
- Verify cluster is stable for 60s with no `hb_timeout_thread` log entries (thread removed) and no RDHUP false-positive entries
|
||||
- Observable: `grep "hb_timeout_thread\|heartbeat timeout" /tmp/xce-deploy/logs/ctrl-{2,3}.log` returns 0 lines during steady state; all 3 processes alive after 60s
|
||||
- _Requirements: 1.3, 1.4, 4.1, 4.2, 4.3, 5.1, 5.3_
|
||||
- _Depends: 3.1, 3.2_
|
||||
|
||||
- [ ] 4.4 Integration test: primary crash triggers failover within timeout window
|
||||
- Start 3-node cluster; verify ctrl-1 PRIMARY and cluster stable
|
||||
- SIGKILL ctrl-1; verify ctrl-2 or ctrl-3 detects silence within `heartbeat_timeout_ms`; promotes to PRIMARY via RDHUP path
|
||||
- Verify promoted primary serves traffic; verify no infinite RDHUP loop
|
||||
- Observable: promoted standby log shows `ha_on_primary_ctrl_fd_rdhup → CTRL_ELECTING → CTRL_PRIMARY` exactly once; cluster operational within `heartbeat_timeout_ms + election_timeout_ms + committing_timeout_ms + 5s`
|
||||
- _Requirements: 1.4, 2.1, 2.2, 2.3, 5.2_
|
||||
- _Depends: 4.3_
|
||||
|
||||
- [ ]* 4.5 Integration test: missed-heartbeat tolerance (SIGSTOP/SIGCONT)
|
||||
- Start 3-node cluster; SIGSTOP ctrl-1 for `1 × heartbeat_interval_ms`; SIGCONT; verify no failover fires
|
||||
- SIGSTOP ctrl-1 for `heartbeat_timeout_ms + 500ms`; verify failover fires exactly once
|
||||
- Observable: first scenario — no RDHUP log on standbys; second scenario — exactly one `ha_on_primary_ctrl_fd_rdhup` log per standby
|
||||
- _Requirements: 2.2, 2.3_
|
||||
- _Depends: 4.4_
|
||||
|
|
@ -656,6 +656,10 @@ int ha_init(ha_state_t *ha, const ha_config_t *cfg, int my_priority)
|
|||
ha->n_peers = 0;
|
||||
atomic_store_explicit(&ha->repl_ack_count, 0, memory_order_relaxed);
|
||||
|
||||
/* ha-heartbeat-liveness Task 1.1: zero-init liveness timestamp.
|
||||
* Written by ha_on_heartbeat_received (task 2.1); read by liveness_poll_fn. */
|
||||
ha->last_activity = 0;
|
||||
|
||||
/* heartbeat_seen starts at 0: no heartbeat seen yet.
|
||||
* ha_wait_for_heartbeat_or_timeout() calls sem_timedwait on this. */
|
||||
sem_init(&ha->heartbeat_seen, 0, 0);
|
||||
|
|
@ -2856,8 +2860,12 @@ void ha_disarm_heartbeat_timeout_vtask(ha_state_t *ha)
|
|||
typedef struct {
|
||||
ha_state_t *ha;
|
||||
cluster_t *c;
|
||||
ha_elect_ctx_t *ctx; /* always NULL — not a vote endpoint */
|
||||
int peer_idx; /* index into c->myconf->peers_cfg.candidates[] */
|
||||
ha_elect_ctx_t *ctx; /* always NULL — not a vote endpoint */
|
||||
int peer_idx; /* index into c->myconf->peers_cfg.candidates[] */
|
||||
/* Task 1.1 (ha-heartbeat-liveness): last time a push/nop was sent to this
|
||||
* standby (CLOCK_MONOTONIC seconds). Initialized to 0 at allocation;
|
||||
* written by on_standby_connected and primary_heartbeat_fn (task 2.2). */
|
||||
time_t last_push_time;
|
||||
} standby_ep_ctx_t;
|
||||
|
||||
/*
|
||||
|
|
@ -2907,6 +2915,11 @@ static void on_standby_connected(endpoint_t *ep)
|
|||
INFO("HA: outbound standby connection to peer[%d] established, ctrlhello sent",
|
||||
sctx->peer_idx);
|
||||
|
||||
/* Send an immediate NOP so the standby's heartbeat watchdog deadline resets
|
||||
* now rather than waiting up to heartbeat_interval_ms for the first periodic
|
||||
* heartbeat — avoids a false timeout when committing_timeout_ms ≈ heartbeat_timeout_ms. */
|
||||
xce_nop_req(ep);
|
||||
|
||||
/* Set state-aware dispatcher for subsequent messages on this endpoint */
|
||||
ep->msg_handler = peer_ctrl_msg_handler;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -183,6 +183,8 @@ typedef struct {
|
|||
/* ── New fields added by raft-library spec (Task 6.2) ──────── */
|
||||
cluster_t *raft_c; /* cluster_t* set by ha_start_election_async; used by raft callbacks */
|
||||
void *raft_election_timeout_ctx; /* election_timeout_ctx_t* for cancel (task 6.2) */
|
||||
/* ── New fields added by ha-heartbeat-liveness spec (Task 1.1) ── */
|
||||
time_t last_activity; /* CLOCK_MONOTONIC seconds of last primary message; 0 = none yet */
|
||||
} ha_state_t;
|
||||
|
||||
/* ── Public API ────────────────────────────────────────────── */
|
||||
|
|
|
|||
|
|
@ -589,6 +589,24 @@ static void test_dispatch_gate_committing_allows_repl_ops(void)
|
|||
printf("test_dispatch_gate_committing_allows_repl_ops: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Task 1.1 (ha-heartbeat-liveness): ha_init() zero-initializes last_activity ─
|
||||
*
|
||||
* Req 1.1, 1.3, 3.1: ha_state_t must carry a `time_t last_activity` field that
|
||||
* is zero-initialized by ha_init(). Later tasks (2.1) write to this field via
|
||||
* ha_on_heartbeat_received(); task 3.1 reads it for liveness polling.
|
||||
*/
|
||||
static void test_ha_init_last_activity_is_zero(void)
|
||||
{
|
||||
ha_state_t ha;
|
||||
ha_init(&ha, &g_dispatch_cfg, 0);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, (int)ha.last_activity,
|
||||
"ha_init must zero-initialize last_activity (liveness field)");
|
||||
|
||||
ha_fini(&ha);
|
||||
printf("test_ha_init_last_activity_is_zero: PASS\n");
|
||||
}
|
||||
|
||||
/* ── Task 5.1: myapp_run startup sequence ────────────────────────────────
|
||||
*
|
||||
* Req 1.1, 9.3, 9.4: myapp_run must call ha_start_election_async (non-blocking)
|
||||
|
|
@ -744,4 +762,7 @@ void test_ha_startup(void)
|
|||
RUN_TEST(test_nonblocking_single_node_election_sets_n_peers);
|
||||
RUN_TEST(test_nonblocking_single_node_election_enters_committing);
|
||||
RUN_TEST(test_nonblocking_election_returns_in_under_10ms);
|
||||
|
||||
/* Task 1.1 (ha-heartbeat-liveness) — ha_init zero-initializes last_activity */
|
||||
RUN_TEST(test_ha_init_last_activity_is_zero);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue