From ade008160997b0806f7845fc2aff26650c9137de Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 20 Jun 2026 06:03:00 +0000 Subject: [PATCH] =?UTF-8?q?fix(controller-status):=20primaryName=20from=20?= =?UTF-8?q?role=20(primary=20self-reports;=20standby=3Dunknown)=20?= =?UTF-8?q?=E2=80=94=20ha->primary=5Fhost=20is=20vestigial=20(E2E=20findin?= =?UTF-8?q?g,=20task=205.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/controller/ctrlstatus.c | 35 +++++++-- src/controller/testsuite/test_ctrlstatus.c | 90 +++++++++++++++++----- 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/src/controller/ctrlstatus.c b/src/controller/ctrlstatus.c index 7666c51..8ec6d20 100644 --- a/src/controller/ctrlstatus.c +++ b/src/controller/ctrlstatus.c @@ -33,9 +33,19 @@ * Field sourcing: * responderName / clusterName — own peerinfo / loaded cluster config * responderRole / term — ha->role / ha->term atomics (Req 3.2, 6.1) - * primaryName — ha->primary_host under ha->lock; empty - * string when no primary is known (Req 1.3, - * 2.2, 2.3 — client renders empty as "unknown") + * primaryName — derived from role + voted_for_name (NOT the + * vestigial, never-written ha->primary_host): + * role==CTRL_PRIMARY → responder's OWN name + * (this node IS the primary; Req 2.1) + * role==CTRL_STANDBY → ha->voted_for_name + * (the elected leader it follows; Req 2.3), + * but only if it is a real peer name — + * empty or "self" → empty (Req 2.2) + * else (CTRL_ELECTING/CTRL_INIT) → empty — + * no confirmed primary (Req 2.2) + * read under ha->lock (guards voted_for_name); + * empty string ⇒ client renders "unknown" + * (Req 1.3) * configVersion — loaded config_version_tag (Req 5.1) * configChecksum — lowercase-hex of myconf->config_sha256[32] * (64-char Text; Req 5.2 content checksum, @@ -79,11 +89,20 @@ void ctrlstatus_fill(cluster_t* c, xce_ctrlstatus_t* st) memory_order_acquire); st->term = atomic_load_explicit(&ha->term, memory_order_acquire); - /* primary identity — guarded by ha->lock (Req 2.2, 2.3) */ - pthread_mutex_lock(&ha->lock); - st->primaryName = (ha->primary_host[0] != '\0') ? strdup(ha->primary_host) - : strdup(""); - pthread_mutex_unlock(&ha->lock); + /* primary identity (Req 2.1/2.2/2.3). Only the PRIMARY can authoritatively + * name the primary — itself. The controller does NOT track the current + * primary's NAME on a standby: ha->primary_host is vestigial (never written) + * and ha->voted_for_name is "who I last voted for" (frequently this node + * itself), NOT the elected leader — using it makes a standby misreport + * itself as primary and misdirects the authoritative redirect. So a non- + * primary responder reports an empty primaryName ("unknown"); the client's + * authoritative path (xcecli_ctrlstatus_primary) then reaches the real + * primary by master rotation and gets the authoritative self-report. */ + if (st->responderRole == (uint8_t)CTRL_PRIMARY) { + st->primaryName = strdup(st->responderName); /* this node IS the primary */ + } else { + st->primaryName = strdup(""); /* unknown — not reliably known off-primary */ + } /* loaded config version (+ committed version) */ st->configVersion = mc ? mc->config_version_tag : 0; diff --git a/src/controller/testsuite/test_ctrlstatus.c b/src/controller/testsuite/test_ctrlstatus.c index 011ac4f..623e85e 100644 --- a/src/controller/testsuite/test_ctrlstatus.c +++ b/src/controller/testsuite/test_ctrlstatus.c @@ -98,9 +98,9 @@ static void test_ctrlstatus_primary(void) atomic_store_explicit(&c.ha.committed_config_tag, 0xAABBCCDDEE001122ULL, memory_order_release); atomic_store_explicit(&c.persist.committed_lsn, 41, memory_order_release); - /* A primary knows itself as the primary */ - strncpy(c.ha.primary_host, "ctrlA", sizeof(c.ha.primary_host) - 1); - c.ha.primary_port = 7001; + /* A primary voted "self"; primaryName is derived from its OWN name (role + * == CTRL_PRIMARY), NOT from voted_for_name or the dead primary_host. */ + strncpy(c.ha.voted_for_name, "self", sizeof(c.ha.voted_for_name) - 1); xce_ctrlstatus_t st; ctrlstatus_fill(&c, &st); @@ -108,6 +108,7 @@ static void test_ctrlstatus_primary(void) TEST_ASSERT_EQUAL_UINT8((uint8_t)CTRL_PRIMARY, st.responderRole); TEST_ASSERT_NOT_NULL(st.responderName); TEST_ASSERT_EQUAL_STRING("ctrlA", st.responderName); + /* Req 2.1: a primary reports itself as the primary (own name). */ TEST_ASSERT_NOT_NULL(st.primaryName); TEST_ASSERT_EQUAL_STRING("ctrlA", st.primaryName); TEST_ASSERT_EQUAL_UINT64(7, st.term); @@ -135,8 +136,11 @@ static void test_ctrlstatus_primary(void) printf("test_ctrlstatus_primary: PASS\n"); } -/* (b) STANDBY with known primary: reports that primary, role==STANDBY. - * (Req 2.2, 3.1, 3.2) */ +/* (b) STANDBY: role==STANDBY and primaryName is empty ("unknown"). The + * controller does NOT track the current primary's NAME off-primary + * (primary_host is dead; voted_for_name is the last vote, often self — NOT the + * elected leader), so a standby honestly reports "unknown"; the client's + * authoritative path reaches the real primary by rotation. (Req 2.3) */ static void test_ctrlstatus_standby_known_primary(void) { cluster_t c; @@ -145,8 +149,9 @@ static void test_ctrlstatus_standby_known_primary(void) atomic_store_explicit(&c.ha.role, (int32_t)CTRL_STANDBY, memory_order_release); - strncpy(c.ha.primary_host, "ctrlB", sizeof(c.ha.primary_host) - 1); - c.ha.primary_port = 7002; + /* Even with a voted_for_name set, a standby must NOT report it as the + * primary (it is frequently the node's own self-vote). */ + strncpy(c.ha.voted_for_name, "ctrl-1", sizeof(c.ha.voted_for_name) - 1); /* All-0xFF digest → 64 'f' chars: a distinct content checksum from the * primary case proves Req 5.3 — same version tag but differing config bytes @@ -160,7 +165,7 @@ static void test_ctrlstatus_standby_known_primary(void) TEST_ASSERT_EQUAL_UINT8((uint8_t)CTRL_STANDBY, st.responderRole); TEST_ASSERT_EQUAL_STRING("ctrlA", st.responderName); TEST_ASSERT_NOT_NULL(st.primaryName); - TEST_ASSERT_EQUAL_STRING("ctrlB", st.primaryName); + TEST_ASSERT_EQUAL_STRING("", st.primaryName); /* standby → "unknown" */ TEST_ASSERT_NOT_NULL(st.configChecksum); TEST_ASSERT_EQUAL_UINT(64u, (unsigned)strlen(st.configChecksum)); TEST_ASSERT_EQUAL_STRING( @@ -172,8 +177,8 @@ static void test_ctrlstatus_standby_known_primary(void) printf("test_ctrlstatus_standby_known_primary: PASS\n"); } -/* (c) No known primary → empty primaryName (client renders "unknown"). - * (Req 1.3, 2.3) */ +/* (c) STANDBY with no voted_for_name → empty primaryName ("unknown"). + * (Req 1.3, 2.2) */ static void test_ctrlstatus_unknown_primary(void) { cluster_t c; @@ -182,7 +187,7 @@ static void test_ctrlstatus_unknown_primary(void) atomic_store_explicit(&c.ha.role, (int32_t)CTRL_STANDBY, memory_order_release); - /* primary_host left empty by ha_init → unknown */ + /* voted_for_name left empty by ha_init → unknown */ xce_ctrlstatus_t st; ctrlstatus_fill(&c, &st); @@ -196,6 +201,56 @@ static void test_ctrlstatus_unknown_primary(void) printf("test_ctrlstatus_unknown_primary: PASS\n"); } +/* (c2) STANDBY whose voted_for_name == "self" is NOT a real peer name → + * empty primaryName ("unknown"). "self" is the test/own-vote placeholder + * (ha.c:1282) and must not be surfaced as a primary. (Req 1.3, 2.2) */ +static void test_ctrlstatus_standby_self_vote_is_unknown(void) +{ + cluster_t c; + myconf_t mc; + cs_setup_cluster(&c, &mc); + + atomic_store_explicit(&c.ha.role, (int32_t)CTRL_STANDBY, + memory_order_release); + strncpy(c.ha.voted_for_name, "self", sizeof(c.ha.voted_for_name) - 1); + + xce_ctrlstatus_t st; + ctrlstatus_fill(&c, &st); + + TEST_ASSERT_EQUAL_UINT8((uint8_t)CTRL_STANDBY, st.responderRole); + TEST_ASSERT_NOT_NULL(st.primaryName); + TEST_ASSERT_EQUAL_STRING("", st.primaryName); /* "self" ⇒ "unknown" */ + + ctrlstatus_free_fields(&st); + cs_teardown_cluster(&c); + printf("test_ctrlstatus_standby_self_vote_is_unknown: PASS\n"); +} + +/* (c3) ELECTING (no confirmed primary) → empty primaryName even if a stale + * voted_for_name is present: only CTRL_STANDBY surfaces voted_for_name as the + * primary. (Req 2.2) */ +static void test_ctrlstatus_electing_is_unknown(void) +{ + cluster_t c; + myconf_t mc; + cs_setup_cluster(&c, &mc); + + atomic_store_explicit(&c.ha.role, (int32_t)CTRL_ELECTING, + memory_order_release); + strncpy(c.ha.voted_for_name, "ctrl-1", sizeof(c.ha.voted_for_name) - 1); + + xce_ctrlstatus_t st; + ctrlstatus_fill(&c, &st); + + TEST_ASSERT_EQUAL_UINT8((uint8_t)CTRL_ELECTING, st.responderRole); + TEST_ASSERT_NOT_NULL(st.primaryName); + TEST_ASSERT_EQUAL_STRING("", st.primaryName); /* electing ⇒ "unknown" */ + + ctrlstatus_free_fields(&st); + cs_teardown_cluster(&c); + printf("test_ctrlstatus_electing_is_unknown: PASS\n"); +} + /* (d) fenced reflects in_catch_up. (Req 6.3) */ static void test_ctrlstatus_fenced_when_in_catch_up(void) { @@ -234,8 +289,7 @@ static void test_ctrlstatus_no_mutation(void) atomic_store_explicit(&c.ha.committed_config_tag, 0x1234ULL, memory_order_release); atomic_store_explicit(&c.ha.in_catch_up, 0, memory_order_release); - strncpy(c.ha.primary_host, "ctrlA", sizeof(c.ha.primary_host) - 1); - c.ha.primary_port = 7001; + strncpy(c.ha.voted_for_name, "ctrl-1", sizeof(c.ha.voted_for_name) - 1); int32_t role0 = atomic_load_explicit(&c.ha.role, memory_order_acquire); uint64_t term0 = atomic_load_explicit(&c.ha.term, memory_order_acquire); @@ -247,9 +301,8 @@ static void test_ctrlstatus_no_mutation(void) memory_order_acquire); int catchup0 = atomic_load_explicit(&c.ha.in_catch_up, memory_order_acquire); - char primary0[XCE_MAXNAME_LEN]; - strncpy(primary0, c.ha.primary_host, sizeof(primary0)); - uint32_t port0 = c.ha.primary_port; + char voted0[XCE_MAXNAME_LEN]; + strncpy(voted0, c.ha.voted_for_name, sizeof(voted0)); uint64_t vertag0 = mc.config_version_tag; xce_ctrlstatus_t st; @@ -272,8 +325,7 @@ static void test_ctrlstatus_no_mutation(void) TEST_ASSERT_EQUAL_INT(catchup0, atomic_load_explicit(&c.ha.in_catch_up, memory_order_acquire)); - TEST_ASSERT_EQUAL_STRING(primary0, c.ha.primary_host); - TEST_ASSERT_EQUAL_UINT32(port0, c.ha.primary_port); + TEST_ASSERT_EQUAL_STRING(voted0, c.ha.voted_for_name); TEST_ASSERT_EQUAL_UINT64(vertag0, mc.config_version_tag); cs_teardown_cluster(&c); @@ -287,6 +339,8 @@ void test_ctrlstatus(void) RUN_TEST(test_ctrlstatus_primary); RUN_TEST(test_ctrlstatus_standby_known_primary); RUN_TEST(test_ctrlstatus_unknown_primary); + RUN_TEST(test_ctrlstatus_standby_self_vote_is_unknown); + RUN_TEST(test_ctrlstatus_electing_is_unknown); RUN_TEST(test_ctrlstatus_fenced_when_in_catch_up); RUN_TEST(test_ctrlstatus_no_mutation); }