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
This commit is contained in:
Rongsong Shen 2026-08-03 05:23:53 +00:00
parent 065025a3a6
commit 4d8d72142e
5 changed files with 409 additions and 2 deletions

View file

@ -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

View file

@ -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);

View file

@ -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);
}

View file

@ -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'])

View file

@ -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 <errno.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <tlog.h>
#include <unity/unity.h>
#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, &params);
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();
}