diff --git a/.kiro/specs/scheduler-sim-decision-parity/tasks.md b/.kiro/specs/scheduler-sim-decision-parity/tasks.md index c1dc4609..dcf92930 100644 --- a/.kiro/specs/scheduler-sim-decision-parity/tasks.md +++ b/.kiro/specs/scheduler-sim-decision-parity/tasks.md @@ -1,7 +1,7 @@ # Implementation Plan - [ ] 1. Foundation: shared accounting surface and contention test data -- [ ] 1.1 Make resource-in-use accounting callable from either driver +- [x] 1.1 Make resource-in-use accounting callable from either driver - Relocate the resource re-accounting service (currently tied to the normal-mode-only driver struct) so it operates on the mode-agnostic core state shared by both drivers, mirroring the same relocation already done for the job-picker resolution service - Update all three existing call sites to the new signature: the normal-mode round handler's accounting call, the scheduler's own session-begin path's accounting call, and the scheduler's decision-commit path's call into the job-allocation-commit function that is part of this same relocated group - Body of the accounting logic itself is unchanged — this is a pure capability-relocation step, not new behavior diff --git a/src/scheduler/include/core.h b/src/scheduler/include/core.h index 181f3e0d..3e960647 100644 --- a/src/scheduler/include/core.h +++ b/src/scheduler/include/core.h @@ -246,6 +246,47 @@ extern int sched_queue_is_ready(xce_sched_queue_t*); */ extern int sched_resolve_job_picker(xce_sched_core_t*, xce_sched_plugin_t**); +/* + * sched_account_jobs/sched_charge_account/sched_uncharge_account/ + * sched_commit_job_alloc - resource-in-use accounting (scheduler-sim- + * decision-parity spec, task 1.1). Relocated from scheduler.c/resource.c's + * xce_scheduler_t* signature to this xce_sched_core_t* signature, following + * the exact same precedent sched_resolve_job_picker established above (task + * 4.3): pure signature relocation, body unchanged -- every reference in + * these functions' bodies already only touched sched->core.* before this + * move. Moved so the simulation driver (which has no xce_scheduler_t* of + * its own, only xce_sched_core_t) can call the same accounting service the + * normal-mode driver already uses, restoring the resource-in-use accounting + * parity `.kiro/steering/scheduler-driver-equivalence.md` documents. + * + * sched_account_jobs() - re-derives resInUse/sharedInUse from every + * currently-running job's known resource allocation (job->accounts != NULL) + * by re-charging each one via sched_charge_account(). Skips a RUN-state job + * whose allocation is not yet known -- not an error; see + * scheduler-sim-decision-parity design.md's Recorded-Decision Reconciliation + * component for why that is the expected state before this call runs. + * Callers (both drivers) must call this strictly after the session's resval + * reset and strictly before the picker/allocator are unblocked -- see + * xce_core_begin_session()/xce_core_signal_go()'s own doc comment below for + * the full ordering contract this depends on. Was exported (no longer + * `static`) as of scheduler-core spec task 5.2 for the identical reason; + * still defined in scheduler.c, only its declaration/signature move here. + * + * sched_charge_account()/sched_uncharge_account() - add/remove one job's + * xce_job_account_t (shared + per-host resource usage) to/from + * core->sharedInUse and each worker's resInUse. + * + * sched_commit_job_alloc() - charges a job's already-resolved allocation + * (job->accounts) via sched_charge_account(); a no-op (returns 0) if the + * job's allocation is not yet known. Called by sched_commit_decision() + * (scheduler.c) immediately after a fresh DISPATCH decision populates + * job->allocs/job->accounts. + */ +extern int sched_account_jobs(xce_sched_core_t*); +extern int sched_charge_account(xce_sched_core_t*, xce_job_account_t*); +extern int sched_uncharge_account(xce_sched_core_t*, xce_job_account_t*); +extern int sched_commit_job_alloc(xce_sched_core_t*, xce_sched_job_t*); + /* * xce_core_event_type_t / xce_core_event_t - the core's typed input-event * representation (scheduler-core spec, task 4.3, Req 5 AC 4). Exactly the 9 diff --git a/src/scheduler/include/scheduler.h b/src/scheduler/include/scheduler.h index e7f9c10c..cec42f48 100644 --- a/src/scheduler/include/scheduler.h +++ b/src/scheduler/include/scheduler.h @@ -252,17 +252,14 @@ extern int sched_update(xce_scheduler_t*); extern void sched_go(xce_scheduler_t*); extern void sched_begin_session(xce_scheduler_t*); extern void sched_end_session(xce_scheduler_t*); -/* sched_account_jobs - per-running-job resource-in-use accounting - * (re-charges every RUN-state job's resources via sched_charge_account() - * after sched_begin_session()'s resval reset zeroes them). Exported as of - * scheduler-core spec task 5.2 so the new normal-mode driver loop - * (driver.c's xce_sched_driver_run_normal_mode_round()) can call it - * directly at the exact - * point xce_core_begin_session()'s caller must call it: after the resval - * reset, strictly before xce_core_signal_go() unblocks the picker thread. - * Was `static`; body unchanged, still also called from - * sched_begin_session() below (unchanged legacy call site). */ -extern int sched_account_jobs(xce_scheduler_t*); +/* sched_account_jobs/sched_charge_account/sched_uncharge_account/ + * sched_commit_job_alloc relocated to core.h (scheduler-sim-decision-parity + * spec, task 1.1) -- same rationale as sched_resolve_job_picker above: they + * now operate on xce_sched_core_t*, not xce_scheduler_t*, so the simulation + * driver (mode-agnostic, no xce_scheduler_t* of its own) gains the same + * accounting capability the normal-mode driver already had. Call sites in + * this file (sched_begin_session() below, sched_commit_decision()) now pass + * &sched->core. See core.h for the full contract; body unchanged. */ /* sched_get_job/sched_add_job/sched_get_queue/sched_add_queue/ * sched_get_worker/sched_add_worker/sched_get_user/sched_add_user/ * sched_get_project/sched_add_project/sched_job_is_ready/ @@ -284,9 +281,6 @@ extern int sched_check_host_satisfy(xce_scheduler_t*, xce_sched_worker_t*, extern int sched_collect_workers(xce_scheduler_t*, xlist_t*, zarray_iter_skip_fn_t, sched_group_worker_fn_t); -extern int sched_charge_account(xce_scheduler_t*, xce_job_account_t*); -extern int sched_uncharge_account(xce_scheduler_t*, xce_job_account_t*); -extern int sched_commit_job_alloc(xce_scheduler_t*, xce_sched_job_t*); extern uint64_t sched_get_id(xce_scheduler_t*); extern xce_alloc_t** sched_make_allocs(xce_scheduler_t*, char*, int, int*, float*, int*); diff --git a/src/scheduler/src/driver.c b/src/scheduler/src/driver.c index 57549251..b0da87d5 100644 --- a/src/scheduler/src/driver.c +++ b/src/scheduler/src/driver.c @@ -692,10 +692,12 @@ int xce_sched_driver_on_decision(void* driver_ctx, xce_decision_t* decision) * builtin_fcfs) and sched_account_jobs() (re-charges every RUNNING job's * resources after the resval reset zeroes them) BEFORE sched_go() signals * the picker thread. xce_core_begin_session()/xce_core_end_session() - * (core_session.c, task 4.4) deliberately exclude both -- they need the - * full xce_scheduler_t*, not just xce_sched_core_t* -- so this function - * calls them itself, in the same slot: strictly AFTER - * xce_core_begin_session()'s resval reset, strictly BEFORE + * (core_session.c, task 4.4) deliberately exclude both -- plugin_begin_session() + * needs the full xce_scheduler_t* (sched_account_jobs() no longer does, as + * of scheduler-sim-decision-parity task 1.1's relocation to xce_sched_core_t*, + * but this call site still holds an xce_scheduler_t* and passes &(sched->core) + * for it) -- so this function calls them itself, in the same slot: strictly + * AFTER xce_core_begin_session()'s resval reset, strictly BEFORE * xce_core_signal_go() unblocks the picker. Neither plugin_begin_session() * nor sched_account_jobs() has any data dependency on WHERE within that * window it runs (verified: plugin_begin_session() only touches @@ -746,7 +748,7 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx) * xce_core_begin_session() just performed, MUST complete before * xce_core_signal_go() below. */ plugin_begin_session(sched); - sched_account_jobs(sched); + sched_account_jobs(&(sched->core)); /* Req 5 AC 10 (scheduler-core spec, task 9.2): "On core_go(): begin * building a ReplaySession message with beginRound = xce_core_now(core)" diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index e13a4abd..72b55551 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -398,7 +398,7 @@ int sched_check_host_satisfy(xce_scheduler_t* sched, xce_sched_worker_t* w, return rc; } -static int sched_update_res_accounts(xce_scheduler_t* sched, zarray_t* array, +static int sched_update_res_accounts(xce_sched_core_t* core, zarray_t* array, int global, int naccounts, xce_res_account_t** accounts, int charge) { @@ -421,9 +421,9 @@ static int sched_update_res_accounts(xce_scheduler_t* sched, zarray_t* array, xce_resource_t* r = NULL; int rc = 0; - lock(sched->core.resmgr, RTTI_LOCK_READ); - rc = key_find(sched->core.resmgr, (void*)acct->resname, (void**)&r); - unlock(sched->core.resmgr); + lock(core->resmgr, RTTI_LOCK_READ); + rc = key_find(core->resmgr, (void*)acct->resname, (void**)&r); + unlock(core->resmgr); if (rc < 0) { return rc; @@ -433,15 +433,15 @@ static int sched_update_res_accounts(xce_scheduler_t* sched, zarray_t* array, if (r == NULL) { int rc; - rc = add_vres(&r, sched->core.resmgr, acct->resname, global); + rc = add_vres(&r, core->resmgr, acct->resname, global); if (rc < 0) { return rc; } - lock(sched->core.resmgr, RTTI_LOCK_READ); - rc = key_find(sched->core.resmgr, (void*)acct->resname, + lock(core->resmgr, RTTI_LOCK_READ); + rc = key_find(core->resmgr, (void*)acct->resname, (void**)&r); - unlock(sched->core.resmgr); + unlock(core->resmgr); if (rc < 0) { return rc; } @@ -556,12 +556,12 @@ int sched_try_alloc_on_hosts(xce_worker_group_t* g, va_list ap) return sched_try_alloc_on_host_(w, ap); } -static int sched_update_account(xce_scheduler_t* sched, +static int sched_update_account(xce_sched_core_t* core, xce_job_account_t* account, int charge) { int i; - sched_update_res_accounts(sched, &(sched->core.sharedInUse), 1, + sched_update_res_accounts(core, &(core->sharedInUse), 1, account->nres_shared, account->accounts_shared, charge); for (i = 0; i < account->nhosts; i++) { @@ -572,34 +572,34 @@ static int sched_update_account(xce_scheduler_t* sched, continue; } - w = sched_get_worker(&(sched->core), haccount->host); + w = sched_get_worker(core, haccount->host); if (w == NULL) { continue; } - sched_update_res_accounts(sched, &(w->resInUse), 0, haccount->naccount, + sched_update_res_accounts(core, &(w->resInUse), 0, haccount->naccount, haccount->accounts, charge); } return 0; } -int sched_charge_account(xce_scheduler_t* sched, xce_job_account_t* acct) +int sched_charge_account(xce_sched_core_t* core, xce_job_account_t* acct) { - return sched_update_account(sched, acct, 1); + return sched_update_account(core, acct, 1); } -int sched_uncharge_account(xce_scheduler_t* sched, xce_job_account_t* acct) +int sched_uncharge_account(xce_sched_core_t* core, xce_job_account_t* acct) { - return sched_update_account(sched, acct, 0); + return sched_update_account(core, acct, 0); } -int sched_commit_job_alloc(xce_scheduler_t* sched, xce_sched_job_t* job) +int sched_commit_job_alloc(xce_sched_core_t* core, xce_sched_job_t* job) { if (job->accounts == NULL) { return 0; } - return sched_charge_account(sched, job->accounts); + return sched_charge_account(core, job->accounts); } static float get_resval_by_id(xvec_idval_t* vec, int idx) diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index 84d59e6b..66d9f252 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -1250,14 +1250,17 @@ static int sched_reset_workers(xce_scheduler_t* sched) } /* No longer `static` as of scheduler-core spec task 5.2 -- see the - * declaration comment in scheduler.h for why the new normal-mode driver - * loop (main.c) needs to call this directly. Body unchanged. */ -int sched_account_jobs(xce_scheduler_t* sched) + * declaration comment in core.h for why the new normal-mode driver loop + * (main.c) needs to call this directly. Relocated from an xce_scheduler_t* + * to an xce_sched_core_t* signature (scheduler-sim-decision-parity spec, + * task 1.1) -- see core.h's declaration comment for the full rationale; + * body unchanged. */ +int sched_account_jobs(xce_sched_core_t* core) { iterator_t it; int rc = 0; - rc = iter_make(&it, sched->core.jobs); + rc = iter_make(&it, core->jobs); if (rc < 0) { return rc; } @@ -1275,7 +1278,7 @@ int sched_account_jobs(xce_scheduler_t* sched) } if (job->accounts != NULL) { - sched_charge_account(sched, job->accounts); + sched_charge_account(core, job->accounts); } } @@ -1305,7 +1308,7 @@ void sched_begin_session(xce_scheduler_t* sched) sched_reset_shared_resval(sched); sched_reset_workers(sched); - sched_account_jobs(sched); + sched_account_jobs(&(sched->core)); /* Staging merge done — sched_end_session's GC guard reads this field. */ atomic_store_explicit(&sched->core.staging_merge_complete, 1, @@ -1440,7 +1443,7 @@ int sched_commit_decision(xce_scheduler_t* sched, xce_decision_t* decision) job->allocs = decision->u.alloc; job->accounts = make_job_account(job->allocs); - return sched_commit_job_alloc(sched, job); + return sched_commit_job_alloc(&(sched->core), job); } return 0; diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index b340adca..71242651 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -405,13 +405,14 @@ sched_driver_decision_cb_test = executable( # through a genuinely test-registered xce_sched_plugin_t whose # begin_session_fn/end_session_fn set/check an ordering flag -- per this # task's own suggested technique, this is a stronger proof than spying on -# plugin_begin_session() itself. sched_account_jobs() (scheduler.c, -# xce_scheduler_t*-coupled, pulls in resource.c's sched_charge_account()) -# is instead a SPY test double recording an order index -- this test's -# target is the DRIVER's ordering discipline, not sched_account_jobs()'s -# own resource-accounting correctness (out of this task's scope, same -# "spy the xce_scheduler_t*-coupled collaborator" rationale -# test_driver_decision_cb.c/test_driver_translate.c already established for +# plugin_begin_session() itself. sched_account_jobs() (scheduler.c, defined +# on xce_sched_core_t* as of scheduler-sim-decision-parity task 1.1, pulls +# in resource.c's sched_charge_account()) is instead a SPY test double +# recording an order index -- this test's target is the DRIVER's ordering +# discipline, not sched_account_jobs()'s own resource-accounting +# correctness (out of this task's scope, same "spy the collaborator" +# rationale test_driver_decision_cb.c/test_driver_translate.c already +# established for the xce_scheduler_t*-coupled # sched_publish_dispatch()/sched_publish_pend_reason()). sched_driver_session_ordering_test = executable( 'sched_driver_session_ordering_test', @@ -420,6 +421,23 @@ sched_driver_session_ordering_test = executable( dependencies: [scheduler_test_deps], install: false) +# Requirement 1.4 proof for the Core-Level Resource Accounting component +# (scheduler-sim-decision-parity spec, task 1.1): sched_charge_account()/ +# sched_uncharge_account()/sched_commit_job_alloc() (resource.c, relocated +# from xce_scheduler_t* to xce_sched_core_t* by this task) produce +# byte-identical resInUse/sharedInUse output whether `core` came from an +# xce_scheduler_t (normal-mode driver) or an xce_sched_sim_driver_t +# (simulation driver, sim.h) -- see test_resource_accounting.c's own file +# header comment for why sched_account_jobs() itself (scheduler.c) is not +# also white-box #included here (no existing test in this directory +# white-box #includes scheduler.c, for the same reason documented there). +sched_resource_accounting_test = executable( + 'sched_resource_accounting_test', + ['test_resource_accounting.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + # Cross-core behavioral-equivalence proof for # xce_sched_core_save_snapshot()/xce_sched_core_load_snapshot() # (scheduler-core spec, task 8.1, Req 5 AC 21 / Req 6 AC 5): save immediately @@ -996,6 +1014,9 @@ test('scheduler driver decision cb test', test('scheduler driver session ordering test', sched_driver_session_ordering_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +test('scheduler resource accounting test', + sched_resource_accounting_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler snapshot save load test', sched_snapshot_save_load_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/scheduler/testsuite/test_admin_request.c b/src/scheduler/testsuite/test_admin_request.c index 7ecaf193..c0ef917d 100644 --- a/src/scheduler/testsuite/test_admin_request.c +++ b/src/scheduler/testsuite/test_admin_request.c @@ -178,9 +178,9 @@ int plugin_end_session(void* arg) return 0; } -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; } diff --git a/src/scheduler/testsuite/test_driver_decision_cb.c b/src/scheduler/testsuite/test_driver_decision_cb.c index 93a918e0..75ac22de 100644 --- a/src/scheduler/testsuite/test_driver_decision_cb.c +++ b/src/scheduler/testsuite/test_driver_decision_cb.c @@ -142,9 +142,9 @@ int plugin_end_session(void* arg) return 0; } -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; } diff --git a/src/scheduler/testsuite/test_driver_session_ordering.c b/src/scheduler/testsuite/test_driver_session_ordering.c index c36c0d77..3e5c89e1 100644 --- a/src/scheduler/testsuite/test_driver_session_ordering.c +++ b/src/scheduler/testsuite/test_driver_session_ordering.c @@ -80,13 +80,16 @@ * sched_account_jobs() (scheduler.c) is instead a SPY test double (this * file never links scheduler.c) -- same rationale * test_driver_decision_cb.c/test_driver_translate.c already established - * for sched_publish_dispatch()/sched_publish_pend_reason(): it is - * xce_scheduler_t*-coupled and pulls in resource.c's sched_charge_account() - * machinery, whose OWN resource-accounting correctness is out of THIS - * task's scope (this test's target is the DRIVER's ordering discipline -- - * does it call sched_account_jobs() at all, and in the right window -- - * not whether sched_account_jobs() itself charges resources correctly, - * which is scheduler-allocation spec territory with its own coverage). + * for sched_publish_dispatch()/sched_publish_pend_reason(): it pulls in + * resource.c's sched_charge_account() machinery, whose OWN resource- + * accounting correctness is out of THIS task's scope (this test's target + * is the DRIVER's ordering discipline -- does it call sched_account_jobs() + * at all, and in the right window -- not whether sched_account_jobs() + * itself charges resources correctly, which is scheduler-allocation spec + * territory, plus test_resource_accounting.c's own coverage as of + * scheduler-sim-decision-parity task 1.1). As of that same task, + * sched_account_jobs() takes xce_sched_core_t* (relocated from + * xce_scheduler_t*) -- this spy's signature matches. * * White-box #include of ../src/core.c, ../src/core_clock.c, * ../src/core_events.c, ../src/core_session.c, ../src/decision.c, @@ -346,8 +349,8 @@ int sched_resolve_job_picker(xce_sched_core_t* core, xce_sched_plugin_t** out) } /* sched_account_jobs() itself is defined further below (SPY test double - * section) -- its prototype already comes from scheduler.h, included - * above. */ + * section) -- its prototype already comes from core.h (included above via + * scheduler.h). */ void setUp(void) { return; } void tearDown(void) { return; } @@ -409,9 +412,9 @@ static int find_log_index(ordering_event_tag_t tag) static _Atomic int g_account_jobs_calls; -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; atomic_fetch_add(&g_account_jobs_calls, 1); log_event(EVT_ACCOUNT_JOBS); diff --git a/src/scheduler/testsuite/test_driver_translate.c b/src/scheduler/testsuite/test_driver_translate.c index 6ccd7004..e54ea751 100644 --- a/src/scheduler/testsuite/test_driver_translate.c +++ b/src/scheduler/testsuite/test_driver_translate.c @@ -228,9 +228,9 @@ int plugin_end_session(void* arg) return 0; } -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; } diff --git a/src/scheduler/testsuite/test_record_lifecycle.c b/src/scheduler/testsuite/test_record_lifecycle.c index c1a32ddd..93888b85 100644 --- a/src/scheduler/testsuite/test_record_lifecycle.c +++ b/src/scheduler/testsuite/test_record_lifecycle.c @@ -143,9 +143,9 @@ int sched_fill_limit_requests(xce_scheduler_t* sched, xvec_idval_t* vec, return 0; } -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; } diff --git a/src/scheduler/testsuite/test_resource_accounting.c b/src/scheduler/testsuite/test_resource_accounting.c new file mode 100644 index 00000000..cfd806f5 --- /dev/null +++ b/src/scheduler/testsuite/test_resource_accounting.c @@ -0,0 +1,359 @@ +/* + * test_resource_accounting.c - Requirement 1.4 proof for the Core-Level + * Resource Accounting component (scheduler-sim-decision-parity spec, task + * 1.1; design.md's "Core-Level Resource Accounting" Unit Tests bullet): + * "produces identical resInUse/sharedInUse output whether called with a + * core obtained from xce_scheduler_t or from xce_sched_sim_driver_t." + * + * Task 1.1 itself is a pure signature relocation (xce_scheduler_t* -> + * xce_sched_core_t*) of sched_account_jobs()/sched_charge_account()/ + * sched_uncharge_account()/sched_update_account()/ + * sched_update_res_accounts()/sched_commit_job_alloc() -- body unchanged, + * every reference already `sched->core.*` before the move (confirmed via + * `grep "sched->" resource.c | grep -v "sched->core"` returning zero + * matches, design.md's Existing Architecture Analysis). Requirement 1.4 is + * nonetheless a genuinely NEW observable property worth its own test (not + * just "nothing broke"): now that these functions take xce_sched_core_t* + * directly, they are usable identically regardless of which driver-level + * container struct (xce_scheduler_t, the normal-mode driver's; or + * xce_sched_sim_driver_t, the simulation driver's, sim.h) embeds the core. + * This file drives the REAL sched_charge_account()/sched_uncharge_account()/ + * sched_commit_job_alloc() (resource.c) against two independently + * initialized xce_sched_core_t instances -- one embedded in an + * xce_scheduler_t, one embedded in an xce_sched_sim_driver_t -- with + * identical worker/account state, and asserts byte-identical + * resInUse/sharedInUse results. + * + * sched_account_jobs() itself (scheduler.c) is NOT white-box #included here + * -- unlike this file's own sched_charge_account()/sched_commit_job_alloc() + * targets (resource.c, a lightweight, already-proven-includable file per + * test.c's own #include "../src/resource.c"), scheduler.c is a ~1400-line + * file with a much larger link-time dependency surface (controller-join, + * capnp decision publishing, HA candidate handling, picker/allocator thread + * lifecycle) that no existing test in this directory white-box #includes for + * exactly that reason (confirmed via grep across testsuite/ before writing + * this file). sched_account_jobs() itself is a thin, ~20-line wrapper + * (iterate core->jobs, call sched_charge_account() for each RUN-state job + * with a known allocation) with no accounting logic of its own -- its + * driver-ordering discipline (called at the right point in the round + * boundary, on both drivers) is already proven by + * test_driver_session_ordering.c (a spy, by design -- see that file's own + * header comment) and this feature's later tasks (2.x/3.x, sim_loop.c's own + * round-boundary integration). This file instead proves the actual + * accounting *body* -- the part design.md's Testing Strategy bullet is + * about -- via its real callees, exercised directly. + * + * White-box #include of ../src/core.c (for the real + * xce_sched_core_init()/sched_add_worker()/sched_get_worker() the fixture + * needs) and ../src/resource.c (the functions under test). Neither file + * needs core_clock.c/core_events.c/core_session.c/decision.c/plugin.c -- + * this test never calls xce_core_push_events()/xce_core_begin_session()/ + * anything decision- or plugin-registration-related, only the plain + * add-worker/charge-account entry points core.c/resource.c themselves + * define. Stubs below (kill_sched_* family, plugin_unregister) are the + * standard link-satisfying no-ops xce_sched_core_init()'s kvmgr datakillers + * reference (never actually invoked -- this fixture is never torn down, + * same convention as test_driver_session_ordering.c/ + * test_snapshot_save_load.c's identical stub set). + * sched_worker_release_host_ctx() is the one real (non-stub) symbol this + * pulls from resource.c itself, not from core_session.c -- unused here + * (this test never calls it), listed only because resource.c defines it. + */ +#include +#include +#include + +#include + +#include "scheduler.h" +#include "sim.h" + +/* Pull in the real xce_sched_core_init()/sched_add_worker()/ + * sched_get_worker() (core.c) and sched_charge_account()/ + * sched_uncharge_account()/sched_commit_job_alloc()/get_resvals() (resource.c, + * the functions under test) into this TU (white-box #include, same + * convention as every other file in this directory). */ +#include "../src/core.c" +#include "../src/resource.c" + +void kill_sched_job(void* arg) { (void)arg; } +void kill_sched_queue(void* arg) { (void)arg; } +void kill_sched_worker(void* arg) { (void)arg; } +void kill_sched_user(void* arg) { (void)arg; } +void kill_sched_project(void* arg) { (void)arg; } +void kill_sched_resval(xce_val_t* val) { (void)val; } +void plugin_unregister(void* arg) { (void)arg; } + +/* Link-satisfying stubs for resource.c's sched_try_alloc_on_host_() (its + * OWN callees, plugin_prepare_allocation()/sched_ctx_get_uncomit_host(), + * live in plugin.c/schedobj.c -- neither white-box #included here). Never + * actually invoked: this file never calls sched_try_alloc_on_hosts()/ + * sched_try_alloc_on_host_(), only sched_charge_account()/ + * sched_uncharge_account()/sched_commit_job_alloc(). Same two stubs + * test.c's own #include "../src/resource.c" already establishes needing. */ +int plugin_prepare_allocation(void* 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; +} + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* ---------------------------------------------------------------------- */ +/* Fixture helpers */ +/* ---------------------------------------------------------------------- */ + +/* + * setup_core_with_worker - real xce_sched_core_init() (core.c) plus one + * real worker (sched_add_worker(), core.c), its resvals/resInUse zarrays + * freshly zarray_init()'d exactly the way a real worker join would leave + * them (sched_add_worker() itself does not do this -- callers own it, same + * as every other white-box fixture in this directory that adds a worker + * directly rather than through the JOBS_PUSH/WORKERS_PUSH event path). + */ +static void setup_core_with_worker(xce_sched_core_t* core, + const char* worker_name) +{ + xce_sched_worker_t* w; + int rc; + + rc = xce_sched_core_init(core); + TEST_ASSERT_EQUAL_INT(0, rc); + + w = (xce_sched_worker_t*)calloc(1, sizeof(*w)); + TEST_ASSERT_NOT_NULL(w); + + w->name = strdup(worker_name); + TEST_ASSERT_NOT_NULL(w->name); + + rc = zarray_init(&(w->resvals), 8, NULL); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = zarray_init(&(w->resInUse), 8, NULL); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = sched_add_worker(core, w); + TEST_ASSERT_TRUE(rc >= 0); +} + +/* + * make_test_account - one shared-resource charge (residx `shared_residx`) + * plus one per-host charge (residx `host_residx`, against `worker_name`) -- + * enough to exercise both of sched_update_account()'s two branches + * (core->sharedInUse and a specific worker's resInUse). residx is set + * non-negative directly (bypassing sched_update_res_accounts()'s + * resmgr-lookup branch entirely) -- this test targets the charge/uncharge + * arithmetic itself, not resource-name resolution, which is unrelated, + * already-existing logic this task's relocation does not touch. + */ +static xce_job_account_t* +make_test_account(const char* worker_name, int shared_residx, + double shared_used, double shared_reserved, int host_residx, + double host_used, double host_reserved) +{ + xce_job_account_t* acct + = (xce_job_account_t*)calloc(1, sizeof(*acct)); + TEST_ASSERT_NOT_NULL(acct); + + acct->nres_shared = 1; + acct->accounts_shared = (xce_res_account_t**)calloc( + 1, sizeof(*acct->accounts_shared)); + TEST_ASSERT_NOT_NULL(acct->accounts_shared); + acct->accounts_shared[0] + = (xce_res_account_t*)calloc(1, sizeof(xce_res_account_t)); + TEST_ASSERT_NOT_NULL(acct->accounts_shared[0]); + acct->accounts_shared[0]->residx = shared_residx; + acct->accounts_shared[0]->used = shared_used; + acct->accounts_shared[0]->reserved = shared_reserved; + + acct->nhosts = 1; + acct->accounts = (xce_host_account_t**)calloc(1, sizeof(*acct->accounts)); + TEST_ASSERT_NOT_NULL(acct->accounts); + acct->accounts[0] + = (xce_host_account_t*)calloc(1, sizeof(xce_host_account_t)); + TEST_ASSERT_NOT_NULL(acct->accounts[0]); + acct->accounts[0]->host = strdup(worker_name); + acct->accounts[0]->naccount = 1; + acct->accounts[0]->accounts + = (xce_res_account_t**)calloc(1, sizeof(*acct->accounts[0]->accounts)); + TEST_ASSERT_NOT_NULL(acct->accounts[0]->accounts); + acct->accounts[0]->accounts[0] + = (xce_res_account_t*)calloc(1, sizeof(xce_res_account_t)); + TEST_ASSERT_NOT_NULL(acct->accounts[0]->accounts[0]); + acct->accounts[0]->accounts[0]->residx = host_residx; + acct->accounts[0]->accounts[0]->used = host_used; + acct->accounts[0]->accounts[0]->reserved = host_reserved; + + return acct; +} + +/* ---------------------------------------------------------------------- */ +/* Tests */ +/* ---------------------------------------------------------------------- */ + +#define WORKER_NAME "worker-a" +#define SHARED_RESIDX 0 +#define HOST_RESIDX 1 +#define SHARED_USED 3.0 +#define SHARED_RESERVED 1.5 +#define HOST_USED 2.0 +#define HOST_RESERVED 0.5 +/* SHARED_USED + SHARED_RESERVED, HOST_USED + HOST_RESERVED */ +#define EXPECTED_SHARED_CHARGE 4.5f +#define EXPECTED_HOST_CHARGE 2.5f + +/* + * test_charge_account_identical_across_core_containers - Requirement 1.4: + * sched_charge_account()/sched_uncharge_account() (resource.c) produce + * byte-identical resInUse/sharedInUse results whether `core` came from an + * xce_scheduler_t (sched.core, the normal-mode driver's container) or an + * xce_sched_sim_driver_t (sim.core, the simulation driver's container) -- + * the exact property that makes the simulation driver's build now able to + * call the same accounting service the normal-mode driver already used. + */ +static void test_charge_account_identical_across_core_containers(void) +{ + xce_scheduler_t sched; + xce_sched_sim_driver_t sim; + xce_job_account_t* acct; + xce_sched_worker_t* w_sched; + xce_sched_worker_t* w_sim; + float shared_sched[1]; + float shared_sim[1]; + float host_sched[1]; + float host_sim[1]; + int shared_idx[1] = { SHARED_RESIDX }; + int host_idx[1] = { HOST_RESIDX }; + int rc; + + memset(&sched, 0, sizeof(sched)); + memset(&sim, 0, sizeof(sim)); + + setup_core_with_worker(&(sched.core), WORKER_NAME); + setup_core_with_worker(&(sim.core), WORKER_NAME); + + /* Same account struct fed to both cores -- sched_charge_account() never + * mutates it when residx is already non-negative (see + * make_test_account()'s own comment), so reuse is safe and keeps the + * two calls genuinely identical, not merely equal-by-construction. */ + acct = make_test_account(WORKER_NAME, SHARED_RESIDX, SHARED_USED, + SHARED_RESERVED, HOST_RESIDX, HOST_USED, + HOST_RESERVED); + + rc = sched_charge_account(&(sched.core), acct); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = sched_charge_account(&(sim.core), acct); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* --- sharedInUse: identical across both containers, and correct --- */ + get_resvals(&(sched.core.sharedInUse), 1, shared_idx, shared_sched); + get_resvals(&(sim.core.sharedInUse), 1, shared_idx, shared_sim); + TEST_ASSERT_EQUAL_FLOAT(EXPECTED_SHARED_CHARGE, shared_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(shared_sched[0], shared_sim[0]); + + /* --- per-worker resInUse: identical across both containers --- */ + w_sched = sched_get_worker(&(sched.core), WORKER_NAME); + w_sim = sched_get_worker(&(sim.core), WORKER_NAME); + TEST_ASSERT_NOT_NULL(w_sched); + TEST_ASSERT_NOT_NULL(w_sim); + + get_resvals(&(w_sched->resInUse), 1, host_idx, host_sched); + get_resvals(&(w_sim->resInUse), 1, host_idx, host_sim); + TEST_ASSERT_EQUAL_FLOAT(EXPECTED_HOST_CHARGE, host_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(host_sched[0], host_sim[0]); + + /* --- sched_uncharge_account() mirrors this identically too --- */ + rc = sched_uncharge_account(&(sched.core), acct); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = sched_uncharge_account(&(sim.core), acct); + TEST_ASSERT_EQUAL_INT(0, rc); + + get_resvals(&(sched.core.sharedInUse), 1, shared_idx, shared_sched); + get_resvals(&(sim.core.sharedInUse), 1, shared_idx, shared_sim); + TEST_ASSERT_EQUAL_FLOAT(0.0f, shared_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(shared_sched[0], shared_sim[0]); + + get_resvals(&(w_sched->resInUse), 1, host_idx, host_sched); + get_resvals(&(w_sim->resInUse), 1, host_idx, host_sim); + TEST_ASSERT_EQUAL_FLOAT(0.0f, host_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(host_sched[0], host_sim[0]); +} + +/* + * test_commit_job_alloc_identical_across_core_containers - Requirement 1.4 + * for sched_commit_job_alloc() specifically (the third relocated group + * member scheduler.c's sched_commit_decision() calls, design.md's own File + * Structure Plan entry for scheduler.c): both its no-op (job->accounts == + * NULL) and its charging path behave identically regardless of which + * driver-level struct the core came from. + */ +static void test_commit_job_alloc_identical_across_core_containers(void) +{ + xce_scheduler_t sched; + xce_sched_sim_driver_t sim; + xce_sched_job_t job_sched; + xce_sched_job_t job_sim; + xce_job_account_t* acct; + float shared_sched[1]; + float shared_sim[1]; + int shared_idx[1] = { SHARED_RESIDX }; + int rc; + + memset(&sched, 0, sizeof(sched)); + memset(&sim, 0, sizeof(sim)); + memset(&job_sched, 0, sizeof(job_sched)); + memset(&job_sim, 0, sizeof(job_sim)); + + setup_core_with_worker(&(sched.core), WORKER_NAME); + setup_core_with_worker(&(sim.core), WORKER_NAME); + + /* --- job->accounts == NULL: documented no-op, identical on both --- */ + job_sched.accounts = NULL; + job_sim.accounts = NULL; + + rc = sched_commit_job_alloc(&(sched.core), &job_sched); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = sched_commit_job_alloc(&(sim.core), &job_sim); + TEST_ASSERT_EQUAL_INT(0, rc); + + get_resvals(&(sched.core.sharedInUse), 1, shared_idx, shared_sched); + get_resvals(&(sim.core.sharedInUse), 1, shared_idx, shared_sim); + TEST_ASSERT_EQUAL_FLOAT(0.0f, shared_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(shared_sched[0], shared_sim[0]); + + /* --- job->accounts set: charges through sched_charge_account(), + * identical on both --- */ + acct = make_test_account(WORKER_NAME, SHARED_RESIDX, + SHARED_USED, SHARED_RESERVED, + HOST_RESIDX, HOST_USED, + HOST_RESERVED); + job_sched.accounts = acct; + job_sim.accounts = acct; + + rc = sched_commit_job_alloc(&(sched.core), &job_sched); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = sched_commit_job_alloc(&(sim.core), &job_sim); + TEST_ASSERT_EQUAL_INT(0, rc); + + get_resvals(&(sched.core.sharedInUse), 1, shared_idx, shared_sched); + get_resvals(&(sim.core.sharedInUse), 1, shared_idx, shared_sim); + TEST_ASSERT_EQUAL_FLOAT(EXPECTED_SHARED_CHARGE, shared_sched[0]); + TEST_ASSERT_EQUAL_FLOAT(shared_sched[0], shared_sim[0]); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_charge_account_identical_across_core_containers); + RUN_TEST(test_commit_job_alloc_identical_across_core_containers); + return UNITY_END(); +} diff --git a/src/scheduler/testsuite/test_resval_ownership.c b/src/scheduler/testsuite/test_resval_ownership.c index a1a0b726..8cc2bf22 100644 --- a/src/scheduler/testsuite/test_resval_ownership.c +++ b/src/scheduler/testsuite/test_resval_ownership.c @@ -233,9 +233,9 @@ int plugin_end_session(void* arg) return 0; } -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; } diff --git a/src/scheduler/testsuite/test_sim_equivalence.c b/src/scheduler/testsuite/test_sim_equivalence.c index 1236a588..1680f8ae 100644 --- a/src/scheduler/testsuite/test_sim_equivalence.c +++ b/src/scheduler/testsuite/test_sim_equivalence.c @@ -195,9 +195,9 @@ int sched_fill_limit_requests(xce_scheduler_t* sched, xvec_idval_t* vec, * a no-op stand-in, same rationale test_record_lifecycle.c's own copy * documents (real resource accounting is scheduler.c/resource.c territory, * not exercised here). */ -int sched_account_jobs(xce_scheduler_t* sched) +int sched_account_jobs(xce_sched_core_t* core) { - (void)sched; + (void)core; return 0; }