test(scheduler-sim-eager-reconciliation): task 2.1 resInUse deferral proof

Adds test_captured_pend_job_excluded_from_resinuse_until_run: drives a
job captured while still PEND through sched_account_jobs() and confirms
its resource-in-use contribution is zero; transitions the same job to
RUN and confirms sched_account_jobs() now includes its exact captured
allocation, with no second reconciliation call in between -- proving the
nonzero value comes from the earlier capture, not a fresh one.

Required promoting the file's sched_account_jobs() from a no-op stub to
a real reimplementation (verbatim body, matching the exact precedent
already established in test_sim_contention_parity.c and
test_sim_round_boundary_reconciliation.c) plus a resource.c white-box
include -- no production code touched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
Rongsong Shen 2026-08-27 04:15:16 +00:00
parent e2bb98c87e
commit 0893fd50af
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 272 additions and 22 deletions

View file

@ -18,7 +18,7 @@
- _Boundary: sim_reconcile_unaccounted_jobs_
- [ ] 2. Integration: prove captured-but-not-running allocations stay excluded from resource-in-use accounting until the job actually runs
- [ ] 2.1 Verify resource-in-use accounting defers to the job's real running state, not its capture time
- [x] 2.1 Verify resource-in-use accounting defers to the job's real running state, not its capture time
- Drive a captured, not-yet-running job's allocation through the resource-in-use recomputation and confirm it contributes nothing to worker or shared resource usage
- Transition that same job to a running state and drive resource-in-use recomputation again, confirming its allocation now contributes from that point forward
- Observable: a test demonstrates the same captured job's resource usage is zero before its running-state transition and correctly non-zero after, using the exact allocation captured earlier

View file

@ -2,7 +2,12 @@
* test_sim_reconcile_unaccounted.c - contract test for
* sim_reconcile_unaccounted_jobs() (sim_loop.c, scheduler-sim-decision-
* parity spec, tasks 2.2-2.4, "Recorded-Decision Reconciliation" component,
* design.md; Requirement 1.1, 1.2, 3.1, 3.2, 3.3, 4.1, 4.2).
* design.md; Requirement 1.1, 1.2, 3.1, 3.2, 3.3, 4.1, 4.2). Also covers
* scheduler-sim-eager-reconciliation task 2.1 (Requirement 1.2, 1.3, 1.4):
* resource-in-use accounting via the REAL sched_account_jobs() (scheduler.c,
* unchanged by that spec, verbatim-copied in below -- see this file's own
* "White-box #include set" paragraph) correctly defers to a captured job's
* REAL running state, not its (now possibly much earlier) capture time.
*
* Scope: the once-per-round trigger (RUN-state job with job->accounts ==
* NULL), the jobid-based search through this round's
@ -22,13 +27,25 @@
* sched_sim_driver_run()'s round-boundary sequence) -- called directly here
* instead, standalone, per design.md's own Testing Strategy.
*
* White-box #include set and stub set: IDENTICAL to
* White-box #include set and stub set: originally IDENTICAL to
* test_sim_event_loop.c's own (same rationale -- sim_loop.c's compiled TU
* unconditionally references every one of these as externs, whether or not
* any single test here calls sched_sim_driver_run() itself). This file
* never calls sched_sim_driver_run()/spawns the picker thread, so (unlike
* test_sim_event_loop.c) no `static` storage workaround is needed for its
* own `xce_sched_sim_driver_t` fixtures.
* own `xce_sched_sim_driver_t` fixtures. Task 2.1 (scheduler-sim-eager-
* reconciliation) additionally #includes ../src/resource.c and replaces the
* PREVIOUS no-op sched_account_jobs() stub with a REAL reimplementation
* (body copied verbatim from scheduler.c, same precedent test_sim_
* contention_parity.c's own copy documents -- scheduler.c itself is never
* white-box #included in this directory, too large a link-time dependency
* surface, see test_resource_accounting.c's own header comment) -- this
* file's own task 2.1 test is the first test here to actually call that
* function, and a no-op stub would defeat its entire premise. Pulling in
* resource.c makes sched_worker_release_host_ctx() a REAL (non-stub) symbol
* too (previously a local no-op here) and adds one new link-satisfying-only
* stub, sched_ctx_get_uncomit_host() -- see the stub definitions below for
* both.
*
* Fixture technique: xce_sched_core_init() plus core_build_job()/
* sched_add_job() (core_events.c/core.c, both white-box #included) to place
@ -54,6 +71,7 @@
#include "controller.capnp.h"
#include "kvmgr.h"
#include "resobj.h"
#include "scheduler.h"
#include "xstring.h"
@ -73,6 +91,7 @@
#include "../src/repl_break.c"
#include "../src/repl_info.c"
#include "../src/repl_params.c"
#include "../src/resource.c"
#include "../src/sim.c"
#include "../src/sim_loop.c"
#include "../src/sim_picker.c" /* bug-1567: sim_loop.c calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */
@ -89,31 +108,84 @@ 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; }
/* core_reset_workers()'s (core_session.c) linker dependency -- never
* invoked (no worker is ever added here). */
void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; }
/* sched_worker_release_host_ctx() is now a REAL (non-stub) symbol pulled in
* from resource.c (task 2.1 addition, below) -- no local stand-in needed or
* permitted (a local definition here would collide at link time). Unused by
* any test in this file (no worker is ever released), listed only because
* resource.c defines it -- same precedent test_resource_accounting.c's own
* header comment documents. */
/* plugin_prepare_allocation()'s (plugin.c, the "limit" plugin category)
* linker dependency -- never invoked (this file never drives real
* allocation). */
int sched_fill_limit_requests(xce_scheduler_t* sched, xvec_idval_t* vec,
xlist_t* limits)
/* sched_fill_limit_requests() (plugin_prepare_allocation()'s, plugin.c, the
* "limit" plugin category, linker dependency) is likewise now a REAL
* (non-stub) symbol pulled in from resource.c -- no local stand-in needed
* or permitted (a local definition here would collide at link time, same
* as sched_worker_release_host_ctx() above). Never invoked by any test in
* this file (no real allocation is ever driven), listed only because
* resource.c defines it. */
/* resource.c's sched_try_alloc_on_host_()'s own link-time dependency --
* never actually invoked (this file never drives real allocation, only
* calls sched_account_jobs()/sched_charge_account() directly). Same
* link-satisfying-only stub test_sim_contention_parity.c's own copy
* establishes needing once resource.c is white-box #included. */
int sched_ctx_get_uncomit_host(xvec_t** v, xce_sched_ctx_t* c, int i)
{
(void)sched;
(void)vec;
(void)limits;
(void)v;
(void)c;
(void)i;
return 0;
}
/* sched_sim_driver_run()'s (sim_loop.c) round-boundary call to the
* relocated sched_account_jobs() (core.h) -- a no-op stand-in here since
* this file does not link scheduler.c/resource.c and never calls
* sched_sim_driver_run() itself; needed purely to satisfy the linker
* (sim_loop.c's compiled TU references it unconditionally). */
/* Task 2.1 addition (scheduler-sim-eager-reconciliation, Req 1.2/1.3): a
* REAL (not no-op) reimplementation of scheduler.c's own sched_account_jobs()
* (scheduler.c:1258, scheduler-sim-decision-parity task 1.1's relocated
* xce_sched_core_t* signature) -- body copied verbatim, same precedent
* test_sim_contention_parity.c's own copy of this function documents
* (scheduler.c itself is never white-box #included in this directory --
* ~1400 lines, too large a link-time dependency surface, see
* test_resource_accounting.c's own header comment). This file's task 2.1
* test exists specifically to observe what THIS function does to
* w->resInUse for a captured-but-not-yet-RUN job vs. the same job once it
* transitions to RUN -- a no-op stub would defeat that test's entire
* premise, exactly like test_sim_contention_parity.c's own rationale for
* needing the real body. This supersedes the file's previous no-op stand-in
* (task 1.1 and earlier tests here never called this function themselves,
* so the no-op was harmless for them; task 2.1 is the first test in this
* file to call it). */
int sched_account_jobs(xce_sched_core_t* core)
{
(void)core;
return 0;
iterator_t it;
int rc = 0;
rc = iter_make(&it, core->jobs);
if (rc < 0) {
return rc;
}
for (;;) {
xce_sched_job_t* job = NULL;
rc = iter_next(&it, (void**)&job);
if ((rc < 0) || (rc == RTTI_ITER_NO_DATA)) {
break;
}
if (!JOB_IS_RUN(job->state)) {
continue;
}
if (job->accounts != NULL) {
sched_charge_account(core, job->accounts);
}
}
if (rc == RTTI_ITER_NO_DATA) {
rc = 0;
}
iter_destroy(&it);
return rc;
}
/* Real reimplementation of schedobj.c's sched_resolve_job_picker -- same
@ -303,6 +375,69 @@ static replay_decision_t* make_dispatch_entry_from_alloc(xce_job_alloc_t* alloc)
return rd;
}
/* Task 2.1 fixture helpers (scheduler-sim-eager-reconciliation, Req
* 1.2/1.3): register a real, consumable "slots" resource and add a real
* worker owning a finite amount of it -- same technique
* test_sim_contention_parity.c's own register_slots_resource()/add_worker()
* establish (that file's own header comment documents why the resource must
* be pre-registered: reconciliation's real make_job_account()/
* make_res_account() path always produces residx == -1, lazily resolved by
* sched_update_res_accounts()'s own key_find(core->resmgr, ...) against
* whatever the resource was registered as -- pre-registering here means
* this fixture's own worker resvals/resInUse setup and the reconciled
* job's eventual account both resolve to the SAME index). */
static int register_slots_resource(xce_sched_core_t* core)
{
xce_resource_t* res_template
= (xce_resource_t*)calloc(1, sizeof(*res_template));
xce_resource_t* registered;
TEST_ASSERT_NOT_NULL(res_template);
res_template->name = strdup("slots");
res_template->valtype = XCE_VAL_NUMERIC;
res_template->ordtype = XCE_ORD_DECR;
res_template->flags = XCE_RESF_LIMIT | XCE_RESF_CONSUMABLE
| XCE_RESF_RELEASE | XCE_RESF_DEFAULT_HOST;
registered = makeResource(core->resmgr, res_template);
TEST_ASSERT_NOT_NULL(registered);
return registered->idx;
}
/* Adds a real xce_sched_worker_t owning `capacity` units of `residx`, added
* directly via sched_add_worker() -- sched_add_worker() itself does not
* initialize resvals/resInUse, callers own it (same convention every
* worker-adding fixture in this directory establishes). resInUse starts
* genuinely empty (zarray_init only, nothing put()) -- Requirement 1.2's
* own "still zero" checkpoint below depends on this starting state, not on
* an explicit zero being written. */
static xce_sched_worker_t* add_worker(xce_sched_core_t* core, const char* name,
int residx, float capacity)
{
xce_sched_worker_t* w;
int rc;
float val[1] = { capacity };
w = (xce_sched_worker_t*)calloc(1, sizeof(*w));
TEST_ASSERT_NOT_NULL(w);
w->name = strdup(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 = put_resvals(&(w->resvals), 1, &residx, val);
TEST_ASSERT_TRUE(rc >= 0);
rc = sched_add_worker(core, w);
TEST_ASSERT_TRUE(rc >= 0);
return w;
}
/* ---------------------------------------------------------------------- */
/* Test 1 (the task's own primary Observable): a RUN-state job with no */
/* known allocation and no matching recorded decision in this round's */
@ -853,6 +988,120 @@ static void test_reconcile_null_args_are_a_safe_noop(void)
printf("test_reconcile_null_args_are_a_safe_noop: PASS\n");
}
/* ---------------------------------------------------------------------- */
/* Test 12 (scheduler-sim-eager-reconciliation task 2.1, Req 1.2, 1.3, */
/* 1.4): resource-in-use accounting must defer to the job's REAL running */
/* state, not its capture time. task 1.1 widened sim_reconcile_unaccounted_ */
/* jobs()'s trigger so a job's allocation is captured (job->allocs/ */
/* job->accounts set) as soon as its recorded DISPATCH decision is found, */
/* regardless of job->state -- this test proves the OTHER half of that */
/* design's correctness: sched_account_jobs() (scheduler.c, UNCHANGED, */
/* real body pulled in above) still gates charging on JOB_IS_RUN(state), */
/* so a captured-but-still-PEND job contributes nothing to w->resInUse */
/* (Req 1.2) until it genuinely transitions to RUN, at which point the */
/* SAME captured allocation (never re-captured, never re-typed) is charged */
/* (Req 1.3). Req 1.4 ("matches normal-mode's equivalent-point reporting") */
/* is satisfied by construction: this test drives the exact same */
/* sched_account_jobs() normal mode's own driver.c calls at the identical */
/* round-boundary position (design.md's Architecture Integration note) -- */
/* no separate normal-mode comparison fixture is needed to prove a shared */
/* function behaves consistently with itself. */
/* ---------------------------------------------------------------------- */
static void
test_captured_pend_job_excluded_from_resinuse_until_run(void)
{
xce_sched_sim_driver_t sim;
xce_sched_job_t* job;
xce_sched_worker_t* w;
xce_job_alloc_t* jalloc;
replay_session_t* decoded;
int residx;
float got;
int rc;
fixture_init(&sim);
residx = register_slots_resource(&(sim.core));
w = add_worker(&(sim.core), "worker1", residx, 10.0f);
/* PEND-state job, no known allocation yet -- exactly task 1.1's own
* widened trigger shape: its recorded DISPATCH decision has been found
* this round, but its RUN-state confirmation has not been replayed
* yet. */
job = add_job(&(sim.core), 7030ULL, JOB_STAT_PEND, NULL);
TEST_ASSERT_NULL(job->accounts);
TEST_ASSERT_NULL(job->allocs);
jalloc = make_test_job_alloc_with_host(7030ULL, "worker1", "slots", 4.0f);
decoded = make_decoded_session(1000ULL,
make_dispatch_entry_from_alloc(jalloc));
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
TEST_ASSERT_EQUAL_PTR_MESSAGE(
jalloc, job->allocs,
"sanity: the PEND job's recorded DISPATCH decision must be captured "
"(task 1.1's own widened gate) before this test's own checkpoints "
"mean anything");
TEST_ASSERT_NOT_NULL_MESSAGE(
job->accounts, "sanity: job->accounts must be built from the "
"captured allocation even though the job is still "
"PEND");
TEST_ASSERT_TRUE_MESSAGE(
JOB_STAT_PEND == job->state,
"sanity: the job must still genuinely be PEND at this checkpoint -- "
"otherwise the next assertion (resInUse == 0) would be trivially "
"true for the wrong reason");
/* Checkpoint 1 (Requirement 1.2): recompute resInUse from scratch via
* the REAL sched_account_jobs() -- a captured-but-PEND job must
* contribute nothing. */
rc = sched_account_jobs(&(sim.core));
TEST_ASSERT_EQUAL_INT(0, rc);
got = -1.0f;
rc = get_resvals(&(w->resInUse), 1, &residx, &got);
TEST_ASSERT_EQUAL_INT(1, rc);
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
0.0f, got,
"Requirement 1.2: a job whose allocation was captured early (while "
"still PEND) must contribute ZERO to its worker's resInUse until it "
"genuinely transitions to RUN -- sched_account_jobs()'s own "
"JOB_IS_RUN gate (scheduler.c, unchanged by this feature) is the "
"sole mechanism deferring this, and must still be effective now "
"that capture happens earlier than before");
/* Transition the SAME job to RUN -- direct field write, same white-box
* technique test_sim_contention_parity.c's own capacity_aware_
* allocator_thread_fn() uses to model a worker-confirmed state change
* without driving the full plugin_job_state_change() machinery (out of
* this file's own boundary -- scheduler.c is not white-box included). */
job->state = JOB_STAT_RUN;
/* Checkpoint 2 (Requirement 1.3): recompute resInUse again -- the SAME
* captured allocation (jalloc, never re-captured -- sim_reconcile_
* unaccounted_jobs() is not called a second time here, proving this
* checkpoint's nonzero value comes from the EARLIER capture, not a
* fresh one) must now be charged. */
rc = sched_account_jobs(&(sim.core));
TEST_ASSERT_EQUAL_INT(0, rc);
got = -1.0f;
rc = get_resvals(&(w->resInUse), 1, &residx, &got);
TEST_ASSERT_EQUAL_INT(1, rc);
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
4.0f, got,
"Requirement 1.3: once the captured-early job genuinely transitions "
"to RUN, its worker's resInUse must reflect its allocation from "
"that point forward -- using the EXACT allocation amount (4.0) "
"captured earlier by sim_reconcile_unaccounted_jobs(), not a "
"freshly re-captured or re-typed value");
printf(
"test_captured_pend_job_excluded_from_resinuse_until_run: PASS\n");
}
int main(void)
{
UNITY_BEGIN();
@ -870,5 +1119,6 @@ int main(void)
RUN_TEST(
test_reconcile_reports_reconstructed_decision_through_existing_channel);
RUN_TEST(test_reconcile_null_args_are_a_safe_noop);
RUN_TEST(test_captured_pend_job_excluded_from_resinuse_until_run);
return UNITY_END();
}