From 4d8d72142e8ffed35c5548357c99bd7159ffcd2c Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 3 Aug 2026 05:23:53 +0000 Subject: [PATCH] feat(scheduler-core): register picker plugin category + job_elect_fn compat wrapper (task 1.2) plugin_register() now adapts an old-style job_elect_fn-only plugin to xce_job_picker_iface_t via legacy_picker_wrapper_iface (next_job() forwards to job_elect_fn; other hooks are safe no-ops covered elsewhere in the existing lifecycle), logging a one-time deprecation warning. A plugin that already sets picker_iface directly skips the wrapper and the warning. Reuses the existing plugin_register()/key_find() mechanism -- no parallel registration system. Requirements: 1.6, 2.5 --- .kiro/specs/scheduler-core/tasks.md | 2 +- src/scheduler/include/plugin.h | 17 +- src/scheduler/src/plugin.c | 128 +++++++++ src/scheduler/testsuite/meson.build | 15 ++ .../test_plugin_picker_registration.c | 249 ++++++++++++++++++ 5 files changed, 409 insertions(+), 2 deletions(-) create mode 100644 src/scheduler/testsuite/test_plugin_picker_registration.c diff --git a/.kiro/specs/scheduler-core/tasks.md b/.kiro/specs/scheduler-core/tasks.md index a07a9b8a..79a4088a 100644 --- a/.kiro/specs/scheduler-core/tasks.md +++ b/.kiro/specs/scheduler-core/tasks.md @@ -8,7 +8,7 @@ - Observable: the header compiles standalone and defines a complete function-pointer struct matching every hook category - _Requirements: 1.2, 1.5, 2.6_ -- [ ] 1.2 Register the picker plugin category and deprecate the old election hook +- [x] 1.2 Register the picker plugin category and deprecate the old election hook - Add a picker plugin registration category to the plugin registration table, parallel to existing plugin categories - Add a compatibility wrapper that adapts an old-style `job_elect_fn`-only plugin to the new interface, logging a deprecation warning at registration time - Observable: registering a plugin that implements only `job_elect_fn` succeeds via the wrapper and logs a deprecation warning; registering one that implements the new interface directly succeeds without a warning diff --git a/src/scheduler/include/plugin.h b/src/scheduler/include/plugin.h index cccc60c8..326c0976 100644 --- a/src/scheduler/include/plugin.h +++ b/src/scheduler/include/plugin.h @@ -53,6 +53,13 @@ typedef struct { plugin_job_rusage_hook_t rusage_hook; } plugin_hooks_t; +/* Forward declaration -- xce_job_picker_iface_t is fully defined below + * (Req 1 AC 2). Only needed here as a pointer field on xce_sched_plugin_t + * (picker plugin registration category, scheduler-core task 1.2); + * re-declared as the same typedef once the struct body is complete below + * -- same idiom as cluster_t across myconf.h/persist.h/ha.h/cluster_def.h. */ +typedef struct xce_job_picker_iface_s xce_job_picker_iface_t; + typedef struct { char* name; void* sched; @@ -65,6 +72,14 @@ typedef struct { plugin_job_allocate_fn_t job_alloc_fn; plugin_hooks_t hooks; plugin_data_t data; + /* Picker plugin registration category (task 1.2): non-NULL once this + * plugin participates in the xce_job_picker_iface_t contract -- either + * because it was authored against the new interface directly, or + * because plugin_register() adapted it from the deprecated job_elect_fn + * hook via the compatibility wrapper (Req 5 "Migration from old + * job_elect_fn"). Stays NULL for plugin categories that are neither + * (allocator/limit plugins never set job_elect_fn). */ + xce_job_picker_iface_t* picker_iface; } xce_sched_plugin_t; extern int plugin_register(void*, void*); @@ -175,7 +190,7 @@ typedef struct xce_sched_core_s xce_sched_core_t; * - next_job() returns NULL exactly when no ready job remains for the * current session. */ -typedef struct { +typedef struct xce_job_picker_iface_s { int (*init)(void* picker, xce_sched_core_t* core); void (*fini)(void* picker); int (*begin_session)(void* picker); diff --git a/src/scheduler/src/plugin.c b/src/scheduler/src/plugin.c index 9f37fa05..8e86d3cf 100644 --- a/src/scheduler/src/plugin.c +++ b/src/scheduler/src/plugin.c @@ -7,8 +7,123 @@ #include "plugin.h" #include "resreq.h" #include "scheduler.h" +#include "utils.h" #include "xlist.h" +/* --------------------------------------------------------------------- + * Picker plugin registration category (scheduler-core spec, task 1.2). + * + * xce_job_picker_iface_t (plugin.h) is the new job-selection contract + * (Req 1 AC 2). It registers and is looked up through the very same + * plugin_register()/key_find() mechanism every other plugin category + * uses -- not a parallel system (design.md "Existing plugin framework"). + * A plugin joins the picker category by setting xce_sched_plugin_t's + * picker_iface (new-style) or job_elect_fn (old-style, the interface this + * spec deprecates -- Req 5 "Migration from old job_elect_fn"). + * + * plugin_register() below adapts any old-style, job_elect_fn-only plugin + * to xce_job_picker_iface_t via legacy_picker_wrapper_iface and logs a + * one-time deprecation warning, so pickers written against either + * interface register -- and are found via key_find() -- identically. A + * plugin that already sets picker_iface is left untouched: no wrapper, no + * warning. Plugins that set neither (allocator/limit categories) are + * unaffected -- the check below only fires when job_elect_fn is non-NULL. + * --------------------------------------------------------------------- */ + +/* The only hook this wrapper gives real behavior to: forwards to the + * legacy job_elect_fn, which already accepts the plugin struct itself as + * its void* argument (see plugins/fcfs.c's fcfs_elect_job) -- so the + * wrapper treats xce_sched_plugin_t* as its "picker" instance handle, the + * only handle an old-style plugin has. void* -> xce_sched_job_t* is the + * same cast job_elect_fn's own callers already perform. */ +static xce_sched_job_t* legacy_picker_next_job(void* picker) +{ + xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)picker; + + if ((plugin == NULL) || (plugin->job_elect_fn == NULL)) { + return NULL; + } + + return (xce_sched_job_t*)plugin->job_elect_fn(plugin); +} + +/* init/fini/begin_session/end_session are safe no-ops, not dead wiring: + * plugin_register()/plugin_unregister() already invoke the legacy + * init_fn/fini_fn directly (below, and above in plugin_unregister()), and + * begin_session_fn/end_session_fn are already driven by + * plugin_begin_session()/plugin_end_session() (existing iteration over + * sched->plugins). Re-invoking them through the picker_iface hooks too + * would double-call plugin lifecycle callbacks. */ +static int legacy_picker_init(void* picker, xce_sched_core_t* core) +{ + (void)picker; + (void)core; + return 0; +} + +static void legacy_picker_fini(void* picker) { (void)picker; } + +static int legacy_picker_session_noop(void* picker) +{ + (void)picker; + return 0; +} + +/* on_dispatch/on_pend: no old-style equivalent -- job_elect_fn's caller + * removes elected jobs via the separate commit_hook path (see + * plugins/fcfs.c's fcfs_commit) -- safe no-ops so new driving code can + * call these unconditionally without a NULL check. */ +static int legacy_picker_jobid_noop(void* picker, uint64_t jobid) +{ + (void)picker; + (void)jobid; + return 0; +} + +static int legacy_picker_queue_update_noop(void* picker, + xce_sched_queue_t* queue) +{ + (void)picker; + (void)queue; + return 0; +} + +static int legacy_picker_job_noop(void* picker, xce_sched_job_t* job) +{ + (void)picker; + (void)job; + return 0; +} + +static int legacy_picker_job_state_change_noop(void* picker, + xce_sched_job_t* job, + uint32_t state) +{ + (void)picker; + (void)job; + (void)state; + return 0; +} + +/* Stateless -- every hook resolves the specific plugin from the "picker" + * argument it is called with (see legacy_picker_next_job), so a single + * shared instance safely serves every old-style plugin registered. */ +static xce_job_picker_iface_t legacy_picker_wrapper_iface = { + .init = legacy_picker_init, + .fini = legacy_picker_fini, + .begin_session = legacy_picker_session_noop, + .next_job = legacy_picker_next_job, + .end_session = legacy_picker_session_noop, + .on_dispatch = legacy_picker_jobid_noop, + .on_pend = legacy_picker_jobid_noop, + .on_queue_update = legacy_picker_queue_update_noop, + .on_job_push = legacy_picker_job_noop, + .on_job_state_change = legacy_picker_job_state_change_noop, + .on_job_update = legacy_picker_job_noop, + .on_config = NULL, + .on_job_rusage = NULL, +}; + int plugin_register(void* arg1, void* arg2) { xce_scheduler_t* sched = (xce_scheduler_t*)arg1; @@ -20,6 +135,19 @@ int plugin_register(void* arg1, void* arg2) } plugin->sched = sched; + + /* Picker plugin registration category: adapt an old-style, + * job_elect_fn-only plugin to xce_job_picker_iface_t and warn once, at + * registration time (Req 5 "Migration from old job_elect_fn"). A + * plugin that already provides picker_iface is left untouched. */ + if ((plugin->picker_iface == NULL) && (plugin->job_elect_fn != NULL)) { + plugin->picker_iface = &legacy_picker_wrapper_iface; + WARN("plugin '%s' selects jobs via the deprecated job_elect_fn " + "hook; migrate to xce_job_picker_iface_t (scheduler-core spec, " + "Req 5 'Migration from old job_elect_fn')", + plugin->name); + } + if (plugin->init_fn != NULL) { rc = plugin->init_fn(plugin); } diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 75fcb93a..0227c1ec 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -68,6 +68,19 @@ sched_picker_iface_test = executable('sched_picker_iface_test', c_args: ['-Werror=incompatible-pointer-types'], install: false) +# White-box test for the picker plugin registration category and the +# job_elect_fn compatibility wrapper (scheduler-core spec, task 1.2). It +# #includes ../src/plugin.c directly (same white-box pattern as +# sched_handlers_test/sched_job_array_test) so it exercises the real +# plugin_register() and the real legacy_picker_wrapper_iface, and needs a +# REAL sched->plugins kvmgr (same setup scheduler.c's sched_plugin_init() +# uses) to register plugins through it for real. +sched_picker_registration_test = executable('sched_picker_registration_test', + ['test_plugin_picker_registration.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + # Register as meson tests so `meson test --suite scheduler` runs them all. # detect_leaks=0 matches the suite convention (fixtures are not torn down). test('scheduler test', scheduler_test, suite: 'scheduler', @@ -82,3 +95,5 @@ test('scheduler job-array wire e2e test', sched_job_array_wire_e2e_test, suite: env: ['ASAN_OPTIONS=detect_leaks=0']) 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']) diff --git a/src/scheduler/testsuite/test_plugin_picker_registration.c b/src/scheduler/testsuite/test_plugin_picker_registration.c new file mode 100644 index 00000000..09bfe1e0 --- /dev/null +++ b/src/scheduler/testsuite/test_plugin_picker_registration.c @@ -0,0 +1,249 @@ +/* + * test_plugin_picker_registration.c - white-box tests for the picker + * plugin registration category and job_elect_fn compatibility wrapper + * added to plugin.c (scheduler-core spec, task 1.2). + * + * RED: before task 1.2's plugin.h/plugin.c changes, xce_sched_plugin_t had + * no picker_iface field and legacy_picker_wrapper_iface did not exist -- + * this file failed to compile. + * GREEN: plugin_register() adapts an old-style, job_elect_fn-only plugin + * to xce_job_picker_iface_t via a compatibility wrapper and logs a + * deprecation warning exactly once, at registration time (Req 5 + * "Migration from old job_elect_fn"); a plugin that already provides + * picker_iface registers as-is, with no warning; a plugin that provides + * neither (today's allocator/limit categories) is unaffected. + * + * #include's the real plugin.c (white-box, same convention as + * test_handlers.c / test_job_array_expand.c / test_job_array_wire_e2e.c) + * so this test exercises the real plugin_register() and the real + * legacy_picker_wrapper_iface -- not a re-implementation of either. + */ +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "kvmgr.h" +#include "scheduler.h" + +/* plugin.c's plugin_prepare_allocation() (unrelated to the picker + * registration category under test here) calls sched_fill_limit_requests, + * defined in scheduler.c -- not pulled into this white-box TU. Stubbed the + * same way test_handlers.c stubs sched_init_config/sched_get_job: this + * test never exercises plugin_prepare_allocation, so a no-op body is + * sufficient to satisfy the linker. */ +int sched_fill_limit_requests(xce_scheduler_t* sched, xvec_idval_t* req, + xlist_t* limits) +{ + (void)sched; + (void)req; + (void)limits; + return 0; +} + +#include "../src/plugin.c" + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* ---------------------------------------------------------------------- */ +/* Early-print log capture: same pattern as test_job_array_wire_e2e.c's + * wire_e2e_early_print_cb. No test binary in this suite calls tlog_init(), + * so WARN() (tlog_warn -> tlog(TLOG_WARN, ...)) routes through tlog's + * pre-init "early print" fallback, which tlog_reg_early_printf_callback() + * lets a test intercept synchronously on the calling thread -- no + * tlog_init()/tlog_exit() lifecycle, so it cannot disturb any other test + * sharing this process. + * ---------------------------------------------------------------------- */ + +static int g_warn_count = 0; + +static void picker_reg_early_print_cb(struct tlog_loginfo* loginfo, + const char* format, va_list ap) +{ + char buf[512]; + va_list ap_copy; + + if (loginfo == NULL || loginfo->level != TLOG_WARN) { + return; + } + + va_copy(ap_copy, ap); + vsnprintf(buf, sizeof(buf), format, ap_copy); + va_end(ap_copy); + + if (strstr(buf, "job_elect_fn") != NULL) { + g_warn_count++; + } +} + +static void start_log_capture(void) +{ + g_warn_count = 0; + tlog_set_early_printf(1, 1, 0); + tlog_reg_early_printf_callback(picker_reg_early_print_cb); +} + +static void stop_log_capture(void) { tlog_reg_early_printf_callback(NULL); } + +/* ---------------------------------------------------------------------- */ +/* Fixture: a real sched->plugins kvmgr -- same setup as scheduler.c's + * (static, so not reusable) sched_plugin_init(). Each test gets its own + * fresh instance; nothing is torn down (suite convention, see meson.build: + * ASAN_OPTIONS=detect_leaks=0). */ +/* ---------------------------------------------------------------------- */ + +static void fixture_init(xce_scheduler_t* sched) +{ + kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING, + .nslots = 16, + .delta = 8, + .datakiller = plugin_unregister }; + + memset(sched, 0, sizeof(*sched)); + sched->plugins = makeInstance(&xkvmgr_type, ¶ms); + TEST_ASSERT_NOT_NULL(sched->plugins); +} + +/* ---------------------------------------------------------------------- */ +/* Old-style plugin fixture: only job_elect_fn is set, no native + * picker_iface -- the same shape as today's plugins/fcfs.c (out of scope + * for this task; this is a minimal standalone stand-in so the test does + * not depend on fcfs.c's real threading behavior). */ +/* ---------------------------------------------------------------------- */ + +static xce_sched_job_t g_legacy_elected_job; +static int g_legacy_elect_calls = 0; + +static void* legacy_job_elect_fn(void* arg) +{ + TEST_ASSERT_NOT_NULL(arg); + g_legacy_elect_calls++; + + return (void*)&g_legacy_elected_job; +} + +static void legacy_plugin_init(xce_sched_plugin_t* plugin, char* name) +{ + memset(plugin, 0, sizeof(*plugin)); + plugin->name = name; + plugin->job_elect_fn = (plugin_job_election_fn_t)legacy_job_elect_fn; +} + +/* Req 5 "Migration from old job_elect_fn": an old-style, job_elect_fn-only + * plugin registers successfully via the compatibility wrapper and logs a + * deprecation warning exactly once, at registration time. */ +static void +test_register_legacy_job_elect_fn_only_plugin_uses_wrapper_and_warns(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + int rc; + + fixture_init(&sched); + legacy_plugin_init(&plugin, "test_legacy_picker"); + + start_log_capture(); + rc = plugin_register(&sched, &plugin); + stop_log_capture(); + + TEST_ASSERT_GREATER_OR_EQUAL(0, rc); + TEST_ASSERT_EQUAL_INT(1, g_warn_count); + TEST_ASSERT_NOT_NULL(plugin.picker_iface); + TEST_ASSERT_EQUAL_PTR(&legacy_picker_wrapper_iface, plugin.picker_iface); +} + +/* The wrapper must actually work: next_job() genuinely forwards to the old + * job_elect_fn, it does not merely register without crashing. */ +static void test_legacy_wrapper_next_job_calls_job_elect_fn(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_sched_job_t* got; + + fixture_init(&sched); + legacy_plugin_init(&plugin, "test_legacy_picker_2"); + g_legacy_elect_calls = 0; + + TEST_ASSERT_TRUE(plugin_register(&sched, &plugin) >= 0); + TEST_ASSERT_NOT_NULL(plugin.picker_iface); + TEST_ASSERT_NOT_NULL(plugin.picker_iface->next_job); + + got = plugin.picker_iface->next_job(&plugin); + + TEST_ASSERT_EQUAL_INT(1, g_legacy_elect_calls); + TEST_ASSERT_EQUAL_PTR(&g_legacy_elected_job, got); +} + +/* Req 1 AC 2 / Req 5: a plugin that already implements + * xce_job_picker_iface_t natively registers as-is -- no wrapper + * substitution, no deprecation warning. */ +static xce_sched_job_t* new_style_next_job_stub(void* picker) +{ + (void)picker; + return NULL; +} + +static xce_job_picker_iface_t g_new_style_iface = { + .next_job = new_style_next_job_stub, +}; + +static void +test_register_new_style_picker_plugin_skips_wrapper_and_does_not_warn(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + int rc; + + fixture_init(&sched); + memset(&plugin, 0, sizeof(plugin)); + plugin.name = "test_new_style_picker"; + plugin.picker_iface = &g_new_style_iface; + + start_log_capture(); + rc = plugin_register(&sched, &plugin); + stop_log_capture(); + + TEST_ASSERT_GREATER_OR_EQUAL(0, rc); + TEST_ASSERT_EQUAL_INT(0, g_warn_count); + TEST_ASSERT_EQUAL_PTR(&g_new_style_iface, plugin.picker_iface); +} + +/* Regression guard: a plugin that is neither category (no job_elect_fn, no + * picker_iface -- today's allocator/limit plugins) registers unaffected -- + * no wrapper substitution, no warning, picker_iface stays NULL. */ +static void test_register_non_picker_plugin_is_unaffected(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + int rc; + + fixture_init(&sched); + memset(&plugin, 0, sizeof(plugin)); + plugin.name = "test_non_picker_plugin"; + + start_log_capture(); + rc = plugin_register(&sched, &plugin); + stop_log_capture(); + + TEST_ASSERT_GREATER_OR_EQUAL(0, rc); + TEST_ASSERT_EQUAL_INT(0, g_warn_count); + TEST_ASSERT_NULL(plugin.picker_iface); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST( + test_register_legacy_job_elect_fn_only_plugin_uses_wrapper_and_warns); + RUN_TEST(test_legacy_wrapper_next_job_calls_job_elect_fn); + RUN_TEST( + test_register_new_style_picker_plugin_skips_wrapper_and_does_not_warn); + RUN_TEST(test_register_non_picker_plugin_is_unaffected); + return UNITY_END(); +}