fix(scheduler): mid-round rejoin config push truncated live rounds (bug-1846 part 2)
sched_publish_retry_loop() calls sched_check_pending_rejoin() after every successful publish -- inside the driver's decision-drain loop -- so after a client-library reconnect the scheduler re-joins mid-round. The controller answers every join (on_scheduler_join(), controller/sched.c) by re-pushing config, workers and jobs. handler.c applied the config push synchronously on the handler thread: sched_init_config() -> sched_init_queues() -> builtin_fcfs on_queue_update(), which nulls the picker cursor when it reinserts the queue the cursor rests on. next_job() then returned NULL and the round ended with the rest of the queue never considered. Evidence (scheduler-replay-20260923-085356.replay, temporary event-stream diagnostics, reverted): 9 recorded rounds end after 11-12 decisions; every one is followed by a workersPush + full 50-job jobsPush (the join's resync, reporting just-dispatched jobs back as PEND), every full-length dispatching round has none, and the config push never appears because it bypassed the pending-update queue the recorder writes from -- which is also why replay could not reproduce the truncation. Fix (defer to the round edge, same direction as bug-1813): handler.c now calls new sched_stash_config(), parking the Conf4Sched in new xce_scheduler_t.pending_config under pending_config_lock (an older unapplied push is freed). main.c's loop calls new sched_apply_pending_config() before each round, running the unchanged sched_init_config() strictly between sessions. sched_fini() frees a never-applied push. Tests: - test_handlers.c: test_config_push_is_stashed_not_applied_on_handler_thread -- RED (sched_init_config called on the handler thread), GREEN after. - new test_config_push_deferred.c (sched_config_push_deferred_test): real scheduler_req_handler() + real builtin_fcfs mid-session -- RED (next_job() returned NULL right after the push), GREEN (session walks the remaining jobs; configver unchanged until applied once between rounds). - test_admin_request.c / test_job_array_expand.c / test_job_array_wire_e2e.c #include handler.c directly and get a link stub for sched_stash_config(). Full scheduler suite 87/87 (asan). Not addressed: config pushes are still not written to a recording (a replay spanning a config change won't see it); the ~100s reconnect churn itself (see bug-1812/1813). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
77f047f2e2
commit
a9feded88e
11 changed files with 413 additions and 7 deletions
208
src/scheduler/testsuite/test_config_push_deferred.c
Normal file
208
src/scheduler/testsuite/test_config_push_deferred.c
Normal file
|
|
@ -0,0 +1,208 @@
|
|||
/*
|
||||
* 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 <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#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");
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_config_push_mid_session_does_not_end_round);
|
||||
return UNITY_END();
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue