lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(scheduler-sim-live-allocation): narrow reconciliation to the timing-gap safety net (task 4.2)

Browse source 
Adds a plugin_job_election(&(sim->core), job) call to
sim_reconcile_unaccounted_jobs()'s FOUND branch, before the existing
alloc-reconstruction/state-transition code, so a job whose outcome
comes from the recording still gets its global-limit usage registered
-- the same core-safe dispatcher the live path (task 4.1) already
uses, not a separate mechanism.

The existing job->accounts != NULL trigger guard needs no change: it
already excludes any job the live path has already decided, so this
call is naturally idempotent (match_job_global_limit_'s own
obj->configver guard makes a redundant call a safe no-op) -- narrowing
this function to a genuine timing-gap safety net rather than the
general-case decision source it used to be, per design's Live/Recorded
Authority Rule.

Host-scoped limit matching is deliberately not attempted here (needs a
resolved worker context this function doesn't independently
construct) -- that gap is tracked separately as task 4.3.

TDD: new tests prove a reconciled job's global limit is genuinely
registered via the real limit_get_usage_instances() accessor, and that
a job the live path already decided is never reprocessed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-19 05:57:55 +00:00
parent 08df64930b
commit 915782fc66
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
4 changed files with 329 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.kiro/specs/scheduler-sim-live-allocation/tasks.md
View file

@ -76,7 +76,7 @@
- _Requirements: 1.1, 1.2, 3.3, 4.2, 4.3, 5.1_
- _Boundary: Unified Picker/Allocator Threads, Sim Driver_
- [ ] 4.2 Narrow recorded-decision reconciliation to the timing-gap safety net
- [x] 4.2 Narrow recorded-decision reconciliation to the timing-gap safety net
- Add a call to the now-driver-agnostic job-election dispatch (from 1.2) for a job this mechanism reconstructs from the recording, so a job whose outcome comes from the recording still gets its global-limit usage registered
- Confirm the existing trigger guard (a job already decided is never reconsidered) needs no change, since it already excludes any job the live path from 4.1 has already handled
- TDD: extend the existing reconciliation test suite with a case confirming a reconciled job's global limit is registered, and a case confirming a job the live path already decided is never reprocessed by this mechanism

24
src/scheduler/src/sim_loop.c
View file

@ -732,6 +732,30 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
continue;
}
/* FOUND -- task 4.2 (scheduler-sim-live-allocation spec,
* "Sim-Mode Reconciliation Narrowing" component): elect the job
* through the same core-safe plugin_job_election() dispatch
* (task 1.2) sched_allocator() calls for a job the live path
* decides directly (sched_election.c), so a job whose outcome
* this mechanism reconstructs from the recording still gets its
* GLOBAL (non-host-scoped) limit usage registered --
* match_job_global_limit_()'s own idempotency guard
* (`obj->configver >= core.configver`, plugins/limit.c) makes
* this safe to call even if the live path also happens to reach
* this same job (it cannot: this branch's own trigger,
* `job->accounts == NULL`, already excludes any job the live
* path has decided -- see this function's own header comment).
* Deliberately GLOBAL-limit-only, not host-scoped: host-scoped
* matching (match_job_host_limit_(), via prepare_allocation_
* hook) needs a resolved worker/ctx this reconstruction path
* does not independently build -- design.md's own text for this
* component explains why that is a structurally different
* problem, not one this call can also close. Called before the
* allocation reconstruction below (mirrors sched_allocator()'s
* own ordering: election, then allocation), though nothing
* downstream of it reads job->allocs/accounts. */
plugin_job_election(&(sim->core), job);
/* FOUND -- task 2.3: reconstruct job->allocs/job->accounts by
* TAKING OVER `entry`'s allocation, the same ownership-transfer
* pattern scheduler.c's sched_commit_decision() already uses

348
src/scheduler/testsuite/test_sim_reconcile_unaccounted.c
View file

@ -114,6 +114,19 @@
#include "../src/decision.c"
#include "../src/plugin.c"
#include "../src/plugins/builtin_fcfs.c"
#include "../src/plugins/limit.c" /* task 4.2 (scheduler-sim-live-allocation
* spec): pulled in for REAL (same file test_repl_limit_table.c's own
* fixtures already white-box #include) so this file's reconciliation tests
* can prove sim_reconcile_unaccounted_jobs()'s new plugin_job_election()
* call (sim_loop.c) genuinely registers a reconciled job's GLOBAL limit
* usage -- a stub returning NULL from limit_new_plugin() (this file's own
* previous stand-in, removed below) would defeat that test's entire
* premise. This supersedes the four link-satisfying stubs immediately
* below this include block in earlier revisions of this file
* (limit_get_configured_limits()/limit_get_usage_instances()/
* limit_new_plugin()/limit_seed_config()), which are now real symbols
* defined by this include and would collide at link time if left in
* place. */
#include "../src/recording.c"
#include "../src/recording_session.c"
#include "../src/repl.c"
@ -140,68 +153,16 @@ 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; }
/* repl_info.c (scheduler-sim-diagnostics spec, task 2.1) now calls
* limit_get_configured_limits() (plugins/limit.c, not part of this TU) to
* resolve `info limit`'s name/all argument -- link-satisfying stub only,
* same rationale as kill_sched_job()/kill_sched_resval() above; this
* file's own fixtures never register the limit plugin, so NULL
* (limit_get_configured_limits()'s own documented "plugin absent" return)
* is also the behaviorally correct answer here, not just a link
* placeholder. repl_info_limit() itself is not yet wired into
* repl_cmd_info() (task 4), so no test in this file can reach it anyway. */
var limit_get_configured_limits(xce_sched_core_t* core)
{
(void)core;
return NULL;
}
/* scheduler-sim-diagnostics spec, task 3.1: repl_info.c now also calls
* limit_get_usage_instances() (plugins/limit.c, not part of this TU) to
* read a limit's live usage instances -- identical link-satisfying stub,
* same rationale as limit_get_configured_limits() immediately above; this
* file's own fixtures never register the limit plugin, so NULL
* (limit_get_usage_instances()'s own documented "plugin absent" return) is
* also the behaviorally correct answer here, not just a link placeholder. */
var limit_get_usage_instances(xce_sched_core_t* core)
{
(void)core;
return NULL;
}
/* scheduler-sim-diagnostics spec, task 6 (bug-1825): sim_loop.c's
* sched_sim_driver_run() now also calls sim_bootstrap_limit_plugin()
* (sim_limit.c, pulled into this TU via the #include above), which in
* turn calls limit_new_plugin() (plugins/limit.c, not part of this TU) --
* identical link-satisfying stub rationale as limit_get_configured_
* limits()/limit_get_usage_instances() immediately above. Returning NULL
* here is also the behaviorally correct answer, not just a link
* placeholder: sim_bootstrap_limit_plugin() already handles a NULL
* limit_new_plugin() return gracefully (logs an ERROR, registers nothing,
* returns) -- the exact same "out of memory" path sim_bootstrap_picker()
* (sim_picker.c) takes for a NULL builtin_fcfs_new_plugin() -- so this
* file's fixtures simply end up with no limit plugin registered, same
* observable state as before this task. */
xce_sched_plugin_t* limit_new_plugin(void)
{
return NULL;
}
/* limit_seed_config() (plugins/limit.c, not part of this TU) -- bug-1825
* remediation round 2's new honestly-typed config-seed entry point.
* sim_bootstrap_limit_plugin() (sim_limit.c) only calls it after
* plugin_register_core() has already succeeded against a real, non-NULL
* limit_new_plugin() plugin -- unreachable in this file since the
* limit_new_plugin() stub immediately above always returns NULL, but the
* symbol must still resolve at link time. Same link-satisfying-stub
* rationale as limit_new_plugin() immediately above. */
int limit_seed_config(xce_sched_plugin_t* plugin, xce_sched_core_t* core,
xce_conf4sched_t* config)
{
(void)plugin;
(void)core;
(void)config;
return 0;
}
/* task 4.2 (scheduler-sim-live-allocation spec): limit_get_configured_
* limits()/limit_get_usage_instances()/limit_new_plugin()/limit_seed_
* config() are now REAL symbols, pulled in for real by the "../src/plugins/
* limit.c" white-box #include above -- the four link-satisfying stubs that
* previously stood in for them here (each returning NULL/0, since this
* file's fixtures never registered a real limit plugin) are removed, not
* left alongside the real definitions, which would collide at link time.
* sim_bootstrap_limit_plugin() (sim_limit.c, already included) now
* genuinely registers the limit plugin exactly as production does when a
* test's fixture calls it, which the new task 4.2 tests below rely on. */
/* 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
@ -231,6 +192,23 @@ int sched_ctx_get_uncomit_host(xvec_t** v, xce_sched_ctx_t* c, int i)
return 0;
}
/* Task 4.2 (scheduler-sim-live-allocation spec): plugins/limit.c's
* limit_job_election() calls reset_sched_obj() (../src/schedobj.c, not part
* of this white-box TU -- schedobj.c is not included here) -- a genuine
* link-time dependency of limit.c, not previously reachable in this file
* before this task's #include "../src/plugins/limit.c" addition above.
* Stubbed as a no-op purely to satisfy the linker, same precedent
* test_repl_limit_table.c's own identical stub establishes for the same
* symbol/reason. This is behaviorally correct here, not just a link
* placeholder: every job this file's fixtures build goes through the real
* core_build_job() (core_events.c, white-box #included above), which
* already xlist_init()s/zarray_init()s obj->glimits/hlimits fresh and
* empty -- the real reset_sched_obj() would only ever clear an EARLIER
* match's leftovers, which no job in this file's fixtures ever has (each
* test's job is freshly built and matched against its limit at most
* once), so skipping that clear changes nothing observable here. */
void reset_sched_obj(xce_sched_obj_t* obj) { (void)obj; }
/* 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
@ -2304,6 +2282,249 @@ test_reconcile_removes_reconciled_job_from_picker_candidates(void)
"PASS\n");
}
/* ---------------------------------------------------------------------- */
/* Task 4.2 fixtures (scheduler-sim-live-allocation spec, "Sim-Mode */
/* Reconciliation Narrowing" component, Requirement 3.4/3.5): register a */
/* real, core-safe limit plugin via the SAME production bootstrap path */
/* sched_sim_driver_run() itself uses (sim_bootstrap_limit_plugin(), */
/* sim_limit.c, bug-1825), mirroring test_repl_limit_table.c's own */
/* test_limit_job_election_matches_global_limit_via_sim_registered_ */
/* plugin() fixture technique -- the same resource/limit-config shape, */
/* duplicated here per this file family's own "duplicate, don't share" */
/* convention (this file's header comment) rather than sharing statics */
/* across translation units. */
/* ---------------------------------------------------------------------- */
/* Registers a plain, non-host-scoped "mem" resource directly into `core`'s
* real resmgr -- is_host_based_limit() (src/lib/xcelimit.c) key_find()s a
* limit's val.resname directly against core->resmgr, so this fixture must
* exist for a GLOBAL (non-host-scoped) limit to resolve and match at all;
* same rationale/shape test_repl_limit_table.c's own
* register_per_host_resource() documents (that name describes the
* resource's own per-host consumption shape, not the limit's scope --
* flags carry no host-scope indicator, so is_host_based_limit() falls back
* to GLOBAL for a limit against it, exactly what these two tests need). */
static xce_resource_t* register_mem_resource(xce_sched_core_t* core)
{
xce_resource_t wire_resource;
xce_resource_t* r;
memset(&wire_resource, 0, sizeof(wire_resource));
wire_resource.name = (char*)"mem";
wire_resource.valtype = XCE_VAL_NUMERIC;
wire_resource.flags = XCE_RESF_CONSUMABLE;
r = makeResource(core->resmgr, &wire_resource);
TEST_ASSERT_NOT_NULL(r);
return r;
}
/* A minimal, heap-allocated xce_limit_config_t against the "mem" resource
* registered above -- identical shape test_repl_limit_table.c's own
* make_limit_config() documents (no per-consumer groups, so build_limit()'s
* build_consumer() falls back to get_group(mgr, "all") for every
* consumer). `name` is expected to be a string literal (build_limit()
* strdup()s it). */
static xce_limit_config_t* make_global_limit_config(const char* name)
{
xce_limit_config_t* cfg = (xce_limit_config_t*)calloc(1, sizeof(*cfg));
xce_limit_val_t* val = (xce_limit_val_t*)calloc(1, sizeof(*val));
TEST_ASSERT_NOT_NULL(cfg);
TEST_ASSERT_NOT_NULL(val);
val->resname = "mem";
val->percent = 0;
val->val = 10.0;
cfg->name = (char*)name;
cfg->val = val;
return cfg;
}
/* Bootstraps a real, core-safe limit plugin (production path:
* plugin_register_core() + limit_seed_config(), via sim_bootstrap_limit_
* plugin() -- sim_limit.c, bug-1825) against `sim`, seeded with one GLOBAL
* limit named `limit_name`, and bumps core->configver past the fresh
* xce_sched_obj_t default of 0 -- limit_job_election()'s own "already
* evaluated against this config" guard (obj->configver >= core->configver)
* would otherwise skip matching entirely, since both default to 0. Same
* ordering/rationale test_repl_limit_table.c's own
* test_limit_job_election_matches_global_limit_via_sim_registered_plugin()
* establishes. Returns the registered plugin (for callers that want to
* assert on it directly; both tests below only need the side effects). */
static xce_sched_plugin_t*
bootstrap_global_limit_plugin(xce_sched_sim_driver_t* sim,
const char* limit_name)
{
xce_sched_plugin_t* plugin = NULL;
xce_limit_config_t* limits[1];
xce_conf4sched_t config;
int rc;
TEST_ASSERT_NOT_NULL(register_mem_resource(&(sim->core)));
limits[0] = make_global_limit_config(limit_name);
memset(&config, 0, sizeof(config));
config.nlimits = 1;
config.limits = limits;
sim->core.config = &config;
sim_bootstrap_limit_plugin(sim);
rc = key_find(sim->core.plugins, "xce_sched_limit", (void**)&plugin);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_NOT_NULL(plugin);
sim->core.configver = 1;
return plugin;
}
/* ---------------------------------------------------------------------- */
/* Test (task 4.2's own PRIMARY Observable, Requirement 3.4): a job whose */
/* dispatch is found only via reconciliation still gets its GLOBAL (non- */
/* host-scoped) limit usage registered, through the same core-safe */
/* plugin_job_election() dispatch normal mode's own sched_allocator() calls */
/* (task 1.2) -- design.md's "Sim-Mode Reconciliation Narrowing" component. */
/* Host-scoped matching is deliberately OUT of scope for this call (design */
/* .md's own text: it needs a resolved worker context this function does */
/* not independently construct) -- this test only proves the GLOBAL path. */
/* ---------------------------------------------------------------------- */
static void
test_reconcile_registers_global_limit_usage_for_reconciled_job(void)
{
xce_sched_sim_driver_t sim;
xce_sched_job_t* job;
xce_job_alloc_t* jalloc;
replay_session_t* decoded;
var instances_handle;
iterator_t it;
void* item = NULL;
int rc;
fixture_init(&sim);
bootstrap_global_limit_plugin(&sim, "sim_reconcile_global_limit");
job = add_job(&(sim.core), 7090ULL, JOB_STAT_RUN, NULL);
TEST_ASSERT_NULL(job->accounts);
/* match_job_global_limit_() (plugins/limit.c) matches against
* job->schedobj->user/queue/project directly -- add_job()'s own
* core_build_job() path leaves ->project NULL (submit.project_name is
* never set by that shared fixture helper, and makeString(NULL)
* returns NULL, src/lib/xstring.c), which match_limit() cannot safely
* take. Set it directly here, scoped to this test only, rather than
* widening add_job() itself for every other test in this file that has
* no need of it. */
job->schedobj->project = (char*)"";
jalloc = make_test_job_alloc_with_host(7090ULL, "worker1", "mem", 2.0f);
decoded = make_decoded_session(1000ULL,
make_dispatch_entry_from_alloc(jalloc));
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
TEST_ASSERT_NOT_NULL_MESSAGE(
job->accounts,
"sanity: the job's dispatch must still be reconstructed exactly as "
"before this task");
instances_handle = limit_get_usage_instances(&(sim.core));
TEST_ASSERT_NOT_NULL_MESSAGE(
instances_handle,
"task 4.2: a job whose dispatch is found only via reconciliation "
"must still have its GLOBAL limit usage registered, via the same "
"core-safe plugin_job_election() dispatch normal mode uses");
rc = iter_make(&it, instances_handle);
TEST_ASSERT_TRUE(rc >= 0);
rc = iter_next(&it, &item);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_NOT_EQUAL(RTTI_ITER_NO_DATA, rc);
TEST_ASSERT_NOT_NULL_MESSAGE(
item, "a real usage instance must exist for the reconciled job's "
"global limit match, visible through the existing "
"limit_get_usage_instances() accessor -- the same one `info "
"limit` reads");
printf("test_reconcile_registers_global_limit_usage_for_reconciled_job: "
"PASS\n");
}
/* ---------------------------------------------------------------------- */
/* Test (task 4.2, Requirement 3.5's supporting guard): a job the LIVE path */
/* has already decided (job->accounts != NULL, as task 4.1's live picker/ */
/* allocator would leave it) is never reprocessed by this mechanism -- the */
/* existing trigger guard (job->accounts == NULL) already excludes it */
/* BEFORE this task's new plugin_job_election() call is ever reached, so no */
/* limit usage is (re-)registered and the job's own schedobj->configver */
/* stays untouched. Extends test_reconcile_skips_job_that_already_has_ */
/* accounts (Test 3 above), which already proves job->accounts/n_decisions */
/* are untouched for this shape -- this test proves the NEW call site */
/* respects that same exclusion. */
/* ---------------------------------------------------------------------- */
static void
test_reconcile_does_not_reregister_limit_for_already_decided_job(void)
{
xce_sched_sim_driver_t sim;
xce_sched_job_t* job;
xce_job_account_t* accounts_before;
replay_session_t* decoded;
var instances_handle;
iterator_t it;
void* item = NULL;
int rc;
fixture_init(&sim);
bootstrap_global_limit_plugin(&sim, "sim_reconcile_already_decided_limit");
/* Job already decided by the LIVE path (task 4.1) -- job->accounts !=
* NULL, the same "already accounted" fixture shape Test 3
* (test_reconcile_skips_job_that_already_has_accounts) above
* establishes. */
job = add_job(&(sim.core), 7091ULL, JOB_STAT_RUN,
make_job_alloc(7091ULL, 0, NULL));
TEST_ASSERT_NOT_NULL_MESSAGE(
job->accounts,
"fixture setup: job must already have accounts, as if the live "
"path already decided it");
accounts_before = job->accounts;
job->schedobj->project = (char*)"";
decoded = make_decoded_session(1000ULL, make_dispatch_entry(7091ULL));
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
TEST_ASSERT_EQUAL_PTR_MESSAGE(
accounts_before, job->accounts,
"an already-decided job must never be re-touched by reconciliation "
"(unchanged, pre-existing guard)");
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, job->schedobj->configver,
"task 4.2: the existing trigger guard already excludes an "
"already-decided job before the new plugin_job_election() call is "
"reached -- its schedobj->configver (bumped by a real election "
"call) must stay at its untouched default");
instances_handle = limit_get_usage_instances(&(sim.core));
TEST_ASSERT_NOT_NULL(instances_handle);
rc = iter_make(&it, instances_handle);
TEST_ASSERT_TRUE(rc >= 0);
rc = iter_next(&it, &item);
TEST_ASSERT_EQUAL_INT_MESSAGE(
RTTI_ITER_NO_DATA, rc,
"no limit usage instance may be created for an already-decided "
"job via this mechanism -- plugin_job_election() must never reach "
"it");
printf("test_reconcile_does_not_reregister_limit_for_already_decided_"
"job: PASS\n");
}
int main(void)
{
UNITY_BEGIN();
@ -2338,5 +2559,8 @@ int main(void)
RUN_TEST(
test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch);
RUN_TEST(test_reconcile_removes_reconciled_job_from_picker_candidates);
RUN_TEST(test_reconcile_registers_global_limit_usage_for_reconciled_job);
RUN_TEST(
test_reconcile_does_not_reregister_limit_for_already_decided_job);
return UNITY_END();
}

18
src/scheduler/testsuite/test_sim_session_ordering.c
View file

@ -256,6 +256,24 @@ int sched_fill_limit_requests(xce_sched_alloc_req_t* alloc_req,
return 0;
}
/* Task 4.2 (scheduler-sim-live-allocation spec): sim_reconcile_unaccounted_
* jobs() (sim_loop.c, white-box #included above) now also calls
* plugin_job_election() (plugin.c, not part of this TU -- this file does
* not include plugin.c) for a job it reconstructs from the recording, so a
* genuine reconciliation round in this file's own fixtures now reaches it
* for the first time -- link-satisfying stub only, same rationale as
* sched_fill_limit_requests() above. A no-op is also the behaviorally
* correct answer here: this file's own fixtures test round-boundary
* ordering (the plugin_begin_session()/plugin_end_session() spies below),
* never limit-matching, and no test here inspects any effect
* plugin_job_election() would have. */
int plugin_job_election(xce_sched_core_t* core, xce_sched_job_t* job)
{
(void)core;
(void)job;
return 0;
}
/* xce_sched_driver_on_decision()'s (driver.c) publish-side linker
* dependencies -- never actually invoked (this test never registers a
* decision callback). Same rationale test_sim_equivalence.c's own copy