From bcb45e59d27b552a05ee1426c97f93b95946c0c1 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 1 Jun 2026 11:59:16 +0000 Subject: [PATCH] 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. --- .kiro/specs/ha-heartbeat-liveness/tasks.md | 103 +++++++++++++++++++++ src/controller/ha.c | 17 +++- src/controller/ha.h | 2 + src/controller/testsuite/test_ha_startup.c | 21 +++++ 4 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 .kiro/specs/ha-heartbeat-liveness/tasks.md diff --git a/.kiro/specs/ha-heartbeat-liveness/tasks.md b/.kiro/specs/ha-heartbeat-liveness/tasks.md new file mode 100644 index 00000000..ee3d89dc --- /dev/null +++ b/.kiro/specs/ha-heartbeat-liveness/tasks.md @@ -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_ diff --git a/src/controller/ha.c b/src/controller/ha.c index 7e819eb8..f5ab7b35 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -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; } diff --git a/src/controller/ha.h b/src/controller/ha.h index ab484a86..de3f3933 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -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 ────────────────────────────────────────────── */ diff --git a/src/controller/testsuite/test_ha_startup.c b/src/controller/testsuite/test_ha_startup.c index 4f47dc17..c5c7247d 100644 --- a/src/controller/testsuite/test_ha_startup.c +++ b/src/controller/testsuite/test_ha_startup.c @@ -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); }