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

test(scheduler-core): controller-side admin opcode dispatch tests (task 10.2)

Browse source 
Closes the two genuine gaps beyond task 10.1's own coverage (standby and
disconnected-scheduler rejection were already tested):

- Forward timeout: adds an XCE_TESTING-only timeout-override seam
  (SCHEDADMIN_TIMEOUT_MS) in cluster.c, verified inert in production
  builds (-DXCE_TESTING absent from xce_controller's target and every
  project-wide arg; the non-test branch is a bare 5000L literal).
  test_schedadmin_forward_timeout drives the real 5s (shrunk to 200ms
  under test) pthread_cond_timedwait to genuine expiry and asserts the
  exact {ok=false, error="request timeout"} shape from AC 10.

- Round-trip coverage: two more outcome-branch tests
  (rpc_status_only, business_error) using the same real xce_encode/
  xce_decode functions production uses. A literal two-binary
  controller<->scheduler integration was judged out of scope (separate
  meson targets, incompatible fixtures) since task 9.4's own
  test_admin_request.c already white-box-drives the real scheduler
  handler rigorously.

Controller suite 3/3 (1093/1093 assertions, up from 1090), scheduler
26/26 unaffected, clang-format clean, zero diff-hygiene noise (git diff
matches git diff -w for cluster.c).
This commit is contained in:
Rongsong Shen 2026-08-06 04:19:58 +00:00
parent 045be6b3d6
commit 30ff995cac
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
3 changed files with 346 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

36
src/controller/cluster.c
View file

@ -309,6 +309,34 @@ static int schedadmin_signal(int status, sched_admin_reply_t* sar)
return 1;
}
#ifdef XCE_TESTING
/*
* schedadmin_set_timeout_ms_for_test / SCHEDADMIN_TIMEOUT_MS (task 10.2, Req
* 5.10 edge case "Scheduler disconnects mid-request"): the OPCode_schedAdmin
* case's forward-and-block-for-reply deadline is a fixed 5s in production
* (requirements.md AC 10: "await scheduler reply (timeout: 5s)"). Proving
* that deadline genuinely expires needs a REAL pthread_cond_timedwait
* timeout to fire -- not a shortcut that bypasses the wait -- but a real 5s
* wait per test run has no precedent in this suite (the longest real waits
* elsewhere are sub-second, e.g. DEFAULT_SYNC_TIMEOUT_MS=500ms in
* test_commit_resolve.c). This test-only override shrinks the deadline so
* the SAME wait-then-give-up code path below executes for real, just
* faster. Guarded by XCE_TESTING like myreq_handler_for_test below -- never
* compiled into production builds, where SCHEDADMIN_TIMEOUT_MS is a bare
* 5000 with zero indirection and the deadline math below folds to exactly
* the prior `deadline.tv_sec += 5` (5000 / 1000 == 5, 5000 % 1000 == 0).
*/
static long g_schedadmin_timeout_ms = 5000;
void schedadmin_set_timeout_ms_for_test(long ms)
{
g_schedadmin_timeout_ms = ms;
}
#define SCHEDADMIN_TIMEOUT_MS g_schedadmin_timeout_ms
#else
#define SCHEDADMIN_TIMEOUT_MS 5000L
#endif
static int myreq_handler(vtask_t* task)
{
vtask_data_t* data = (vtask_data_t*)task->data;
@ -869,8 +897,14 @@ static int myreq_handler(vtask_t* task)
}
struct timespec deadline;
long schedadmin_timeout_ms = SCHEDADMIN_TIMEOUT_MS;
clock_gettime(CLOCK_REALTIME, &deadline);
deadline.tv_sec += 5;
deadline.tv_sec += schedadmin_timeout_ms / 1000;
deadline.tv_nsec += (schedadmin_timeout_ms % 1000) * 1000000L;
if (deadline.tv_nsec >= 1000000000L) {
deadline.tv_sec += 1;
deadline.tv_nsec -= 1000000000L;
}
pthread_mutex_lock(&pending.mu);
while (!pending.done) {

309
src/controller/testsuite/test_sched_admin_dispatch.c
View file

@ -25,6 +25,62 @@
* built for the two Observables above made it straightforward. The full
* dispatch test matrix (malformed requests, concurrent/serialized
* round-trips, etc.) belongs to task 10.2.
*
* ── Task 10.2 additions (Req 5.10) ──────────────────────────────────────
*
* Task 10.2's literal scenario list is "standby rejection, disconnected-
* scheduler rejection, forward timeout, and a successful forward/reply
* round trip against the real scheduler-side handler from 9.4". The first
* two are the Observables above (already task 10.1's). This file adds:
*
* - test_schedadmin_forward_timeout: the genuinely-missing scenario --
* no existing test drove the 5s deadline to actually expire. Uses
* schedadmin_set_timeout_ms_for_test (cluster.c, XCE_TESTING-only) to
* shrink the real deadline so pthread_cond_timedwait genuinely times
* out on a fast schedule, rather than mocking the wait away or eating
* a real 5s per test run (no precedent for that in this suite -- see
* the seam's own doc comment in cluster.c for the survey).
*
* - test_schedadmin_round_trip_rpc_status_only /
* test_schedadmin_round_trip_business_error: task 10.1's round-trip
* test only exercises ONE of the three outcome branches in the
* schedAdmin case's post-wait dispatch (cluster.c: got_done+reply,
* got_done+no-reply, timeout) -- the got_done+reply "success" shape.
* These two cover the other real, previously-uncovered branch
* (got_done, no SchedAdminReply payload -- an RPC-level outcome) and a
* business-level error shape (ok=false, e.g. "already recording" --
* requirements.md's "Reply field semantics by action" error rows),
* proving reply.status stays 0 for a completed request that merely
* carries a domain error, never conflated with an RPC/timeout failure.
*
* ON "against the real scheduler-side handler from 9.4": task 9.4's own
* scheduler_req_handler()/sched_handle_admin_request() are proven for
* real, end-to-end, including sched_record_status()'s actual field
* construction, by scheduler/testsuite/test_admin_request.c (already
* landed with task 9.4) -- that file white-box #includes handler.c,
* recording.c, and their scheduler-internal dependencies (core.c,
* driver.c, snapshot*.c) directly into the SCHEDULER's own test binary
* and spies on xcecli_reply_schedadmin() to capture the real reply. This
* controller test binary (controller_unit_test) is a wholly separate
* meson target/executable with its own main() and its own global state
* (cluster_t/ha_state_t fixtures, not xce_scheduler_t) -- pulling
* scheduler/src/handler.c + recording.c (which itself needs core.h and
* the rest of xce_scheduler_t's dependency graph) into THIS binary to
* invoke scheduler_req_handler() literally would require the same kind
* of careful selective-source-linking test_admin_request.c's own header
* comment documents doing for the scheduler side, replicated cross-
* directory inside an already-large, HA-critical controller test target
* -- real risk (symbol/global collisions between two "whole daemon"
* white-box bundles sharing one process image) for no correctness gain,
* since 9.4's own suite already covers that ground rigorously. A genuine
* two-process E2E (real controller + real scheduler binaries, a real
* connection between them) is an E2E/soak-test concern, not this task's
* scope. So: this file proves the CONTROLLER's forward/relay mechanism
* against the real wire contract (xce_encode/xce_decode, the same
* functions production uses) and the exact reply shapes 9.4's real
* sched_record_status()/sched_handle_admin_request() are proven (by
* their own suite) to produce -- not a second, redundant proof that the
* scheduler-side handler itself is correct.
*/
#define _GNU_SOURCE
#include <errno.h>
@ -33,6 +89,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <unity/unity.h>
@ -50,6 +107,11 @@
* consumed (freed) by the call, never touch/free it afterward. */
extern int myreq_handler_for_test(endpoint_t* ep, io_packet_t* pkt);
/* Test-only override for the OPCode_schedAdmin case's forward-and-block
* deadline (cluster.c, XCE_TESTING-only, task 10.2) -- see its own doc
* comment in cluster.c for why this seam exists. */
extern void schedadmin_set_timeout_ms_for_test(long ms);
/* ── Fixture ──────────────────────────────────────────────────────────────
*
* The OPCode_schedAdmin dispatch case (and the ha_dispatch_gate primary/
@ -115,6 +177,16 @@ static void sa_decode_queued_reply(endpoint_t* ep, xce_request_t* out)
TEST_ASSERT_EQUAL_INT(0, rc);
}
/* Monotonic milliseconds -- for measuring that the forward-timeout test
* genuinely waited out its (shrunk) deadline rather than returning early.
* Same construction as test_commit_resolve.c's mono_ms_r(). */
static uint64_t sa_mono_ms(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
return (uint64_t)ts.tv_sec * 1000u + (uint64_t)ts.tv_nsec / 1000000u;
}
/* ── Observable 1: non-primary rejects immediately, no forwarding attempt ──
*
* ha_dispatch_gate (ha.c) denies OPCode_schedAdmin for every non-primary
@ -300,6 +372,240 @@ static void test_schedadmin_round_trip_success(void)
printf("test_schedadmin_round_trip_success: PASS\n");
}
/* ── Task 10.2: forward timeout ─────────────────────────────────────────────
*
* c->sched.ep points at a real (if unconnected) endpoint_t, exactly like the
* round-trip test above, so xce_schedadmin_req really forwards and the
* dispatch case really blocks on pthread_cond_timedwait -- but NO second
* thread ever delivers a reply. schedadmin_set_timeout_ms_for_test shrinks
* the deadline so the real wait-then-give-up path completes on a fast test
* schedule; the override is restored to the production default IMMEDIATELY
* after myreq_handler_for_test returns, before any TEST_ASSERT below can
* longjmp out on failure (Unity's assertion macros unwind via longjmp) --
* so a failing assertion in this test can never leave a later test in this
* binary running against a shrunk deadline. */
static void test_schedadmin_forward_timeout(void)
{
cluster_t* c = sa_make_cluster();
atomic_store(&c->ha.role, CTRL_PRIMARY);
endpoint_t sched_conn;
memset(&sched_conn, 0, sizeof(sched_conn));
c->sched.ep = &sched_conn;
endpoint_t caller_ep;
memset(&caller_ep, 0, sizeof(caller_ep));
caller_ep.owner = c;
io_packet_t* pkt = sa_build_request_pkt();
const long timeout_ms = 200;
schedadmin_set_timeout_ms_for_test(timeout_ms);
uint64_t t0 = sa_mono_ms();
int rc = myreq_handler_for_test(&caller_ep, pkt);
uint64_t took = sa_mono_ms() - t0;
schedadmin_set_timeout_ms_for_test(5000); /* restore before any assert */
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, rc,
"the dispatch case always replies -- it does not propagate "
"-ETIMEDOUT as its own return, even on a genuine timeout");
TEST_ASSERT_TRUE_MESSAGE(
took + 20 >= (uint64_t)timeout_ms,
"must not give up meaningfully before the (shrunk) deadline -- this "
"is what proves the real pthread_cond_timedwait deadline fired, not "
"a shortcut that skipped the wait");
TEST_ASSERT_TRUE_MESSAGE(
took < (uint64_t)timeout_ms + 2000,
"must give up within a bounded window after the deadline, not hang");
xce_request_t decoded;
sa_decode_queued_reply(&caller_ep, &decoded);
TEST_ASSERT_EQUAL_INT(Request_body_replymsg, decoded.body_which);
xce_reply_t* rr = decoded.body.replymsg;
TEST_ASSERT_NOT_NULL(rr);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, rr->status,
"a timeout is relayed as an ok-RPC SchedAdminReply carrying "
"error=\"request timeout\", per requirements.md AC 10 -- not a "
"negative RPC status");
TEST_ASSERT_EQUAL_INT(RequestReply_data_schedadminreplymsg, rr->data_which);
sched_admin_reply_t* got = rr->data.schedadminreplymsg;
TEST_ASSERT_NOT_NULL(got);
TEST_ASSERT_EQUAL_INT(0, got->ok);
TEST_ASSERT_EQUAL_STRING("request timeout", got->error);
xce_free_request(&decoded);
sa_teardown(c);
printf("test_schedadmin_forward_timeout: PASS\n");
}
/* ── Task 10.2: additional round-trip outcome branches ──────────────────────
*
* Shared "second thread plays the scheduler" helper, generalized from
* test_schedadmin_round_trip_success's sa_deliver_scheduler_reply above so
* both new tests below can drive either outcome (a SchedAdminReply payload,
* or a bare RPC status with none) through the SAME real
* schedadmin_signal()/demux path, without duplicating the thread/encode
* boilerplate. Task 10.1's own sa_deliver_scheduler_reply is left untouched. */
typedef struct {
cluster_t* c;
int status;
sched_admin_reply_t* payload; /* NULL => RequestReply_data_nodata */
} sa_deliver_arg_t;
static void* sa_deliver_reply_generic(void* argp)
{
sa_deliver_arg_t* arg = (sa_deliver_arg_t*)argp;
usleep(200000); /* same margin as sa_deliver_scheduler_reply above */
xce_reply_t rr;
memset(&rr, 0, sizeof(rr));
rr.opcode = OPCode_schedAdmin;
rr.status = arg->status;
if (arg->payload != NULL) {
rr.data_which = RequestReply_data_schedadminreplymsg;
rr.data.schedadminreplymsg = arg->payload;
} else {
rr.data_which = RequestReply_data_nodata;
}
xce_request_t req;
memset(&req, 0, sizeof(req));
req.opcode = OPCode_reply;
req.body_which = Request_body_replymsg;
req.body.replymsg = &rr;
io_packet_t* pkt = calloc(1, sizeof(*pkt));
TEST_ASSERT_NOT_NULL(pkt);
TEST_ASSERT_TRUE(xce_encode(&req, pkt) >= 0);
endpoint_t sched_reply_ep;
memset(&sched_reply_ep, 0, sizeof(sched_reply_ep));
sched_reply_ep.owner = arg->c;
myreq_handler_for_test(&sched_reply_ep, pkt);
return NULL;
}
/* Exercises the got_done && got_reply==NULL branch (cluster.c, OPCode_reply
* case): the scheduler's OPCode_reply can legitimately carry no
* SchedAdminReply payload (an RPC-level outcome) -- the controller must
* still reply promptly, relaying the bare status, not treat it as a
* timeout. Never exercised by task 10.1's round-trip test, which always
* supplies a payload. */
static void test_schedadmin_round_trip_rpc_status_only(void)
{
cluster_t* c = sa_make_cluster();
atomic_store(&c->ha.role, CTRL_PRIMARY);
endpoint_t sched_conn;
memset(&sched_conn, 0, sizeof(sched_conn));
c->sched.ep = &sched_conn;
sa_deliver_arg_t arg = { .c = c, .status = -EIO, .payload = NULL };
pthread_t tid;
TEST_ASSERT_EQUAL_INT(
0, pthread_create(&tid, NULL, sa_deliver_reply_generic, &arg));
endpoint_t caller_ep;
memset(&caller_ep, 0, sizeof(caller_ep));
caller_ep.owner = c;
io_packet_t* pkt = sa_build_request_pkt();
int rc = myreq_handler_for_test(&caller_ep, pkt);
pthread_join(tid, NULL);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, rc,
"the dispatch case always replies, even when the scheduler's "
"OPCode_reply carries no SchedAdminReply payload");
xce_request_t decoded;
sa_decode_queued_reply(&caller_ep, &decoded);
xce_reply_t* rr = decoded.body.replymsg;
TEST_ASSERT_NOT_NULL(rr);
TEST_ASSERT_EQUAL_INT_MESSAGE(
-EIO, rr->status,
"a payload-less scheduler reply relays its bare RPC status verbatim");
TEST_ASSERT_EQUAL_INT(RequestReply_data_nodata, rr->data_which);
xce_free_request(&decoded);
sa_teardown(c);
printf("test_schedadmin_round_trip_rpc_status_only: PASS\n");
}
/* A business-level error from the scheduler (ok=false, e.g. "already
* recording" -- requirements.md's "Reply field semantics by action" error
* rows) is a NORMAL completed round trip, not an RPC failure. Proves the
* controller relays that shape verbatim with reply.status == 0 -- a
* domain error is never conflated with an RPC/timeout error. */
static void test_schedadmin_round_trip_business_error(void)
{
cluster_t* c = sa_make_cluster();
atomic_store(&c->ha.role, CTRL_PRIMARY);
endpoint_t sched_conn;
memset(&sched_conn, 0, sizeof(sched_conn));
c->sched.ep = &sched_conn;
sched_admin_reply_t sreply;
memset(&sreply, 0, sizeof(sreply));
sreply.ok = 0;
sreply.error = "already recording";
sreply.recording = 1;
sreply.file = "/var/log/xce/scheduler-replay-x.replay";
sreply.eventCount = 7;
sreply.startedAt = 999;
sa_deliver_arg_t arg = { .c = c, .status = 0, .payload = &sreply };
pthread_t tid;
TEST_ASSERT_EQUAL_INT(
0, pthread_create(&tid, NULL, sa_deliver_reply_generic, &arg));
endpoint_t caller_ep;
memset(&caller_ep, 0, sizeof(caller_ep));
caller_ep.owner = c;
io_packet_t* pkt = sa_build_request_pkt();
int rc = myreq_handler_for_test(&caller_ep, pkt);
pthread_join(tid, NULL);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, rc,
"a business-level error from the scheduler is still a completed "
"round trip");
xce_request_t decoded;
sa_decode_queued_reply(&caller_ep, &decoded);
xce_reply_t* rr = decoded.body.replymsg;
TEST_ASSERT_NOT_NULL(rr);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, rr->status,
"ok=false is a business error, not an RPC/timeout error -- "
"reply.status must stay 0");
TEST_ASSERT_EQUAL_INT(RequestReply_data_schedadminreplymsg, rr->data_which);
sched_admin_reply_t* got = rr->data.schedadminreplymsg;
TEST_ASSERT_NOT_NULL(got);
TEST_ASSERT_EQUAL_INT(0, got->ok);
TEST_ASSERT_EQUAL_STRING("already recording", got->error);
TEST_ASSERT_EQUAL_INT(1, got->recording);
TEST_ASSERT_EQUAL_STRING("/var/log/xce/scheduler-replay-x.replay",
got->file);
TEST_ASSERT_EQUAL_UINT64(7, got->eventCount);
TEST_ASSERT_EQUAL_UINT64(999, got->startedAt);
xce_free_request(&decoded);
sa_teardown(c);
printf("test_schedadmin_round_trip_business_error: PASS\n");
}
/* ── Suite entry point ──────────────────────────────────────────────────── */
void test_sched_admin_dispatch(void)
{
@ -307,4 +613,7 @@ void test_sched_admin_dispatch(void)
RUN_TEST(test_schedadmin_rejects_when_not_primary);
RUN_TEST(test_schedadmin_rejects_when_not_connected);
RUN_TEST(test_schedadmin_round_trip_success);
RUN_TEST(test_schedadmin_forward_timeout);
RUN_TEST(test_schedadmin_round_trip_rpc_status_only);
RUN_TEST(test_schedadmin_round_trip_business_error);
}