lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(component-failover-adoption): unify worker's dual connect paths into single pipeline

Browse source 
Replace worker's separate default/single-host and multi-candidate connect
branches with one path: synthesize-or-accept a candidate list, validate,
convert via xce_cluster_config_to_client_config(), connect once. Register
a flag-only rejoin callback (no I/O, no client-lib reentry) and remove the
liveness probe's bounded give-up so the library owns retry/recovery.
This commit is contained in:
Rongsong Shen 2026-08-11 05:42:48 +00:00
parent 86e442c9cd
commit ce9f734a38
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
6 changed files with 556 additions and 141 deletions
Download patch file Download diff file Expand all files Collapse all files

24
src/worker/testsuite/meson.build Normal file
View file

@ -0,0 +1,24 @@
# Worker unit test suite (component-failover-adoption, task 2.1). White-box
# tests #include the production .c file(s) under test directly (same pattern
# as scheduler/testsuite's test_handlers.c etc.), so they link the same
# dependency set the real xcecd/xcecd_worker executables use rather than a
# subset of xcecd_srcs -- see test_controller_connector.c's header comment
# for why controller_connector.c is not also listed in xcecd_srcs here (it
# would double-define every symbol in that file).
worker_test_incdir = include_directories('../include', '../../include', '.',
'../src', '../../lib')
worker_test_deps = [xcecd_worker_deps, unity_dep]
controller_connector_test = executable('controller_connector_test',
'test_controller_connector.c',
c_args: ['-DDBUS_MANAGER'],
include_directories: [worker_test_incdir],
dependencies: worker_test_deps,
install: false)
test('worker controller_connector test', controller_connector_test,
suite: 'worker',
timeout: 60,
env: ['ASAN_OPTIONS=detect_leaks=0'])

329
src/worker/testsuite/test_controller_connector.c Normal file
View file

@ -0,0 +1,329 @@
/*
* test_controller_connector.c — white-box tests for the worker's unified
* connect/rejoin pipeline (component-failover-adoption, task 2.1,
* boundary: WorkerControllerConnector).
*
* Pulls the real (mostly-static) connect logic in controller_connector.c
* into this TU (same white-box pattern as scheduler/testsuite/test_handlers.c
* #include-ing handler.c directly), so the tests exercise the actual
* production code, not a re-implementation of it. controller_connector.c's
* job-handling externs this file never exercises (thread_pool, uv_ipc,
* xcecd_worker_utils, xcecd_exdata) are link-satisfied by trivial stubs
* below, same rationale as test_handlers.c's stubs. The stub definitions
* are placed AFTER the #include so their parameter types (thread_pool_t,
* fork_task_t, xcecd_worker_t, res_limits_t) come from the real headers
* controller_connector.c already pulled in -- controller_connector.h's own
* `api_ctx_t` (an unused leftover type, only ever referenced from commented-
* out code) also means this file must NOT additionally include
* src/lib/client_internal.h: that header declares its OWN, differently-
* shaped `api_ctx_t`, and the two conflict in the same translation unit.
*
* Covers:
* - Req 1.5: no-candidate-list startup and a single-entry candidate list
* converge on the identical validated xce_cluster_config_t shape (same
* downstream connect log sequence, since those logs are driven entirely
* by that struct's contents) -- proving there is no separate single-host
* code path anymore.
* - Req 5.1/5.2: a real multi-candidate list passes through unmodified
* (no truncation/branch-specific mangling before the shared helper).
* - Req 5.1/5.2 (rejoin callback contract, design.md "Rejoin callback
* constraint"): the registered rejoin callback only sets a flag and
* returns immediately -- no I/O, no calling back into the client
* library.
* - Req 2.1-2.3: checking_connection()'s old bounded give-up
* (MAX_RETRY_COUNT=5 then handle_disconnect() tearing the worker down
* for good) is gone -- an outage far longer than that old bound must
* never permanently disable the worker, and a single probe must return
* fast (the library, not this probe, now owns retry/backoff).
*/
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unity/unity.h>
/* Pull the real connect/rejoin logic under test into this TU. */
#include "../src/controller_connector.c"
/* controller_connector.c declares `extern hostattrs_t host_attrs;` in its
* non-XRUN branch (this build); provide the definition normally supplied by
* xcecd.c, which this test does not link. */
hostattrs_t host_attrs = { 0 };
/* ── Link-satisfying stubs for externs controller_connector.c references
* but this file never exercises (job start/control/dispatch paths) ── */
int submit_fork_task(thread_pool_t* pool, fork_task_t* task)
{
(void)pool;
(void)task;
return 0;
}
xcecd_worker_t* find_worker_by_jobid(xcecd_worker_t* worker_list,
uint64_t jobid)
{
(void)worker_list;
(void)jobid;
return NULL;
}
bool parse_limits(const char* exdata, res_limits_t* limits)
{
(void)exdata;
(void)limits;
return true;
}
bool ipc_master_send_job_signal(uint64_t job_id, int signal, bool force)
{
(void)job_id;
(void)signal;
(void)force;
return true;
}
bool ipc_master_send_job_ckpt(uint64_t job_id, int kill, const char* method)
{
(void)job_id;
(void)kill;
(void)method;
return true;
}
void xcecd_process_command(void* arg) { (void)arg; }
/* ════════════════════════════════════════════════════════════════════════
* Fixtures
* ════════════════════════════════════════════════════════════════════════ */
void setUp(void)
{
memset(&g_cluster_cfg, 0, sizeof(g_cluster_cfg));
cc_ctx = NULL;
xcecli_ctx = NULL;
needs_rejoin = false;
}
void tearDown(void) { return; }
/* ════════════════════════════════════════════════════════════════════════
* Req 1.5 — unified candidate-list pipeline: no branch for "single host"
* ════════════════════════════════════════════════════════════════════════ */
static void test_build_cluster_config_no_peers_synthesizes_single_candidate(
void)
{
controller_connector_ctx_t fake_cc = { 0 };
fake_cc.remote_host = "localhost";
fake_cc.remote_port = 9990;
fake_cc.cluster_cfg.candidates_count = 0; /* no --peers given */
cc_ctx = &fake_cc;
int rc = build_cluster_config();
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT(1, g_cluster_cfg.candidates_count);
TEST_ASSERT_EQUAL_STRING("localhost", g_cluster_cfg.candidates[0].hostname);
TEST_ASSERT_EQUAL_INT(9990, g_cluster_cfg.candidates[0].port);
TEST_ASSERT_NOT_EQUAL(0,
g_cluster_cfg.identity[0]); /* validate() requires this */
printf(
"test_build_cluster_config_no_peers_synthesizes_single_candidate: "
"PASS\n");
}
/* The core Req 1.5 assertion: starting with NO --peers and starting with a
* SINGLE-ENTRY --peers list pointed at the exact same host/port must
* produce the identical validated candidate-list shape. Every log line in
* xcecd_join_cluster() is derived from g_cluster_cfg's contents, so
* structural equality here is exactly what "identical connect log
* sequence" (the task's observable-completion criterion) reduces to. */
static void
test_build_cluster_config_single_entry_peers_matches_synthesized_shape(void)
{
controller_connector_ctx_t fake_cc_no_peers = { 0 };
fake_cc_no_peers.remote_host = "localhost";
fake_cc_no_peers.remote_port = 9990;
cc_ctx = &fake_cc_no_peers;
TEST_ASSERT_EQUAL_INT(0, build_cluster_config());
xce_cluster_config_t synthesized = g_cluster_cfg;
controller_connector_ctx_t fake_cc_one_peer = { 0 };
fake_cc_one_peer.remote_host = "unused";
fake_cc_one_peer.remote_port = 1;
fake_cc_one_peer.cluster_cfg.candidates_count = 1;
strncpy(fake_cc_one_peer.cluster_cfg.candidates[0].hostname, "localhost",
XCE_MAXHOSTNAME_LEN - 1);
fake_cc_one_peer.cluster_cfg.candidates[0].port = 9990;
strncpy(fake_cc_one_peer.cluster_cfg.identity, "workertest", 255);
cc_ctx = &fake_cc_one_peer;
TEST_ASSERT_EQUAL_INT(0, build_cluster_config());
xce_cluster_config_t explicit_single = g_cluster_cfg;
TEST_ASSERT_EQUAL_INT(explicit_single.candidates_count,
synthesized.candidates_count);
TEST_ASSERT_EQUAL_STRING(explicit_single.candidates[0].hostname,
synthesized.candidates[0].hostname);
TEST_ASSERT_EQUAL_INT(explicit_single.candidates[0].port,
synthesized.candidates[0].port);
printf(
"test_build_cluster_config_single_entry_peers_matches_synthesized_"
"shape: PASS\n");
}
static void test_build_cluster_config_multi_candidate_passthrough(void)
{
controller_connector_ctx_t fake_cc = { 0 };
fake_cc.cluster_cfg.candidates_count = 3;
strncpy(fake_cc.cluster_cfg.candidates[0].hostname, "ctrl-a",
XCE_MAXHOSTNAME_LEN - 1);
fake_cc.cluster_cfg.candidates[0].port = 9991;
strncpy(fake_cc.cluster_cfg.candidates[1].hostname, "ctrl-b",
XCE_MAXHOSTNAME_LEN - 1);
fake_cc.cluster_cfg.candidates[1].port = 9992;
strncpy(fake_cc.cluster_cfg.candidates[2].hostname, "ctrl-c",
XCE_MAXHOSTNAME_LEN - 1);
fake_cc.cluster_cfg.candidates[2].port = 9993;
strncpy(fake_cc.cluster_cfg.identity, "workertest", 255);
cc_ctx = &fake_cc;
int rc = build_cluster_config();
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT(3, g_cluster_cfg.candidates_count);
TEST_ASSERT_EQUAL_STRING("ctrl-a", g_cluster_cfg.candidates[0].hostname);
TEST_ASSERT_EQUAL_INT(9991, g_cluster_cfg.candidates[0].port);
TEST_ASSERT_EQUAL_STRING("ctrl-b", g_cluster_cfg.candidates[1].hostname);
TEST_ASSERT_EQUAL_INT(9992, g_cluster_cfg.candidates[1].port);
TEST_ASSERT_EQUAL_STRING("ctrl-c", g_cluster_cfg.candidates[2].hostname);
TEST_ASSERT_EQUAL_INT(9993, g_cluster_cfg.candidates[2].port);
printf("test_build_cluster_config_multi_candidate_passthrough: PASS\n");
}
/* ════════════════════════════════════════════════════════════════════════
* Rejoin callback contract (design.md "Rejoin callback constraint",
* Req 5.1/5.2): flag-only, no I/O, returns immediately.
* ════════════════════════════════════════════════════════════════════════ */
static long elapsed_us(struct timespec* t0, struct timespec* t1)
{
return (t1->tv_sec - t0->tv_sec) * 1000000L
+ (t1->tv_nsec - t0->tv_nsec) / 1000L;
}
static void test_rejoin_callback_sets_flag_and_returns_immediately(void)
{
struct timespec t0, t1;
TEST_ASSERT_FALSE(needs_rejoin);
clock_gettime(CLOCK_MONOTONIC, &t0);
int rc = xcecd_on_reconnect((xcecli_ctx_t)0x1, NULL);
clock_gettime(CLOCK_MONOTONIC, &t1);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_TRUE(needs_rejoin);
TEST_ASSERT_LESS_THAN_INT(5000, elapsed_us(&t0, &t1)); /* < 5ms: no I/O */
printf("test_rejoin_callback_sets_flag_and_returns_immediately: PASS\n");
}
static void test_rejoin_callback_registered_via_public_api(void)
{
/* Register through the real public entry point the production code
* calls (xcecli_set_reconnect_handler) against a REAL ctx allocated by
* xcecli_ctx_init() -- which does local struct setup only, no socket
* I/O -- so this proves the registration call in xcecd_join_cluster()
* is wired correctly end to end (right function, right signature),
* not just that xcecd_on_reconnect() behaves correctly in isolation. */
xce_client_config_t cfg = { 0 };
xcecli_ctx_t ctx = NULL;
cfg.nmasters = 1;
cfg.masters[0] = "127.0.0.1";
cfg.master_ports[0] = 1;
cfg.from = "test";
cfg.key = "test";
TEST_ASSERT_EQUAL_INT(0, xcecli_ctx_init(&ctx, &cfg));
int src = xcecli_set_reconnect_handler(ctx, xcecd_on_reconnect, NULL);
TEST_ASSERT_EQUAL_INT(0, src);
xcecli_ctx_fini(ctx);
printf("test_rejoin_callback_registered_via_public_api: PASS\n");
}
/* ════════════════════════════════════════════════════════════════════════
* Give-up reconciliation (Req 2.1-2.3): checking_connection() must never
* permanently disable the worker; the library now owns retry/backoff.
* ════════════════════════════════════════════════════════════════════════ */
static void test_checking_connection_never_permanently_disables_worker(void)
{
controller_connector_ctx_t fake_cc = { 0 };
fake_cc.running = true;
cc_ctx = &fake_cc;
/* No connection at all -> xcecli_hello(NULL, ...) fails immediately
* with -EINVAL (checked before any I/O -- see client.c's xcecli_hello),
* so this loop is fast and deterministic without a real socket. */
xcecli_ctx = NULL;
struct timespec t0, t1;
clock_gettime(CLOCK_MONOTONIC, &t0);
/* Far more iterations than the OLD MAX_RETRY_COUNT (5) -- the old code
* tore the worker down for good (cc_ctx->running = false) once, inside
* a SINGLE checking_connection() call, after 5 consecutive failures. */
for (int i = 0; i < 25; i++) {
checking_connection();
TEST_ASSERT_TRUE_MESSAGE(
cc_ctx->running,
"checking_connection() must never permanently disable the "
"worker -- the client library now owns connection recovery");
}
clock_gettime(CLOCK_MONOTONIC, &t1);
long elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000L
+ (t1.tv_nsec - t0.tv_nsec) / 1000000L;
/* The OLD code's single checking_connection() call, against a
* persistently failing hello, looped 5 times with a 5s sleep between
* each -- >=20s for ONE call alone. 25 calls completing well under a
* second proves the bounded retry-then-sleep loop is gone, not merely
* that the give-up flag was removed. */
TEST_ASSERT_LESS_THAN_INT(1000, elapsed_ms);
printf(
"test_checking_connection_never_permanently_disables_worker: "
"PASS (elapsed_ms=%ld)\n",
elapsed_ms);
}
/* ════════════════════════════════════════════════════════════════════════
* main
* ════════════════════════════════════════════════════════════════════════ */
int main(void)
{
xce_init_xcecd_log("/tmp/test_controller_connector.log", 1024 * 1024, 2);
UNITY_BEGIN();
RUN_TEST(test_build_cluster_config_no_peers_synthesizes_single_candidate);
RUN_TEST(
test_build_cluster_config_single_entry_peers_matches_synthesized_shape);
RUN_TEST(test_build_cluster_config_multi_candidate_passthrough);
RUN_TEST(test_rejoin_callback_sets_flag_and_returns_immediately);
RUN_TEST(test_rejoin_callback_registered_via_public_api);
RUN_TEST(test_checking_connection_never_permanently_disables_worker);
return UNITY_END();
}