myxce/src/scheduler/testsuite/test_config_push_deferred.c
Rongsong Shen 3e873f0a66
fix(scheduler): a re-applied config updates queues in place and frees the config it replaces (bug-1886)
sched_init_queues() built a new xce_sched_queue_t for every configured
queue on each config apply; for an existing name the add failed, and the
unregistered queue was handed to the picker (which then tracked it
instead of the registered one) and leaked, while the registered queue
kept its old settings. It now updates the registered queue in place, as
QUEUE_UPDATE's core_upsert_queue() does.

sched_init_config() and the core's CONFIG_PUSH replaced core->config
without freeing the previous one. The core owns its config, and nothing
points into a replaced one (queues, resources, groups and limits built
from it are copies), so it is freed once the new one is applied.

Test: test_config_reapply_updates_queue_in_place_and_frees_old RED before
(priority stayed 1), GREEN after.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-29 10:03:04 +00:00

335 lines
12 KiB
C

/*
* test_config_push_deferred.c - bug-1846 part 2 regression: a config push
* that arrives while a scheduling session is in progress must not end that
* session early.
*
* The controller answers every scheduler join (on_scheduler_join(),
* controller/sched.c) by re-pushing config, workers and jobs. The scheduler
* re-joins from sched_check_pending_rejoin(), which sched_publish_retry_loop()
* calls after a successful publish -- i.e. from inside the driver's decision
* drain loop, mid-round. handler.c used to apply the config push right there
* on the handler thread: sched_init_config() -> sched_init_queues() ->
* builtin_fcfs on_queue_update(), which nulls the picker's cursor when it
* re-inserts the queue the cursor is resting on. next_job() then reports
* "no more candidates" and the round ends with the rest of the queue never
* considered. Captured in scheduler-replay-20260923-085356.replay as 9
* rounds cut off after 11-12 decisions, each followed by the rejoin's
* workersPush + full 50-job jobsPush (the config push itself was never
* recorded, since it bypassed the pending-update queue).
*
* This fixture puts the real fcfs picker mid-session, delivers a config
* push through the real scheduler_req_handler(), and requires the session
* to keep walking the queue; the config must then take effect when the
* driver applies it between rounds.
*/
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <unity/unity.h>
#if defined(__SANITIZE_ADDRESS__)
#include <sanitizer/asan_interface.h>
#endif
#include "controller.capnp.h"
#include "plugin.h"
#include "resobj.h"
#include "sched/types.h"
#include "scheduler.h"
#include "xstring.h"
/* handler.c's request entry point -- no header declares it (main.c passes
* it to sched_init() by name); "extern declared where consumed", the same
* idiom test_sim_contention_limit_driver_parity.c uses. */
extern int scheduler_req_handler(xce_request_t* request, void* args);
#define QUEUE_NAME "normal"
#define TEST_RESREQ ((char*)"select[type==any] rusage[mem=1]")
#define JOBA_ID ((uint64_t)518471ULL)
#define JOBB_ID ((uint64_t)518472ULL)
#define JOBC_ID ((uint64_t)518473ULL)
void setUp(void) { return; }
void tearDown(void) { return; }
static void register_slots_resource(var resmgr)
{
xce_resource_t res;
memset(&res, 0, sizeof(res));
res.name = strdup("slots");
res.valtype = XCE_VAL_NUMERIC;
res.ordtype = XCE_ORD_DECR;
res.flags = XCE_RESF_LIMIT | XCE_RESF_CONSUMABLE | XCE_RESF_RELEASE
| XCE_RESF_DEFAULT_HOST;
TEST_ASSERT_NOT_NULL(makeResource(resmgr, &res));
}
static void push_job(xce_scheduler_t* sched, xce_sched_plugin_t* picker,
uint64_t jobid, uint64_t queue_time)
{
job_submit_data_t submit;
xce_job_t j;
xce_sched_job_t* job;
int rc;
memset(&submit, 0, sizeof(submit));
submit.queue = (char*)QUEUE_NAME;
submit.user = (char*)"cfgpush_user";
submit.project_name = (char*)"default";
submit.resreq = TEST_RESREQ;
submit.mintasks = 1;
submit.maxtasks = 1;
memset(&j, 0, sizeof(j));
j.jobid = jobid;
j.state = JOB_STAT_PEND;
j.sub_time = queue_time;
j.queue_time = queue_time;
j.submit = &submit;
job = make_sched_job(sched, &j);
TEST_ASSERT_NOT_NULL(job);
rc = sched_add_job(&(sched->core), job);
TEST_ASSERT_TRUE(rc >= 0);
picker->picker_iface->on_job_push(picker, job);
}
/* The same shape on_scheduler_join()'s xcesched_push_configs() sends: a
* Conf4Sched naming the queue the picker is already walking. */
static void deliver_config_push(xce_scheduler_t* sched)
{
xce_queue_t* q = (xce_queue_t*)calloc(1, sizeof(*q));
xce_queue_t** queues = (xce_queue_t**)calloc(1, sizeof(*queues));
xce_conf4sched_t* config = (xce_conf4sched_t*)calloc(1, sizeof(*config));
xce_configdata_t configdata;
xce_request_t request;
int rc;
TEST_ASSERT_NOT_NULL(q);
TEST_ASSERT_NOT_NULL(queues);
TEST_ASSERT_NOT_NULL(config);
q->queue = strdup(QUEUE_NAME);
queues[0] = q;
config->n_queues = 1;
config->queues = queues;
memset(&configdata, 0, sizeof(configdata));
configdata.data.conf4sched = config;
memset(&request, 0, sizeof(request));
request.opcode = OPCode_push;
request.body_which = Request_body_configpushmsg;
request.body.configpushmsg = &configdata;
rc = scheduler_req_handler(&request, sched);
TEST_ASSERT_TRUE(rc >= 0);
}
static xce_scheduler_t g_sched;
static void test_config_push_mid_session_does_not_end_round(void)
{
xce_sched_plugin_t* fcfs = NULL;
xce_sched_queue_t* queue;
xce_sched_job_t* job;
int configver_before;
int rc;
memset(&g_sched, 0, sizeof(g_sched));
rc = xce_sched_core_init(&(g_sched.core));
TEST_ASSERT_EQUAL_INT(0, rc);
register_slots_resource(g_sched.core.resmgr);
fcfs = builtin_fcfs_new_plugin();
TEST_ASSERT_NOT_NULL(fcfs);
rc = plugin_register(&g_sched, fcfs);
TEST_ASSERT_TRUE(rc >= 0);
g_sched.core.job_pick_plugin = (char*)"builtin_fcfs";
queue = (xce_sched_queue_t*)calloc(1, sizeof(*queue));
TEST_ASSERT_NOT_NULL(queue);
queue->name = makeString(QUEUE_NAME);
rc = sched_add_queue(&(g_sched.core), queue);
TEST_ASSERT_TRUE(rc >= 0);
fcfs->picker_iface->on_queue_update(fcfs, queue);
push_job(&g_sched, fcfs, JOBA_ID, 1000ULL);
push_job(&g_sched, fcfs, JOBB_ID, 1001ULL);
push_job(&g_sched, fcfs, JOBC_ID, 1002ULL);
/* Mid-session: the picker has handed out job A and rests on job B. */
rc = fcfs->picker_iface->begin_session(fcfs);
TEST_ASSERT_EQUAL_INT(0, rc);
job = fcfs->picker_iface->next_job(fcfs);
TEST_ASSERT_NOT_NULL(job);
TEST_ASSERT_EQUAL_UINT64(JOBA_ID, job->jobid);
configver_before = g_sched.core.configver;
/* A rejoin's config push lands now, on the handler thread. */
deliver_config_push(&g_sched);
job = fcfs->picker_iface->next_job(fcfs);
TEST_ASSERT_NOT_NULL_MESSAGE(
job, "a config push delivered mid-session must not end the round -- "
"job B was never considered");
TEST_ASSERT_EQUAL_UINT64(JOBB_ID, job->jobid);
job = fcfs->picker_iface->next_job(fcfs);
TEST_ASSERT_NOT_NULL(job);
TEST_ASSERT_EQUAL_UINT64(JOBC_ID, job->jobid);
TEST_ASSERT_NULL(fcfs->picker_iface->next_job(fcfs));
TEST_ASSERT_EQUAL_INT_MESSAGE(
configver_before, g_sched.core.configver,
"the pushed config must not be applied while the session runs");
fcfs->picker_iface->end_session(fcfs);
/* Between rounds the driver applies it -- exactly once. */
TEST_ASSERT_EQUAL_INT(1, sched_apply_pending_config(&g_sched));
TEST_ASSERT_EQUAL_INT_MESSAGE(configver_before + 1, g_sched.core.configver,
"the stashed config must take effect when "
"applied between rounds");
TEST_ASSERT_EQUAL_INT(0, sched_apply_pending_config(&g_sched));
TEST_ASSERT_EQUAL_INT(configver_before + 1, g_sched.core.configver);
printf("test_config_push_mid_session_does_not_end_round: PASS\n");
}
/* The same deferral makes the push recordable: it is now applied at a
* round boundary on the driver thread, exactly where the recorder writes
* the round's other controller events. Before, it bypassed the pending-
* update queue entirely and never reached a .replay file, so a replay
* spanning a rejoin (or a genuine config change) never saw it. */
static xce_scheduler_t g_rec_sched;
static void test_applied_config_push_is_recorded(void)
{
xce_queue_t* q = (xce_queue_t*)calloc(1, sizeof(*q));
xce_queue_t** queues = (xce_queue_t**)calloc(1, sizeof(*queues));
xce_conf4sched_t* config = (xce_conf4sched_t*)calloc(1, sizeof(*config));
FILE* f;
int rc;
memset(&g_rec_sched, 0, sizeof(g_rec_sched));
rc = xce_sched_core_init(&(g_rec_sched.core));
TEST_ASSERT_EQUAL_INT(0, rc);
f = tmpfile();
TEST_ASSERT_NOT_NULL(f);
g_rec_sched.recording = true;
g_rec_sched.record_file = f;
q->queue = strdup(QUEUE_NAME);
queues[0] = q;
config->n_queues = 1;
config->queues = queues;
TEST_ASSERT_EQUAL_INT(0, sched_stash_config(&g_rec_sched, config));
TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, g_rec_sched.record_event_count,
"stashing alone must not record");
TEST_ASSERT_EQUAL_INT(1, sched_apply_pending_config(&g_rec_sched));
TEST_ASSERT_EQUAL_UINT64_MESSAGE(
1, g_rec_sched.record_event_count,
"an applied config push must be written to the recording as one "
"configPush event");
TEST_ASSERT_TRUE(ftell(f) > 0);
g_rec_sched.recording = false;
g_rec_sched.record_file = NULL;
fclose(f);
printf("test_applied_config_push_is_recorded: PASS\n");
}
/* bug-1886: stashes and applies a config carrying one queue, QUEUE_NAME,
* at `priority`; returns the applied config. */
static xce_conf4sched_t* apply_queue_config(xce_scheduler_t* sched,
int32_t priority)
{
xce_queue_t* q = (xce_queue_t*)calloc(1, sizeof(*q));
xce_queue_t** queues = (xce_queue_t**)calloc(1, sizeof(*queues));
xce_conf4sched_t* config = (xce_conf4sched_t*)calloc(1, sizeof(*config));
TEST_ASSERT_NOT_NULL(q);
TEST_ASSERT_NOT_NULL(queues);
TEST_ASSERT_NOT_NULL(config);
q->queue = strdup(QUEUE_NAME);
q->priority = priority;
queues[0] = q;
config->n_queues = 1;
config->queues = queues;
TEST_ASSERT_EQUAL_INT(0, sched_stash_config(sched, config));
TEST_ASSERT_EQUAL_INT(1, sched_apply_pending_config(sched));
TEST_ASSERT_EQUAL_PTR(config, sched->core.config);
return config;
}
/*
* bug-1886: re-applying a config updates the registered queue in place
* (the rejected duplicate used to be built, handed to the picker and
* leaked, while the registered queue kept its old settings), and the
* config it replaces is freed.
*/
static xce_scheduler_t g_reapply_sched;
static void test_config_reapply_updates_queue_in_place_and_frees_old(void)
{
xce_sched_plugin_t* fcfs = NULL;
xce_sched_queue_t* q_first;
xce_sched_queue_t* q_second;
xce_conf4sched_t* first;
int rc;
memset(&g_reapply_sched, 0, sizeof(g_reapply_sched));
rc = xce_sched_core_init(&(g_reapply_sched.core));
TEST_ASSERT_EQUAL_INT(0, rc);
fcfs = builtin_fcfs_new_plugin();
TEST_ASSERT_NOT_NULL(fcfs);
rc = plugin_register(&g_reapply_sched, fcfs);
TEST_ASSERT_TRUE(rc >= 0);
g_reapply_sched.core.job_pick_plugin = (char*)"builtin_fcfs";
first = apply_queue_config(&g_reapply_sched, 1);
q_first = sched_get_queue(&(g_reapply_sched.core), QUEUE_NAME);
TEST_ASSERT_NOT_NULL(q_first);
TEST_ASSERT_EQUAL_INT(1, q_first->priority);
(void)apply_queue_config(&g_reapply_sched, 7);
q_second = sched_get_queue(&(g_reapply_sched.core), QUEUE_NAME);
TEST_ASSERT_EQUAL_PTR(q_first, q_second);
TEST_ASSERT_EQUAL_INT_MESSAGE(
7, q_second->priority,
"a re-applied config must update the registered queue in place");
#if defined(__SANITIZE_ADDRESS__)
TEST_ASSERT_TRUE_MESSAGE(__asan_address_is_poisoned(first),
"the replaced config must be freed");
#else
(void)first;
#endif
}
int main(void)
{
UNITY_BEGIN();
RUN_TEST(test_config_push_mid_session_does_not_end_round);
RUN_TEST(test_applied_config_push_is_recorded);
RUN_TEST(test_config_reapply_updates_queue_in_place_and_frees_old);
return UNITY_END();
}