fix(scheduler): close post-failover rejoin detection gap (bug-1760)
An idle scheduler (nothing queued to publish) never noticed a dead controller connection: sched_needs_rejoin was only ever set as a side effect of an active client-library call, and an idle round makes none. After any controller failover with nothing already in flight, jobs submitted to the new primary stayed PEND indefinitely. Adds xcecli_is_connection_broken() (libxce) -- a pure, socket-free read of the transport's already-async-maintained dead/alive flag, set for free by the existing RDHUP disconnect handler with zero application calls. sched_check_pending_rejoin() now retries xcecli_join() on either the existing reactive flag or this new passive signal, so a healthy idle scheduler still makes zero extra network calls while a genuinely dead connection self-heals within one ~1s round -- live-verified against a 3-node HA cluster (rejoin fired ~300ms after new-primary election with nothing submitted in between). Also closes out bug-1735 (job memory/CPU usage reporting): confirmed already fixed on this branch since 2026-08-30, just never marked in the bug log. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
This commit is contained in:
parent
48587368ca
commit
88b0ac9937
9 changed files with 441 additions and 28 deletions
|
|
@ -1313,11 +1313,15 @@ sched_scheduler_decision_publish_retry_test = executable(
|
|||
scheduler_decision_publish_retry_test_srcs,
|
||||
include_directories:[incdir],
|
||||
dependencies: [scheduler_deps, unity_dep],
|
||||
c_args: ['-DXCE_MAIN_C_PATH="'
|
||||
+ (meson.current_source_dir() / '..' / 'src' / 'main.c')
|
||||
+ '"'],
|
||||
link_args: ['-Wl,--wrap=xcecli_jobdispatch',
|
||||
'-Wl,--wrap=xcecli_set_pend_reason',
|
||||
'-Wl,--wrap=xcecli_join',
|
||||
'-Wl,--wrap=xcecli_start',
|
||||
'-Wl,--wrap=xcecli_set_reconnect_handler'],
|
||||
'-Wl,--wrap=xcecli_set_reconnect_handler',
|
||||
'-Wl,--wrap=xcecli_is_connection_broken'],
|
||||
install: false)
|
||||
|
||||
# Worst-case decision/allocation backpressure test (component-failover-
|
||||
|
|
|
|||
|
|
@ -182,6 +182,28 @@ int __wrap_xcecli_join(xcecli_ctx_t c, int role, hostattrs_t* attrs)
|
|||
return rc;
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Test double: __wrap_xcecli_is_connection_broken (bug-1760)
|
||||
*
|
||||
* Defaults to false so every EXISTING test in this file (which never
|
||||
* touches this double) sees the connection as healthy, exactly matching
|
||||
* this file's pre-bug-1760 behavior -- sched_check_pending_rejoin()'s new
|
||||
* OR'd condition is a pure no-op unless a test explicitly sets this true.
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static bool g_connection_broken;
|
||||
|
||||
static void reset_connection_broken_double(void)
|
||||
{
|
||||
g_connection_broken = false;
|
||||
}
|
||||
|
||||
bool __wrap_xcecli_is_connection_broken(xcecli_ctx_t c)
|
||||
{
|
||||
(void)c;
|
||||
return g_connection_broken;
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* Test doubles: __wrap_xcecli_start / __wrap_xcecli_set_reconnect_handler
|
||||
* -- setup-only doubles for the real sched_init() call the rejoin-
|
||||
|
|
@ -215,6 +237,7 @@ void setUp(void)
|
|||
reset_dispatch_double();
|
||||
reset_pend_double();
|
||||
reset_join_double();
|
||||
reset_connection_broken_double();
|
||||
}
|
||||
|
||||
void tearDown(void) { return; }
|
||||
|
|
@ -562,6 +585,243 @@ static void test_rejoin_composition_retries_across_publish_calls(void)
|
|||
printf("test_rejoin_composition_retries_across_publish_calls: PASS\n");
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* bug-1760 — sched_check_pending_rejoin() must work as a standalone public
|
||||
* entry point, invocable with NO decision ever having been published.
|
||||
* This is the direct proof that the chicken-and-egg deadlock (a pending
|
||||
* rejoin that could only ever be checked as a side effect of a successful
|
||||
* publish) is closed: the function is now callable, and does the right
|
||||
* thing, entirely independent of publish activity. The actual production
|
||||
* call site added by this fix (xce_sched_driver_run_normal_mode_round(),
|
||||
* driver.c, called unconditionally once per ~1s round) is verified by
|
||||
* source-text scan below, since driving that function through a full round
|
||||
* would require live picker/allocator threads this fixture deliberately
|
||||
* never starts (see this file's header comment).
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static void test_check_pending_rejoin_invoked_directly_without_a_publish(void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
xce_client_config_t config;
|
||||
xce_cluster_config_t cluster_cfg;
|
||||
int rc;
|
||||
|
||||
/* Same fixture setup as test_rejoin_composition_retries_across_publish_
|
||||
* calls above: reach a real sched_init() far enough to register the
|
||||
* reconnect handler, force its own join to fail so no picker/allocator
|
||||
* thread starts, then simulate a transparent reconnect. */
|
||||
memset(&config, 0, sizeof(config));
|
||||
config.nmasters = 1;
|
||||
config.masters[0] = "127.0.0.1";
|
||||
config.master_ports[0] = 19999;
|
||||
config.from = "sched_pubretry_direct_test";
|
||||
config.key = "keepsecret$";
|
||||
|
||||
memset(&cluster_cfg, 0, sizeof(cluster_cfg));
|
||||
cluster_cfg.candidates_count = 1;
|
||||
strncpy(cluster_cfg.candidates[0].hostname, "127.0.0.1",
|
||||
XCE_MAXHOSTNAME_LEN - 1);
|
||||
cluster_cfg.candidates[0].port = 19999;
|
||||
strncpy(cluster_cfg.identity, "sched_pubretry_direct_test",
|
||||
sizeof(cluster_cfg.identity) - 1);
|
||||
|
||||
g_join_returns[0] = -EINVAL; /* sched_init()'s own join: fail on purpose */
|
||||
g_join_nreturns = 1;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
rc = sched_init(&sched, &config, &cluster_cfg, scheduler_req_handler);
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
rc < 0,
|
||||
"fixture: sched_init()'s own join was deliberately made to fail so "
|
||||
"no picker/allocator thread is spun up by this setup step");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
g_captured_reconnect_cb,
|
||||
"fixture: sched_init() must register the reconnect handler before "
|
||||
"its xcecli_join() call");
|
||||
|
||||
/* No decision has EVER been published at this point -- unlike every
|
||||
* other test in this file, sched_publish_dispatch()/
|
||||
* sched_publish_pend_reason() are never called here at all. */
|
||||
g_captured_reconnect_cb(sched.apictx, g_captured_reconnect_arg);
|
||||
|
||||
reset_join_double();
|
||||
g_join_returns[0] = -E_XCE_NOT_READY; /* first direct check: still fails */
|
||||
g_join_nreturns = 1;
|
||||
|
||||
sched_check_pending_rejoin(&sched);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
1, g_join_call_count,
|
||||
"sched_check_pending_rejoin() must be callable on its own and "
|
||||
"attempt xcecli_join() when a rejoin is pending, with zero "
|
||||
"decisions ever published");
|
||||
|
||||
g_join_returns[1] = 0; /* second direct check: succeeds */
|
||||
g_join_nreturns = 2;
|
||||
|
||||
sched_check_pending_rejoin(&sched);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
2, g_join_call_count,
|
||||
"a still-pending rejoin must be retried on the next direct call");
|
||||
|
||||
/* Flag must now be clear -- a further direct call must not re-attempt
|
||||
* the join. */
|
||||
sched_check_pending_rejoin(&sched);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
2, g_join_call_count,
|
||||
"once the rejoin has succeeded, a further direct call must be a "
|
||||
"no-op -- no third xcecli_join() call");
|
||||
|
||||
printf("test_check_pending_rejoin_invoked_directly_without_a_publish: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* bug-1760 — passive-detection condition: sched_check_pending_rejoin() must
|
||||
* attempt xcecli_join() when the transport is passively known-broken, even
|
||||
* when sched_needs_rejoin was NEVER set (the exact idle-scheduler scenario
|
||||
* a reactive-only check cannot detect on its own) -- and must make ZERO
|
||||
* calls when neither condition holds, so a healthy idle scheduler never
|
||||
* probes its connection. Uses a minimal zeroed fixture (no sched_init()
|
||||
* needed): sched_check_pending_rejoin() only reads sched->apictx to pass
|
||||
* verbatim into the wrapped xcecli_join()/xcecli_is_connection_broken()
|
||||
* doubles, neither of which dereferences it.
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static void test_check_pending_rejoin_triggers_on_passive_broken_signal(void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
|
||||
g_connection_broken = true;
|
||||
g_join_returns[0] = 0;
|
||||
g_join_nreturns = 1;
|
||||
|
||||
sched_check_pending_rejoin(&sched);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
1, g_join_call_count,
|
||||
"a passively-detected broken connection must trigger xcecli_join() "
|
||||
"even when sched_needs_rejoin was never set -- this closes the "
|
||||
"idle-scheduler chicken-and-egg gap a reactive-only check cannot");
|
||||
|
||||
printf("test_check_pending_rejoin_triggers_on_passive_broken_signal: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
static void test_check_pending_rejoin_no_op_when_healthy_and_no_flag(void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
/* g_connection_broken defaults to false (setUp); sched_needs_rejoin
|
||||
* (scheduler.c-private) is not touched by this test and is already
|
||||
* false by this point in the file's test order. */
|
||||
|
||||
sched_check_pending_rejoin(&sched);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, g_join_call_count,
|
||||
"a healthy, idle scheduler (no flag set, connection reports "
|
||||
"healthy) must make ZERO xcecli_join() calls -- this is the "
|
||||
"no-periodic-probe-to-a-healthy-connection guarantee");
|
||||
|
||||
printf("test_check_pending_rejoin_no_op_when_healthy_and_no_flag: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
/* ════════════════════════════════════════════════════════════════════════
|
||||
* bug-1760 — production wiring: main()'s round loop (scheduler/src/main.c)
|
||||
* must call sched_check_pending_rejoin() UNCONDITIONALLY on every
|
||||
* iteration, not only as a side effect of a successful decision publish.
|
||||
* Verified by source-text scan (same technique test_scheduler_decision_
|
||||
* redelivery.c's test_no_worker_resync_symbols_in_scheduler_publish_path
|
||||
* uses) rather than driving main() itself, which isn't linked into any
|
||||
* test binary (main.c is excluded from every scheduler_srcs test target
|
||||
* in meson.build -- see this file's own foreach-exclusion, shared by
|
||||
* every test target in this directory).
|
||||
* ════════════════════════════════════════════════════════════════════════ */
|
||||
|
||||
static char* read_whole_file(const char* path)
|
||||
{
|
||||
FILE* f = fopen(path, "r");
|
||||
long size;
|
||||
char* buf;
|
||||
size_t nread;
|
||||
|
||||
if (!f) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
fseek(f, 0, SEEK_END);
|
||||
size = ftell(f);
|
||||
fseek(f, 0, SEEK_SET);
|
||||
if (size < 0) {
|
||||
fclose(f);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
buf = (char*)malloc((size_t)size + 1);
|
||||
if (!buf) {
|
||||
fclose(f);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
nread = fread(buf, 1, (size_t)size, f);
|
||||
buf[nread] = '\0';
|
||||
fclose(f);
|
||||
return buf;
|
||||
}
|
||||
|
||||
static void test_main_loop_checks_pending_rejoin_unconditionally(void)
|
||||
{
|
||||
char* main_src = read_whole_file(XCE_MAIN_C_PATH);
|
||||
const char* func_sig = "int main(int argc, char* argv[])";
|
||||
const char* call = "sched_check_pending_rejoin(&sched);";
|
||||
const char* round_call
|
||||
= "xce_sched_driver_run_normal_mode_round(&sched);";
|
||||
const char* stop_check = "if (sched.core.stop)";
|
||||
char* func_start;
|
||||
char* call_pos;
|
||||
char* round_pos;
|
||||
char* stop_pos;
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(main_src,
|
||||
"could not read main.c source for scan");
|
||||
|
||||
func_start = strstr(main_src, func_sig);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(func_start,
|
||||
"main.c must still define int main(...)");
|
||||
|
||||
round_pos = strstr(func_start, round_call);
|
||||
stop_pos = strstr(func_start, stop_check);
|
||||
call_pos = strstr(func_start, call);
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
round_pos, "sanity: the round loop's xce_sched_driver_run_normal_"
|
||||
"mode_round(&sched) call must still exist");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
stop_pos, "sanity: the round loop's core.stop check must still "
|
||||
"exist");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
call_pos,
|
||||
"bug-1760: main()'s round loop must call "
|
||||
"sched_check_pending_rejoin(&sched) on every iteration -- without "
|
||||
"this, a pending rejoin after a controller failover can only ever "
|
||||
"be retried as a side effect of a successful decision publish, "
|
||||
"which may never happen (chicken-and-egg deadlock)");
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
(round_pos < call_pos) && (stop_pos < call_pos),
|
||||
"sched_check_pending_rejoin() must run AFTER the round's own "
|
||||
"stop-check (never called on the iteration that is about to "
|
||||
"break/shut down) but is placed unconditionally on every "
|
||||
"surviving iteration regardless of what that round scheduled");
|
||||
|
||||
free(main_src);
|
||||
printf("test_main_loop_checks_pending_rejoin_unconditionally: "
|
||||
"PASS\n");
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
|
|
@ -571,5 +831,9 @@ int main(void)
|
|||
RUN_TEST(test_extended_retry_success_logs_distinctly);
|
||||
RUN_TEST(test_first_attempt_success_does_not_log_extended_retry);
|
||||
RUN_TEST(test_rejoin_composition_retries_across_publish_calls);
|
||||
RUN_TEST(test_check_pending_rejoin_invoked_directly_without_a_publish);
|
||||
RUN_TEST(test_check_pending_rejoin_triggers_on_passive_broken_signal);
|
||||
RUN_TEST(test_check_pending_rejoin_no_op_when_healthy_and_no_flag);
|
||||
RUN_TEST(test_main_loop_checks_pending_rejoin_unconditionally);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue