From f4327ce4f639cc2cdb4f4f19239d3fc2615ce166 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 20 Jun 2026 11:27:46 +0000 Subject: [PATCH] feat(ha-heartbeat-committed-propagation): migrate last_known_committed_config_tag to _Atomic (task 1.1) Mirror the sibling last_known_committed_lsn: all writes release, reads acquire. Prereq for a heartbeat-receive writer + the ctrlstatus reader to interact safely (Req 2.3). Pure access migration, no behavior change; controller suite 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../specs/ha-heartbeat-committed-propagation/tasks.md | 2 +- src/controller/ctrlstatus.c | 10 ++++++---- src/controller/ha.h | 8 ++++---- src/controller/ha_convergence.c | 3 ++- src/controller/testsuite/test_ctrlstatus.c | 9 ++++++--- src/controller/testsuite/test_demotion_convergence.c | 2 +- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/.kiro/specs/ha-heartbeat-committed-propagation/tasks.md b/.kiro/specs/ha-heartbeat-committed-propagation/tasks.md index bcc739c..0d5cbbb 100644 --- a/.kiro/specs/ha-heartbeat-committed-propagation/tasks.md +++ b/.kiro/specs/ha-heartbeat-committed-propagation/tasks.md @@ -4,7 +4,7 @@ > The work runs along a single request path (atomic migration → libxce sender → primary stamp / standby consume → vote-floor verification → E2E), so tasks are sequential by design; no `(P)` markers. The two foundation tasks (1.1 atomic migration, 1.2 sender) touch independent files but are kept ordered for review simplicity. The HA-critical change is the standby consume + term-guard (task 2.2) and its vote-floor interaction (task 3.2). - [ ] 1. Foundation: atomic state + heartbeat sender -- [ ] 1.1 Migrate `last_known_committed_config_tag` to `_Atomic` +- [x] 1.1 Migrate `last_known_committed_config_tag` to `_Atomic` - Change `ha->last_known_committed_config_tag` from a plain `uint64_t` to `_Atomic uint64_t` (`src/controller/include/ha.h`) and update its existing writers — `cluster_apply_ctrlhello_reply` (`ha_convergence.c`) and the election safe-commit path (`ha.c`) — to `atomic_store_explicit(..., release)`, and any reader (incl. the controller-status `ctrlstatus_fill`) to `atomic_load_explicit(..., acquire)`. Pure type/access migration — no behavior change. - Observable: the controller suite builds clean and passes unchanged; every read/write of the field uses atomic accessors (no plain access remains). - _Requirements: 2.3_ diff --git a/src/controller/ctrlstatus.c b/src/controller/ctrlstatus.c index 5718621..cd17530 100644 --- a/src/controller/ctrlstatus.c +++ b/src/controller/ctrlstatus.c @@ -153,10 +153,12 @@ void ctrlstatus_fill(cluster_t* c, xce_ctrlstatus_t* st) } else { st->committedLsn = atomic_load_explicit(&ha->last_known_committed_lsn, memory_order_acquire); - /* last_known_committed_config_tag is a plain (non-atomic) uint64_t - * written under ha->lock; a best-effort aligned 8-byte read is correct - * here and must NOT take the lock on this read path. */ - st->committedConfigVersion = ha->last_known_committed_config_tag; + /* last_known_committed_config_tag is an _Atomic uint64_t; an acquire + * load mirrors last_known_committed_lsn above and must NOT take the + * lock on this read path. */ + st->committedConfigVersion = + atomic_load_explicit(&ha->last_known_committed_config_tag, + memory_order_acquire); } st->appliedLsn = atomic_load_explicit(&ha->applied_lsn, memory_order_acquire); diff --git a/src/controller/ha.h b/src/controller/ha.h index bba3e84..a98506e 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -169,7 +169,7 @@ typedef struct { void * _Atomic elect_ctx; /* Req 2: ha_elect_ctx_t* during election; NULL otherwise */ _Atomic uint64_t last_known_committed_lsn; /* Req 5: from primary heartbeats */ sem_t heartbeat_seen; /* Req 1: posted by heartbeat handler during CTRL_INIT */ - uint64_t last_known_committed_config_tag; /* Req 8: from primary heartbeats */ + _Atomic uint64_t last_known_committed_config_tag; /* Req 8: from primary heartbeats */ _Atomic uint64_t committed_config_tag; /* Req 8.1: set on quorum ACK */ /* ── New fields added by election-iomgr-refactor spec (Task 2.1) ── */ void *client_ssl_ctx; /* SSL_CTX* (TLS_client_method); created in myapp_run */ @@ -769,9 +769,9 @@ int handle_request_vote(ha_state_t *ha, * *out_tag = max(local_config_tag, atomic committed_config_tag) * *out_lsn = max(local_applied_lsn, atomic last_known_committed_lsn) * - * Uses acquire loads of the atomic learned-committed fields; never reads the - * non-atomic last_known_committed_config_tag (data-race safe). Pure aside from - * the atomic reads — no state mutation. + * Uses acquire loads of the atomic learned-committed fields; never reads + * last_known_committed_config_tag (a heartbeat-learned config tag, not the + * vote freshness floor). Pure aside from the atomic reads — no state mutation. */ void ha_vote_self_freshness(ha_state_t *ha, uint64_t local_config_tag, diff --git a/src/controller/ha_convergence.c b/src/controller/ha_convergence.c index 409cbb5..6dd239e 100644 --- a/src/controller/ha_convergence.c +++ b/src/controller/ha_convergence.c @@ -374,7 +374,8 @@ void cluster_apply_ctrlhello_reply(cluster_t *c, uint64_t committed_lsn, { atomic_store_explicit(&c->ha.last_known_committed_lsn, committed_lsn, memory_order_release); - c->ha.last_known_committed_config_tag = config_tag; + atomic_store_explicit(&c->ha.last_known_committed_config_tag, config_tag, + memory_order_release); INFO("HA: CtrlHelloReply from primary: committed_lsn=%" PRIu64 " config_tag=%" PRIu64, committed_lsn, config_tag); diff --git a/src/controller/testsuite/test_ctrlstatus.c b/src/controller/testsuite/test_ctrlstatus.c index 23015be..32dbf6b 100644 --- a/src/controller/testsuite/test_ctrlstatus.c +++ b/src/controller/testsuite/test_ctrlstatus.c @@ -361,7 +361,8 @@ static void test_ctrlstatus_standby_reports_learned_committed(void) /* learned from the primary's heartbeats */ atomic_store_explicit(&c.ha.last_known_committed_lsn, 36, memory_order_release); - c.ha.last_known_committed_config_tag = 0x00C0FFEE0000CAFEULL; + atomic_store_explicit(&c.ha.last_known_committed_config_tag, + 0x00C0FFEE0000CAFEULL, memory_order_release); xce_ctrlstatus_t st; ctrlstatus_fill(&c, &st); @@ -393,7 +394,8 @@ static void test_ctrlstatus_standby_prelearn_committed_zero(void) memory_order_release); atomic_store_explicit(&c.ha.last_known_committed_lsn, 0, memory_order_release); - c.ha.last_known_committed_config_tag = 0; + atomic_store_explicit(&c.ha.last_known_committed_config_tag, 0, + memory_order_release); /* even if the node-local primary marker had a stale value, off-primary it * must NOT be surfaced — prove the learned (zero) value wins. */ atomic_store_explicit(&c.persist.committed_lsn, 77, memory_order_release); @@ -429,7 +431,8 @@ static void test_ctrlstatus_primary_reports_own_committed(void) /* stale learned values must NOT be surfaced on a primary */ atomic_store_explicit(&c.ha.last_known_committed_lsn, 9, memory_order_release); - c.ha.last_known_committed_config_tag = 0x1111ULL; + atomic_store_explicit(&c.ha.last_known_committed_config_tag, 0x1111ULL, + memory_order_release); xce_ctrlstatus_t st; ctrlstatus_fill(&c, &st); diff --git a/src/controller/testsuite/test_demotion_convergence.c b/src/controller/testsuite/test_demotion_convergence.c index 8e99d4f..3af3848 100644 --- a/src/controller/testsuite/test_demotion_convergence.c +++ b/src/controller/testsuite/test_demotion_convergence.c @@ -724,7 +724,7 @@ static void test_ctrlhello_reply_intake_stores_and_reconciles(void) atomic_load(&g_dc.ha.last_known_committed_lsn), "CtrlHelloReply intake must store last_known_committed_lsn (existing)"); TEST_ASSERT_EQUAL_UINT64_MESSAGE(reported_tag, - g_dc.ha.last_known_committed_config_tag, + atomic_load(&g_dc.ha.last_known_committed_config_tag), "CtrlHelloReply intake must store the config tag (existing)"); /* Assert (intake integration): the intake also TRIGGERED reconciliation —