feat(scheduler-sim-live-allocation): make plugin hook dispatch core-safe (task 1.2)

Re-types plugin_job_election()/plugin_prepare_allocation()/
plugin_commit_decision() from (void* arg, ...) to (xce_sched_core_t*
core, ...), iterating core->plugins directly instead of deriving it
from a normal-mode-only driver struct -- mirroring
plugin_begin_session()/plugin_end_session()'s already-completed
migration.

Adds a new, honestly-typed xce_sched_plugin_t.core field, set by both
plugin_register() (normal mode) and plugin_register_core() (simulation
mode, closing a gap left since bug-1825 where this path set nothing).
The existing plugin->sched field and its plugins/fcfs.c legacy
consumer are left untouched.

Necessarily updates the 4 existing normal-mode call sites (scheduler.c,
resource.c, schedobj.c) to pass &(...->core) -- a C signature migration
cannot keep the build green without touching every caller in the same
commit; each fix is a minimal argument wrap with no algorithm change,
matching design.md's own Implementation Notes for this component.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-18 11:48:08 +00:00
parent 89297dc825
commit e8866641a6
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
18 changed files with 403 additions and 34 deletions

View file

@ -91,6 +91,19 @@ sched_picker_registration_test = executable('sched_picker_registration_test',
dependencies: [scheduler_test_deps],
install: false)
# White-box test for the driver-agnostic plugin-hook dispatch functions
# (plugin_job_election/plugin_prepare_allocation/plugin_commit_decision) and
# the new xce_sched_plugin_t.core field (scheduler-sim-live-allocation spec,
# task 1.2, "Plugin Hook Dispatchers"). It #includes ../src/plugin.c
# directly (same white-box pattern as sched_picker_registration_test) so it
# exercises the real dispatch loops and the real plugin_register()/
# plugin_register_core() entry points, not a re-implementation of either.
sched_plugin_hook_dispatch_test = executable('sched_plugin_hook_dispatch_test',
['test_plugin_hook_dispatch.c'],
include_directories:[incdir],
dependencies: [scheduler_test_deps],
install: false)
# White-box test for the picker configuration fields added to
# xce_scheduler_t/xce_sched_queue_t/xce_sched_project_t (scheduler-core
# spec, task 1.3). It #includes ../src/schedobj.c directly (same white-box
@ -1734,6 +1747,8 @@ test('scheduler picker iface test', sched_picker_iface_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
test('scheduler picker registration test', sched_picker_registration_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
test('scheduler plugin hook dispatch test', sched_plugin_hook_dispatch_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
test('scheduler picker config fields test', sched_picker_config_fields_test, suite: 'scheduler',
env: ['ASAN_OPTIONS=detect_leaks=0'])
test('scheduler picker fcfs test', sched_picker_fcfs_test, suite: 'scheduler',

View file

@ -22,7 +22,7 @@ xce_sched_worker_t* sched_get_worker(xce_sched_core_t* s, char* n)
(void)n;
return NULL;
}
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
int sched_ctx_get_uncomit_host(xvec_t** v, xce_sched_ctx_t* c, int i) { (void)v; (void)c; (void)i; return 0; }
int plugin_queue_update(void* s, xce_sched_queue_t* q) { (void)s; (void)q; return 0; }
xce_sched_queue_t* sched_get_queue(xce_sched_core_t* s, char* n)

View file

@ -81,7 +81,7 @@ void kill_sched_resval(xce_val_t* val) { (void)val; }
* two stubs test_resource_accounting.c already establishes needing for the
* identical reason. Never actually invoked: this file's tests never call
* sched_try_alloc_on_hosts()/sched_try_alloc_on_host_(). */
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c)
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c)
{
(void)s; (void)c; return 0;
}

View file

@ -69,7 +69,7 @@
* entirely) nor sched_try_alloc_on_hosts()/sched_update_existing_*(). */
void plugin_unregister(void* arg) { (void)arg; }
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c)
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c)
{
(void)s;
(void)c;

View file

@ -77,7 +77,7 @@ int sched_add_project(xce_sched_core_t* s, xce_sched_project_t* p)
return 0;
}
void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; }
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
/* sched_get_queue: a tiny by-name lookup over a fixed test queue set. */
static xce_sched_queue_t* g_test_queues[8];

View file

@ -130,7 +130,7 @@ static int g_job_level_prepare_calls = 0;
* every registered plugin -- exactly one (parjob's own) in this file's
* fixtures. Mirrors plugin.c:591's own reset-before-fan-out step, same
* rationale as test_seqjob_allocator.c's identical stub. */
int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx)
int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx)
{
xce_sched_plugin_t fake_plugin;

View file

@ -81,7 +81,7 @@ int sched_reconcile_job_resource_usage(xce_sched_core_t* c,
(void)c; (void)a; (void)r; return 0;
}
void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; }
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; }
void plugin_unregister(void* arg) { (void)arg; }
void setUp(void) { return; }

View file

@ -0,0 +1,293 @@
/*
* test_plugin_hook_dispatch.c - white-box tests for the driver-agnostic
* plugin-hook dispatch functions (scheduler-sim-live-allocation spec, task
* 1.2, "Plugin Hook Dispatchers" component).
*
* RED (before this task): plugin_job_election()/plugin_prepare_allocation()/
* plugin_commit_decision() took a `void*` derived from a normal-mode-only
* xce_scheduler_t*, and xce_sched_plugin_t had no `core` field at all -- this
* file fails to compile (`xce_sched_plugin_t` has no member named `core`).
* GREEN: all three dispatchers take `xce_sched_core_t* core` directly and
* iterate `core->plugins` (mirroring plugin_begin_session()/
* plugin_end_session()'s own already-completed migration exactly);
* xce_sched_plugin_t.core is set by BOTH plugin_register() (normal mode) and
* plugin_register_core() (simulation mode).
*
* #include's the real plugin.c (white-box, same convention as
* test_plugin_picker_registration.c) so this test exercises the real
* dispatch loops and the real registration entry points, not a
* re-implementation of either.
*
* Fixture shape: this file never needs a real xce_scheduler_t/
* xce_sched_sim_driver_t beyond their embedded `core` field (both structs
* embed `xce_sched_core_t core` as their first member -- scheduler.h,
* sim.h), so it builds a bare `core->plugins` kvmgr manually, same minimal
* pattern test_plugin_picker_registration.c's own fixture_init() already
* establishes, rather than pulling in core.c's full xce_sched_core_init()
* and its wider dependency graph.
*/
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unity/unity.h>
#include "kvmgr.h"
#include "scheduler.h"
#include "sim.h"
/* plugin_prepare_allocation()'s post-hook-loop limit-request fan-out calls
* sched_fill_limit_requests() (scheduler.c, not part of this white-box TU)
* -- link-satisfying stub only, same rationale/pattern as
* test_plugin_picker_registration.c's own copy: this file's mock plugins
* never populate real limit data, so a no-op body is sufficient. */
int sched_fill_limit_requests(xce_sched_alloc_req_t* alloc_req,
xlist_t* limits, xlist_t* rusages, int nhosts,
int nslots)
{
(void)alloc_req;
(void)limits;
(void)rusages;
(void)nhosts;
(void)nslots;
return 0;
}
#include "../src/plugin.c"
void setUp(void) { return; }
void tearDown(void) { return; }
/* ---------------------------------------------------------------------- */
/* Fixture: a bare core->plugins kvmgr -- same minimal manual setup */
/* test_plugin_picker_registration.c's own fixture_init() uses. Callable */
/* against the `core` field of EITHER a normal-mode-shaped xce_scheduler_t */
/* or a simulation-mode-shaped xce_sched_sim_driver_t -- both embed */
/* xce_sched_core_t as their first member, so `&sched.core`/`&sim.core` are */
/* the two "containers built either way" this file's tests exercise. */
/* ---------------------------------------------------------------------- */
static void core_fixture_init(xce_sched_core_t* core)
{
kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING,
.nslots = 16,
.delta = 8,
.datakiller = plugin_unregister };
memset(core, 0, sizeof(*core));
core->plugins = makeInstance(&xkvmgr_type, &params);
TEST_ASSERT_NOT_NULL(core->plugins);
}
/* ---------------------------------------------------------------------- */
/* Mock plugins: order-recording spies for each of the three hooks under */
/* test. `arg` is the plugin instance itself (the dispatch loop's own */
/* convention, unchanged by this task -- see plugin.h's hook typedefs). */
/* ---------------------------------------------------------------------- */
#define MAX_CALLS 8
static char g_call_order[MAX_CALLS][32];
static int g_call_count;
static void reset_calls(void)
{
g_call_count = 0;
memset(g_call_order, 0, sizeof(g_call_order));
}
static void record_call(const char* name)
{
if (g_call_count < MAX_CALLS) {
strncpy(g_call_order[g_call_count], name,
sizeof(g_call_order[0]) - 1);
}
g_call_count++;
}
static int mock_job_election_hook(void* arg, xce_sched_job_t* job)
{
xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg;
(void)job;
record_call(plugin->name);
return 0;
}
static int mock_prepare_allocation_hook(void* arg, xce_sched_ctx_t* ctx)
{
xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg;
(void)ctx;
record_call(plugin->name);
return 0;
}
static int mock_commit_hook(void* arg, xce_decision_t* decision)
{
xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg;
(void)decision;
record_call(plugin->name);
return 0;
}
static void make_mock_plugin(xce_sched_plugin_t* plugin, char* name)
{
memset(plugin, 0, sizeof(*plugin));
plugin->name = name;
plugin->hooks.job_election_hook
= (plugin_job_election_hook_t)mock_job_election_hook;
plugin->hooks.prepare_allocation_hook
= (plugin_prepare_allocation_hook_t)mock_prepare_allocation_hook;
plugin->hooks.commit_hook
= (plugin_commit_decision_hook_t)mock_commit_hook;
}
/* ---------------------------------------------------------------------- */
/* The dispatch contract: exactly one call per registered plugin, per */
/* dispatch function, in registration order -- unchanged by this task's */
/* signature migration (Observable in tasks.md task 1.2). Run once per */
/* container shape by the two thin wrappers below. */
/* ---------------------------------------------------------------------- */
static void run_dispatch_fanout_case(xce_sched_core_t* core)
{
xce_sched_plugin_t plugin_a;
xce_sched_plugin_t plugin_b;
xce_sched_job_t job;
xce_sched_obj_t obj;
xce_sched_ctx_t ctx;
xce_decision_t decision;
int rc;
core_fixture_init(core);
make_mock_plugin(&plugin_a, "plugin_a");
make_mock_plugin(&plugin_b, "plugin_b");
rc = plugin_register_core(core, &plugin_a);
TEST_ASSERT_TRUE(rc >= 0);
rc = plugin_register_core(core, &plugin_b);
TEST_ASSERT_TRUE(rc >= 0);
/* -- plugin_job_election(): job_election_hook fan-out -- */
memset(&job, 0, sizeof(job));
reset_calls();
rc = plugin_job_election(core, &job);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT(2, g_call_count);
TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]);
TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]);
/* -- plugin_prepare_allocation(): prepare_allocation_hook fan-out --
* obj->tag is deliberately XCE_RESREQ_COMPOUND (not PRIMARY) so the
* post-hook-loop limit-request step's PRIMARY-only rusages branch
* (obj->u.primary->resreq->rusages) is never dereferenced -- this test
* targets the hook fan-out itself, not the limit-request populate step
* (covered elsewhere, e.g. test_seqjob_allocator.c). */
memset(&obj, 0, sizeof(obj));
obj.tag = XCE_RESREQ_COMPOUND;
memset(&ctx, 0, sizeof(ctx));
ctx.schedobj = &obj;
xvec_idval_init(&(ctx.alloc_req.shared), 8);
xvec_idval_init(&(ctx.alloc_req.host), 8);
reset_calls();
rc = plugin_prepare_allocation(core, &ctx);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_EQUAL_INT(2, g_call_count);
TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]);
TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]);
xvec_idval_fini(&(ctx.alloc_req.shared));
xvec_idval_fini(&(ctx.alloc_req.host));
/* -- plugin_commit_decision(): commit_hook fan-out -- */
memset(&decision, 0, sizeof(decision));
reset_calls();
rc = plugin_commit_decision(core, &decision);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT(2, g_call_count);
TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]);
TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]);
}
/* Normal-mode-shaped container: xce_scheduler_t (`sched.core`). */
static void test_dispatch_fanout_normal_mode_shaped_container(void)
{
xce_scheduler_t sched;
memset(&sched, 0, sizeof(sched));
run_dispatch_fanout_case(&(sched.core));
}
/* Simulation-mode-shaped container: xce_sched_sim_driver_t (`sim.core`). */
static void test_dispatch_fanout_simulation_mode_shaped_container(void)
{
xce_sched_sim_driver_t sim;
memset(&sim, 0, sizeof(sim));
run_dispatch_fanout_case(&(sim.core));
}
/* ---------------------------------------------------------------------- */
/* xce_sched_plugin_t.core -- the new, honestly-typed core-state reference */
/* field. Set by BOTH registration entry points; the existing, differently- */
/* typed `sched` field must remain genuinely untouched by */
/* plugin_register_core() and keep meaning exactly what it means today for */
/* plugin_register() (still xce_scheduler_t*, still read by fcfs.c's */
/* legacy job_elect_fn path). */
/* ---------------------------------------------------------------------- */
/* Closes today's gap (bug-1825-adjacent): before this task,
* plugin_register_core() never set anything on plugin->core (the field did
* not exist), so a plugin registered via the simulation-mode entry point
* silently had no way to reach shared core state. */
static void test_plugin_register_core_sets_honest_core_reference(void)
{
xce_sched_core_t core;
xce_sched_plugin_t plugin;
int rc;
core_fixture_init(&core);
make_mock_plugin(&plugin, "core_field_probe_sim");
rc = plugin_register_core(&core, &plugin);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_NOT_NULL(plugin.core);
TEST_ASSERT_EQUAL_PTR(&core, plugin.core);
/* plugin_register_core() has no xce_scheduler_t* to store -- plugin->
* sched stays NULL, unchanged from today (plugin.h's own doc comment). */
TEST_ASSERT_NULL(plugin.sched);
}
static void test_plugin_register_sets_honest_core_reference(void)
{
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
int rc;
memset(&sched, 0, sizeof(sched));
core_fixture_init(&(sched.core));
make_mock_plugin(&plugin, "core_field_probe_normal");
rc = plugin_register(&sched, &plugin);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_NOT_NULL(plugin.core);
TEST_ASSERT_EQUAL_PTR(&(sched.core), plugin.core);
/* The existing, differently-typed `sched` field must remain genuinely
* untouched -- still xce_scheduler_t*, still set, still what fcfs.c's
* legacy job_elect_fn path reads. */
TEST_ASSERT_EQUAL_PTR(&sched, plugin.sched);
}
int main(void)
{
UNITY_BEGIN();
RUN_TEST(test_dispatch_fanout_normal_mode_shaped_container);
RUN_TEST(test_dispatch_fanout_simulation_mode_shaped_container);
RUN_TEST(test_plugin_register_core_sets_honest_core_reference);
RUN_TEST(test_plugin_register_sets_honest_core_reference);
return UNITY_END();
}

View file

@ -102,7 +102,7 @@ void plugin_unregister(void* arg) { (void)arg; }
* sched_try_alloc_on_host_(), only sched_charge_account()/
* sched_uncharge_account()/sched_commit_job_alloc(). Same stub test.c's
* own #include "../src/resource.c" already establishes needing. */
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c)
int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c)
{
(void)s;
(void)c;

View file

@ -81,7 +81,7 @@ void plugin_unregister(void* arg) { (void)arg; }
* the real production call sequence's two prepare_allocation() calls per
* host try (once at ctx->worker==NULL via sched_ctx_set_primary(), again
* per host at ctx->worker!=NULL), since xvec_idval_store() never dedups. */
int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx)
int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx)
{
xce_sched_plugin_t fake_plugin;

View file

@ -111,7 +111,7 @@ void plugin_unregister(void* arg) { (void)arg; }
* every registered plugin -- exactly one (seqjob's own) in this file's
* fixtures. See test_seqjob_allocator.c's identical stub for the full
* xvec_idval_fini()/init() reset-before-fan-out rationale. */
int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx)
int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx)
{
xce_sched_plugin_t fake_plugin;

View file

@ -130,7 +130,7 @@ void plugin_unregister(void* arg) { (void)arg; }
* test_sim_contention_parity.c is not this pair's precedent, since it only
* needs sched_ctx_get_uncomit_host itself (it gets plugin_prepare_allocation
* for real via its own #include "../src/plugin.c"). */
int plugin_prepare_allocation(void* arg, xce_sched_ctx_t* ctx)
int plugin_prepare_allocation(xce_sched_core_t* arg, xce_sched_ctx_t* ctx)
{
(void)arg;
(void)ctx;