diff --git a/.kiro/specs/scheduler-core/tasks.md b/.kiro/specs/scheduler-core/tasks.md index 79a4088a..3968a805 100644 --- a/.kiro/specs/scheduler-core/tasks.md +++ b/.kiro/specs/scheduler-core/tasks.md @@ -15,7 +15,7 @@ - _Requirements: 1.6, 2.5_ - _Depends: 1.1_ -- [ ] 1.3 (P) Add picker configuration fields +- [x] 1.3 (P) Add picker configuration fields - Add a global picker plugin name field to the scheduler, defaulting to `builtin_fcfs` when unset - Add a per-queue picker plugin override field, defaulting to NULL (inherit global) - Add a project-name field to the queue struct, and extend the project struct with a priority field (scaffolding only — no quota fields) @@ -349,3 +349,10 @@ - Observable: the soak run completes with a clean pass across all rounds, matching the zero-regression bar set by the Phase 1 and Phase 2 soak runs - _Requirements: 4.3_ - _Depends: 12.4, 10.6_ + +## Implementation Notes + +- Task 1.3 added `sched_set_default_job_pick_plugin()` (schedobj.c) but deliberately did NOT call it from `sched_init()` — task 2.4 owns replacing `scheduler.c:43`'s hardcoded `"xce_sched_fcfs"` literal and must also wire this defaulting call (or equivalent logic) into scheduler startup, or `sched->job_pick_plugin` stays NULL through a real boot. +- Background IDE diagnostics ("file not found: rtti.h", "unknown type name" for various xce_* types) have appeared consistently on every scheduler.h/plugin.h/types.h/schedobj.c change so far (tasks 1.1-1.3) and are confirmed stale clangd artifacts each time via independent real `meson compile`/`meson test` runs — not real build failures. Don't let these block a task; verify with the real build instead. +- Meson build dir is `src/xce.build`, not repo-root `xce.build` — build/test commands must run from `src/` (e.g. `cd src && nix develop . --command bash -c "meson test -C xce.build --suite scheduler"`). +- Scheduler testsuite convention: run test binaries directly with `ASAN_OPTIONS=detect_leaks=0` (fixtures aren't torn down, by design — matches existing `meson.build` test registration). diff --git a/src/scheduler/include/sched/types.h b/src/scheduler/include/sched/types.h index 8b32785e..7f793603 100644 --- a/src/scheduler/include/sched/types.h +++ b/src/scheduler/include/sched/types.h @@ -64,7 +64,12 @@ typedef struct { } xce_sched_user_t; typedef struct { - char* name; + char* name; + /* Req 2 AC (scheduler-core spec, task 1.3): inter-project ordering + * scaffolding for a future spec (project-resource-ownership). No + * quota/usage/hierarchy fields yet -- see design.md's "Out of Scope". + * Defaults to 0 (make_sched_project, schedobj.c). */ + int32_t priority; } xce_sched_project_t; typedef struct { @@ -75,6 +80,17 @@ typedef struct { int32_t accept_interval; int32_t sched_delay; char* resreq; + /* Req 2 AC 3 (scheduler-core spec, task 1.3): owning project name, + * establishing the queue->project relationship. Scaffolding only -- + * xce_queue_t (the raw config struct) has no project field yet, so + * make_sched_queue (schedobj.c) leaves this NULL; the picker plugin + * may use it internally for project-scoped iteration (v2). */ + char* project_name; + /* Req 2 AC 2: per-queue picker-plugin override. NULL means "inherit + * sched->job_pick_plugin" (the global default). Non-NULL means jobs in + * this queue use this picker instead -- how a picker plugin *uses* the + * override is the plugin's concern, not the core loop's (task 2.4+). */ + char* job_pick_plugin; } xce_sched_queue_t; typedef struct xce_sched_job_ { diff --git a/src/scheduler/include/scheduler.h b/src/scheduler/include/scheduler.h index 13ab9e08..34b0452c 100644 --- a/src/scheduler/include/scheduler.h +++ b/src/scheduler/include/scheduler.h @@ -91,6 +91,14 @@ typedef struct { uint64_t nextid; xce_conf4sched_t* config; uint64_t configver; + /* Req 2 AC 1 (scheduler-core spec, task 1.3): global picker plugin + * name. NULL until sched_set_default_job_pick_plugin() (schedobj.c) + * applies the "builtin_fcfs" default, or a future config-driven + * assignment sets it explicitly. This field only carries the *name* -- + * resolving it against the plugin registry (plugin_register/key_find) + * and failing startup loudly on an unregistered name (Req 2 AC 5) is + * task 2.4's job. */ + char* job_pick_plugin; var plugins; void* seqjob_allocator; void* parjob_allocator; @@ -125,6 +133,10 @@ extern int sched_init(xce_scheduler_t*, xce_client_config_t*, const xce_cluster_config_t*, xce_request_handler_t); extern void sched_fini(xce_scheduler_t*); extern int sched_init_config(xce_scheduler_t* sched, xce_conf4sched_t*); +/* Req 2 AC 1 (scheduler-core spec, task 1.3): applies the "builtin_fcfs" + * default to sched->job_pick_plugin when unset. See the field comment on + * xce_scheduler_t above -- this does NOT resolve/register the name. */ +extern void sched_set_default_job_pick_plugin(xce_scheduler_t*); extern int sched_update(xce_scheduler_t*); extern void sched_go(xce_scheduler_t*); extern void sched_begin_session(xce_scheduler_t*); diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 48fc10df..8380ca4d 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -237,6 +237,14 @@ xce_sched_queue_t* make_sched_queue(void* arg, xce_queue_t* q) queue->priority = q->priority; queue->accept_interval = q->accept_interval; queue->sched_delay = q->sched_delay; + /* Req 2 AC 2/3 (scheduler-core spec, task 1.3): xce_queue_t (`q`, the + * raw config struct) does not carry project_name/job_pick_plugin yet + * -- calloc() above already leaves both NULL (queue inherits the + * global picker; project ownership unset). Explicit here for + * documentation/testability; wiring these from real config is a later + * task. */ + queue->project_name = NULL; + queue->job_pick_plugin = NULL; return queue; } @@ -363,11 +371,44 @@ xce_sched_project_t* make_sched_project(void* arg, char* name) project->name = makeString(name); } + /* Req 2 AC (scheduler-core spec, task 1.3): scaffolding only -- no + * quota/hierarchy inputs exist yet, so priority defaults to 0 + * (calloc() above already zeroes it; explicit here for + * documentation/testability). */ + project->priority = 0; + sched_add_project(sched, project); return project; } +/* + * sched_set_default_job_pick_plugin - Req 2 AC 1 (scheduler-core spec, + * task 1.3). Applies the "builtin_fcfs" default to sched->job_pick_plugin + * when it is unset (NULL). Idempotent: a name already present (whether + * previously defaulted or set by a future config-driven path) is left + * untouched, so this is safe to call defensively. + * + * This function only assigns the *name* -- it never touches the plugin + * registry (sched->plugins) and never calls plugin_register()/key_find(). + * Resolving the configured name against a registered picker plugin, and + * failing scheduler startup loudly when the name is not registered + * (Req 2 AC 5), is task 2.4's job. Kept as a separate, standalone function + * (rather than inlined into sched_init()) so the default-value behavior is + * unit-testable without the network/plugin-registry dependencies + * sched_init() pulls in. + */ +void sched_set_default_job_pick_plugin(xce_scheduler_t* sched) +{ + if (sched == NULL) { + return; + } + + if (sched->job_pick_plugin == NULL) { + sched->job_pick_plugin = makeString("builtin_fcfs"); + } +} + static void host_uncommit_killer(void* obj) { xvec_t* v = (xvec_t*)obj; diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 0227c1ec..9a177bbd 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -81,6 +81,18 @@ sched_picker_registration_test = executable('sched_picker_registration_test', 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 +# pattern as sched_job_update_test) so it exercises the real +# make_sched_queue/make_sched_project/sched_set_default_job_pick_plugin, +# not a re-implementation of any of them. +sched_picker_config_fields_test = executable('sched_picker_config_fields_test', + ['test_picker_config_fields.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', @@ -97,3 +109,5 @@ 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 picker config fields test', sched_picker_config_fields_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/scheduler/testsuite/test_picker_config_fields.c b/src/scheduler/testsuite/test_picker_config_fields.c new file mode 100644 index 00000000..bb9ecb04 --- /dev/null +++ b/src/scheduler/testsuite/test_picker_config_fields.c @@ -0,0 +1,306 @@ +/* + * test_picker_config_fields.c - white-box tests for the picker + * configuration fields added to xce_scheduler_t, xce_sched_queue_t, and + * xce_sched_project_t (scheduler-core spec, task 1.3, Req 2 AC 1-4). + * + * RED: before task 1.3, xce_scheduler_t had no job_pick_plugin field, + * xce_sched_queue_t had no project_name/job_pick_plugin fields, + * xce_sched_project_t had no priority field, and + * sched_set_default_job_pick_plugin() did not exist -- this file failed to + * compile. + * GREEN: the fields exist with the documented types/defaults, and + * make_sched_queue/make_sched_project (schedobj.c) leave the new fields at + * their documented default values. + * + * This task is scaffolding only -- no picker-name lookup/resolution + * (plugin_register/key_find against the plugin registry) exists yet, that + * is task 2.4's job. The tasks.md "Observable" for this task is: + * "a queue config with no override inherits the global picker name; + * a queue config with an override reports that override, not the + * global default" + * Read literally that would require the lookup logic this task explicitly + * excludes, so it is exercised here at the structural level instead: for + * a NULL-override queue, the field values a resolver would read are + * exactly sched->job_pick_plugin; for an overridden queue, the queue's own + * field is a different, non-NULL string, distinct from the scheduler's + * global default -- proving the two fields are independently readable and + * correctly defaulted, without this test (or production code) performing + * the resolution itself. + * + * Strategy: pull the REAL make_sched_queue/make_sched_project/ + * sched_set_default_job_pick_plugin into this TU by including schedobj.c + * (same white-box convention as test_job_update.c / + * test_job_array_expand.c / test_plugin_picker_registration.c). The + * scheduler-internal externs schedobj.c references but which are defined + * in other .c files not linked here are stubbed below -- identical stub + * set to test_job_update.c, since this task added no new external calls + * to schedobj.c (only field assignments plus one new self-contained + * function). + */ +#include +#include +#include +#include + +#include + +#include "kvmgr.h" +#include "scheduler.h" +#include "xstring.h" + +/* Pull the real implementation under test into this TU. */ +#include "../src/schedobj.c" + +/* Stubs for scheduler-internal externs schedobj.c references but which are + * defined in other .c files not linked into this test (same set as + * test_job_update.c). Not exercised by the tests below -- present only to + * satisfy the linker for this whole-file #include. */ +xce_sched_user_t* sched_get_user(xce_scheduler_t* s, char* n) +{ + (void)s; (void)n; return NULL; +} +xce_sched_project_t* sched_get_project(xce_scheduler_t* s, char* n) +{ + (void)s; (void)n; return NULL; +} +xce_sched_queue_t* sched_get_queue(xce_scheduler_t* s, char* n) +{ + (void)s; (void)n; return NULL; +} +xce_sched_job_t* sched_get_job(xce_scheduler_t* s, uint64_t jobid) +{ + (void)s; (void)jobid; return NULL; +} +int plugin_job_update(void* s, xce_sched_job_t* j) { (void)s; (void)j; return 0; } +int plugin_queue_update(void* s, xce_sched_queue_t* q) { (void)s; (void)q; return 0; } +int plugin_job_rusage_update(void* s, xce_sched_job_t* j, xce_job_rusages_t* r) +{ + (void)s; (void)j; (void)r; return 0; +} +xce_job_account_t* make_job_account(xce_job_alloc_t* a) { (void)a; return NULL; } +void free_job_account(xce_job_account_t* a) { (void)a; } +int sched_add_user(xce_scheduler_t* s, xce_sched_user_t* u) { (void)s; (void)u; return 0; } +int sched_add_project(xce_scheduler_t* s, xce_sched_project_t* p) { (void)s; (void)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; } + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* Per-test fixture: a zeroed scheduler, with the xstring intern table + * (needed by makeString(), which every constructor under test calls) and + * the CSE intern table live. Mirrors test_job_update.c's fixture_init -- + * called once per test so each test is self-contained. */ +static void fixture_init(xce_scheduler_t* sched) +{ + memset(sched, 0, sizeof(*sched)); + resreq_init(1); + xstring_table_init(1024 * 16); +} + +static xce_queue_t make_test_raw_queue(const char* name) +{ + xce_queue_t q; + memset(&q, 0, sizeof(q)); + q.queue = (char*)name; + return q; +} + +/* ---------------------------------------------------------------------- */ +/* Req 2 AC 1: xce_scheduler_t.job_pick_plugin */ +/* ---------------------------------------------------------------------- */ + +/* Field exists with the right type (char*) and is NULL ("unset") on a + * freshly zeroed scheduler, before any default is applied. */ +static void test_scheduler_job_pick_plugin_field_is_char_star_and_unset_by_default(void) +{ + xce_scheduler_t sched; + + fixture_init(&sched); + TEST_ASSERT_NULL(sched.job_pick_plugin); + + /* Type check: assigning a char* literal must compile without a cast -- + * this is a compile-time proof of the declared field type. */ + sched.job_pick_plugin = "some_name"; + TEST_ASSERT_EQUAL_STRING("some_name", sched.job_pick_plugin); +} + +/* Req 2 AC 1's literal wording: "defaulting to builtin_fcfs when unset". */ +static void test_sched_set_default_job_pick_plugin_applies_builtin_fcfs_when_unset(void) +{ + xce_scheduler_t sched; + + fixture_init(&sched); + TEST_ASSERT_NULL(sched.job_pick_plugin); + + sched_set_default_job_pick_plugin(&sched); + + TEST_ASSERT_NOT_NULL(sched.job_pick_plugin); + TEST_ASSERT_EQUAL_STRING("builtin_fcfs", sched.job_pick_plugin); +} + +/* "...when unset" implies idempotence: an already-configured name (however + * it got there) must not be clobbered by the default. */ +static void +test_sched_set_default_job_pick_plugin_does_not_clobber_configured_value(void) +{ + xce_scheduler_t sched; + + fixture_init(&sched); + sched.job_pick_plugin = makeString("priority_picker"); + + sched_set_default_job_pick_plugin(&sched); + + TEST_ASSERT_EQUAL_STRING("priority_picker", sched.job_pick_plugin); +} + +static void test_sched_set_default_job_pick_plugin_is_null_safe(void) +{ + sched_set_default_job_pick_plugin(NULL); /* must not crash */ + TEST_PASS(); +} + +/* ---------------------------------------------------------------------- */ +/* Req 2 AC 2/3: xce_sched_queue_t.job_pick_plugin / .project_name */ +/* ---------------------------------------------------------------------- */ + +/* A queue built from real config (make_sched_queue) with no picker/project + * info in the raw config struct leaves both new fields NULL -- the + * documented "unset"/"inherit" sentinel, not garbage or an empty string. */ +static void +test_make_sched_queue_defaults_project_name_and_override_to_null(void) +{ + xce_scheduler_t sched; + xce_queue_t q = make_test_raw_queue("normal"); + xce_sched_queue_t* queue; + + fixture_init(&sched); + + queue = make_sched_queue(&sched, &q); + + TEST_ASSERT_NOT_NULL(queue); + TEST_ASSERT_EQUAL_STRING("normal", queue->name); + TEST_ASSERT_NULL(queue->job_pick_plugin); + TEST_ASSERT_NULL(queue->project_name); +} + +/* tasks.md Observable, half 1: "a queue config with no override inherits + * the global picker name." Structural proof: the queue's own override + * field is NULL, and the value any resolver would fall back to + * (sched->job_pick_plugin) is the defaulted global name. */ +static void test_queue_with_no_override_inherits_global_picker_name(void) +{ + xce_scheduler_t sched; + xce_queue_t q = make_test_raw_queue("batch"); + xce_sched_queue_t* queue; + + fixture_init(&sched); + sched_set_default_job_pick_plugin(&sched); /* global = "builtin_fcfs" */ + + queue = make_sched_queue(&sched, &q); + + TEST_ASSERT_NOT_NULL(queue); + TEST_ASSERT_NULL(queue->job_pick_plugin); + TEST_ASSERT_EQUAL_STRING("builtin_fcfs", sched.job_pick_plugin); +} + +/* tasks.md Observable, half 2: "a queue config with an override reports + * that override, not the global default." Config-driven wiring of this + * field lands in a later task (xce_queue_t has no job_pick_plugin source + * yet), so the override is applied via direct struct assignment here -- + * exactly the "structurally representable" test style this task calls + * for -- and then read back. */ +static void +test_queue_with_override_reports_its_own_override_not_global_default(void) +{ + xce_scheduler_t sched; + xce_queue_t q = make_test_raw_queue("gpu"); + xce_sched_queue_t* queue; + + fixture_init(&sched); + sched_set_default_job_pick_plugin(&sched); /* global = "builtin_fcfs" */ + + queue = make_sched_queue(&sched, &q); + TEST_ASSERT_NOT_NULL(queue); + + queue->job_pick_plugin = makeString("priority_picker"); + + TEST_ASSERT_NOT_NULL(queue->job_pick_plugin); + TEST_ASSERT_EQUAL_STRING("priority_picker", queue->job_pick_plugin); + /* The override is reported, NOT the global default. */ + TEST_ASSERT_TRUE(strcmp(queue->job_pick_plugin, sched.job_pick_plugin) != 0); + TEST_ASSERT_EQUAL_STRING("builtin_fcfs", sched.job_pick_plugin); +} + +/* project_name is independently settable/readable (structural field check + * -- config-source wiring is a later task, same rationale as above). */ +static void test_queue_project_name_is_independently_settable(void) +{ + xce_scheduler_t sched; + xce_queue_t q = make_test_raw_queue("gpu"); + xce_sched_queue_t* queue; + + fixture_init(&sched); + queue = make_sched_queue(&sched, &q); + TEST_ASSERT_NOT_NULL(queue); + + queue->project_name = makeString("acct-42"); + + TEST_ASSERT_EQUAL_STRING("acct-42", queue->project_name); + /* Setting project_name must not disturb the independent picker-override + * field (still inheriting -- NULL). */ + TEST_ASSERT_NULL(queue->job_pick_plugin); +} + +/* ---------------------------------------------------------------------- */ +/* Req 2 AC: xce_sched_project_t.priority (scaffolding) */ +/* ---------------------------------------------------------------------- */ + +static void test_make_sched_project_priority_field_defaults_to_zero(void) +{ + xce_scheduler_t sched; + xce_sched_project_t* project; + + fixture_init(&sched); + + project = make_sched_project(&sched, "acct-42"); + + TEST_ASSERT_NOT_NULL(project); + TEST_ASSERT_EQUAL_STRING("acct-42", project->name); + /* Scaffolding only -- no quota/hierarchy logic exists yet + * (project-resource-ownership spec). */ + TEST_ASSERT_EQUAL_INT32(0, project->priority); +} + +/* Type check: field accepts a plain int32_t assignment/comparison -- + * compile-time proof of the declared type. */ +static void test_project_priority_field_is_int32_and_independently_settable(void) +{ + xce_scheduler_t sched; + xce_sched_project_t* project; + + fixture_init(&sched); + + project = make_sched_project(&sched, "acct-7"); + project->priority = 7; + + TEST_ASSERT_EQUAL_INT32(7, project->priority); + /* Setting priority must not disturb the project's name. */ + TEST_ASSERT_EQUAL_STRING("acct-7", project->name); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_scheduler_job_pick_plugin_field_is_char_star_and_unset_by_default); + RUN_TEST(test_sched_set_default_job_pick_plugin_applies_builtin_fcfs_when_unset); + RUN_TEST(test_sched_set_default_job_pick_plugin_does_not_clobber_configured_value); + RUN_TEST(test_sched_set_default_job_pick_plugin_is_null_safe); + RUN_TEST(test_make_sched_queue_defaults_project_name_and_override_to_null); + RUN_TEST(test_queue_with_no_override_inherits_global_picker_name); + RUN_TEST(test_queue_with_override_reports_its_own_override_not_global_default); + RUN_TEST(test_queue_project_name_is_independently_settable); + RUN_TEST(test_make_sched_project_priority_field_defaults_to_zero); + RUN_TEST(test_project_priority_field_is_int32_and_independently_settable); + return UNITY_END(); +}