diff --git a/.wolf/buglog.json b/.wolf/buglog.json index e05f6013..9408353c 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -25082,9 +25082,9 @@ "id": "bug-1846", "timestamp": "2026-09-23T10:05:00Z", "error_message": "OPEN, NOT YET ROOT-CAUSED. Same replay session as bug-1844/1845 (scheduler-replay-20260923-085356.replay): after excluding instid (bug-1844) and reason-detail (bug-1845), --sim-verify still shows one job (jobid=0-8-1790154325-0[39] in the observed capture) genuinely DISPATCHING one round earlier in replay than in the original live recording -- not a false-positive field comparison, a real positional divergence: replay's round begin_round=...436582151823 dispatches a 13-job batch (jobs [27]..[39]) while the SAME round in the original recording only dispatched 12 (jobs [27]..[38]); job [39]'s recorded dispatch instead lands in the NEXT round, cascading into ~40+ downstream positional MISMATCH lines for that round pair alone.", - "file": "src/scheduler/src/resource.c (sched_account_jobs, sched_charge_account/sched_uncharge_account), src/scheduler/src/sim_loop.c (sim_reconcile_unaccounted_jobs, sim_reconcile_toggle_prior_charges) -- suspected, not confirmed", - "root_cause": "NOT YET ROOT-CAUSED. Ruled out as explanations: (1) sim_loop.c's event-buffer-then-push-before-begin_session mechanism -- verified byte-for-byte faithful to the recorder's own file ordering during the bug-1843 investigation, no live-vs-replay discretization gap there; (2) the unsatisfiable-cache (alloc_cache.c) -- verified sound within a round (see bug-1845's root_cause) and, being purely additive skip-logic on top of the SAME full computation replay always performs, cannot itself cause replay to admit an EXTRA job a full check would have rejected. Leading hypothesis (UNCONFIRMED): a resource-accounting (resInUse/sharedInUse) state divergence between the live recorder and replay by this point in the run -- the class of bug scheduler-driver-equivalence.md documents as bug-1611/bug-1613's lineage (accounting-state parity is load-bearing, not incidental, and invisible at the decision level until contention makes it decision-visible, exactly this scenario: a 12-vs-13-job batch is a marginal-capacity difference). Not yet instrumented: w->resInUse/core->sharedInUse for the specific worker/resource this job batch consumes, round-by-round, to find where (if at all) replay's accounting genuinely differs from what the recorded decisions imply the original live run's accounting held at the equivalent round. Note bug-1611/bug-1613 are independently documented as FIXED (scheduler-driver-equivalence.md's 2026-09-19 update, re-verified via /xce-audit-invariants scheduler) -- if this is real, it is either a NEW instance of the same bug class (e.g. an accounting gap in the newer sim_reconcile_unaccounted_jobs()/bug-1836 charge-peek logic, which postdates that fix) or a currently-undocumented edge case the 2026-09-19 re-verification's fixture didn't exercise.", - "fix": "NOT FIXED -- open, reported for follow-up. Next step: instrument w->resInUse (and core->sharedInUse if the relevant resource is shared) for the specific worker(s) this job batch targets, logged at the start of every round (after sched_account_jobs(), before xce_core_signal_go()) in sim_loop.c, replayed against this exact .replay file, to find the first round (if any) where replay's own accounting state provably differs from what the ORIGINAL recording's decision sequence implies live's accounting held. If confirmed, the fix belongs wherever driver.c and sim_loop.c's per-round accounting calls diverge (scheduler-driver-equivalence.md's own audit method: enumerate every non-core call driver.c's round makes, confirm sim_loop.c's round makes the equivalent one in the same window, and the reverse).", + "file": "src/scheduler/src/plugins/limit.c (match_job_host_limit_)", + "root_cause": "CONFIRMED (2026-09-23, via temporary per-decision diagnostics in the --sim-verify drain loop, since reverted) -- two distinct things. (1) ROOT-CAUSED + FIXED: match_job_host_limit_() (limit.c) runs on EVERY per-host allocation attempt (limit_prepare_allocation() via plugin_prepare_allocation()) and unconditionally xlist_store()d the matched vres into obj->hlimits[worker->idx]; obj->hlimits is only rebuilt by limit_job_election() on a configver bump. A job that kept pending on the same host accumulated one extra copy per attempt; sched_fill_limit_requests() staged every copy into ctx->alloc_req.host and seqjob's make_host_alloc() built a host alloc carrying all of them. In the captured replay the LIVE recording itself shows it (pc13v host allocs with mem+slots+70/79/94/112/136/175 identical limit=limits2/u=rshen/q=xce/p= entries, resetting only on a config push) -- i.e. a PRODUCTION bug, not sim-only. make_job_account() makes one res_account per entry and sched_update_res_accounts() charges each, so the host limit was charged (and later released) N-fold. Replay had the same accumulation with different counts, which is what --sim-verify flagged. (2) NOT ROOT-CAUSED, live-side: 9 recorded rounds (all containing DISPATCHes) end after only 11-12 decisions while replay processes the whole candidate list; the next recorded round then carries the leftovers. Ruled out: recorder growth/truncation (recording_session.c), a picker/round time budget (none exists; truncated rounds last 0.8-2.2s), fcfs readiness predicates (both return 1), the unsatisfiable-cache reason handoff (MOVE_PTR on both paths), queued controller updates (all dataq-enqueued). Remaining candidates: something live-only ending the fcfs session early (e.g. handler.c's synchronous config push -> sched_init_queues() -> on_queue_update() nulling priv->cursor mid-session, too rare to explain all 9), or sched_allocator's silent `alloc_rc < 0 -> continue` (no decision emitted) -- possibly related to (1)'s inflated limit charge. Needs the live scheduler.log for the capture window and/or a fresh capture from a fixed binary.", + "fix": "PARTIAL (2026-09-23, TDD). limit.c: new hlimits_contains_() pointer-identity check (vres is interned per name in privdata->instances) -- match_job_host_limit_() now stores a matched vres into obj->hlimits[worker] only once. New regression test test_limit_host_dedup.c (sched_limit_host_dedup_test, meson suite 'scheduler'): 5 plugin_prepare_allocation() attempts for one job on one worker must leave hlimits[worker]->count==1 and exactly one staged copy of the vres in alloc_req.host -- verified RED before the fix (Expected 1 Was 2 on attempt 2) and GREEN after. Replay re-run after the fix: replay-side host allocs are now always 3 entries; remaining DISPATCH alloc mismatches are the recording's own inflated entries (recorded with the buggy binary) and will disappear only with a fresh capture. Part (2) remains OPEN.", "tags": [ "scheduler", "sim", @@ -25095,7 +25095,12 @@ "driver-equivalence", "unfixed", "needs-diagnosis", - "open" + "open", + "limit", + "hlimits", + "host-limit", + "accounting", + "production" ], "related_bugs": [ "bug-1843", diff --git a/src/scheduler/src/plugins/limit.c b/src/scheduler/src/plugins/limit.c index f75e4167..43a597b4 100644 --- a/src/scheduler/src/plugins/limit.c +++ b/src/scheduler/src/plugins/limit.c @@ -381,6 +381,31 @@ static int limit_job_election(void* arg, xce_sched_job_t* job) return rc; } +/* + * hlimits_contains_ - bug-1846: match_job_host_limit_() runs on EVERY + * per-host allocation attempt (limit_prepare_allocation(), via + * plugin_prepare_allocation()), but obj->hlimits is only rebuilt when + * limit_job_election() sees a config-version bump. Storing the matched vres + * unconditionally therefore appended one more copy per attempt for a job + * that kept pending on the same host, and sched_fill_limit_requests() + * staged every copy into ctx->alloc_req.host -- a real captured replay + * showed a DISPATCH host alloc carrying 70 identical limit entries. A vres + * is interned per name in privdata->instances, so pointer identity is the + * right membership test. + */ +static int hlimits_contains_(xlist_t* hlimits, xce_vres_t* vres) +{ + int i; + + for (i = 0; i < hlimits->count; i++) { + if ((xce_vres_t*)xlist_get(hlimits, i) == vres) { + return 1; + } + } + + return 0; +} + static int match_job_host_limit_(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx, xce_limit_t* limit) { @@ -480,7 +505,7 @@ static int match_job_host_limit_(xce_sched_plugin_t* plugin, } rc = update_limit_res_value(&(worker->resvals), vres); - if (rc >= 0) { + if ((rc >= 0) && !hlimits_contains_(hlimits, vres)) { rc = xlist_store(hlimits, vres); } diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index cf98128a..f4349e75 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -2207,6 +2207,30 @@ test('scheduler sim contention limit driver parity test', sched_sim_contention_limit_driver_parity_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +# bug-1846: a host-scoped limit must be staged once per (job, worker) no +# matter how many allocation attempts the job makes there -- +# match_job_host_limit_() used to append a fresh copy to obj->hlimits on +# every attempt, inflating the DISPATCH host allocation (72 vs 3 entries in +# a real captured replay). Links every scheduler source except main.c, same +# as the driver-parity fixture above. +sched_limit_host_dedup_test_srcs = ['test_limit_host_dedup.c'] +foreach f : scheduler_srcs + if f != 'src/main.c' + sched_limit_host_dedup_test_srcs += ['../' + f] + endif +endforeach + +sched_limit_host_dedup_test = executable( + 'sched_limit_host_dedup_test', + sched_limit_host_dedup_test_srcs, + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + +test('scheduler limit host dedup test', + sched_limit_host_dedup_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) + # scheduler-sim-live-allocation spec, task 5.2 (Req 2.1, 2.2, 2.3, 2.4): the # feature's concrete, original acceptance test (bug-1826) -- a real global # AND host-scoped limit configured in a real SnapshotEntry's Conf4Sched, diff --git a/src/scheduler/testsuite/test_limit_host_dedup.c b/src/scheduler/testsuite/test_limit_host_dedup.c new file mode 100644 index 00000000..29a3f73a --- /dev/null +++ b/src/scheduler/testsuite/test_limit_host_dedup.c @@ -0,0 +1,260 @@ +/* + * test_limit_host_dedup.c - bug-1846 regression: a host-scoped limit must + * be staged ONCE per (job, worker), no matter how many allocation attempts + * that job makes on that worker. + * + * limit_prepare_allocation() -> match_job_host_limit_() (plugins/limit.c) + * runs on EVERY per-host allocation attempt (plugin_prepare_allocation(), + * called by resource.c's sched_try_alloc_on_host_()), and used to + * xlist_store() the matched vres into obj->hlimits[worker->idx] + * unconditionally. obj->hlimits is only rebuilt when limit_job_election() + * sees a config-version bump, so a job that pended on the same host round + * after round accumulated one extra copy of the limit per attempt. + * plugin_prepare_allocation() then staged every copy into + * ctx->alloc_req.host via sched_fill_limit_requests(), and seqjob's + * make_host_alloc() built a host allocation carrying all of them -- live + * captured in scheduler-replay-20260923-085356.replay as a DISPATCH whose + * pc13v host alloc held mem + slots + 70 identical limit entries (72) + * where the recording held 3. + * + * This fixture calls plugin_prepare_allocation() repeatedly for one job on + * one worker -- exactly what successive pending rounds do -- and requires + * the host-limit list and the staged host request to each hold the limit + * exactly once. + */ +#include +#include +#include +#include + +#include + +#include "controller.capnp.h" +#include "plugin.h" +#include "resobj.h" +#include "sched/types.h" +#include "scheduler.h" +#include "xcelimit.h" +#include "xstring.h" + +#define WORKER_NAME "hdedup_worker" +#define QUEUE_NAME "normal" +#define TEST_RESREQ ((char*)"select[type==any] rusage[mem=1]") +#define LIMIT_RESNAME "hdedup_host_res" +#define LIMIT_NAME "hdedup_host_limit" +#define JOB_ID ((uint64_t)518461ULL) +#define NATTEMPTS (5) + +void setUp(void) { return; } +void tearDown(void) { return; } + +static void register_slots_resource(var resmgr) +{ + xce_resource_t res; + + memset(&res, 0, sizeof(res)); + res.name = strdup("slots"); + res.valtype = XCE_VAL_NUMERIC; + res.ordtype = XCE_ORD_DECR; + res.flags = XCE_RESF_LIMIT | XCE_RESF_CONSUMABLE | XCE_RESF_RELEASE + | XCE_RESF_DEFAULT_HOST; + + TEST_ASSERT_NOT_NULL(makeResource(resmgr, &res)); +} + +/* same bare-resource shape test_sim_contention_limit_driver_parity.c uses: + * is_host_based_limit() resolves LIMIT_NAME as host-based from + * limit->val.percent != 0, not from any flag on this resource. */ +static void register_plain_resource(var resmgr, const char* name) +{ + xce_resource_t res; + + memset(&res, 0, sizeof(res)); + res.name = strdup(name); + + TEST_ASSERT_NOT_NULL(makeResource(resmgr, &res)); +} + +static void add_worker_resval(xce_sched_worker_t* worker, + xce_sched_core_t* core, const char* resname, + double value) +{ + xce_resource_t* r = NULL; + xce_val_t* val = NULL; + int rc; + + rc = key_find(core->resmgr, (void*)resname, (void**)&r); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_NULL(r); + + val = (xce_val_t*)calloc(1, sizeof(*val)); + TEST_ASSERT_NOT_NULL(val); + val->which = Value_u_fvalue; + val->u.fvalue = value; + + zarray_put(&(worker->resvals), r->idx, val); +} + +static xce_sched_worker_t* make_worker(xce_sched_core_t* core) +{ + xce_sched_worker_t* w = (xce_sched_worker_t*)calloc(1, sizeof(*w)); + + TEST_ASSERT_NOT_NULL(w); + w->name = strdup(WORKER_NAME); + w->status = 0; + w->idx = -1; + + zarray_init(&(w->resvals), 128, (zarray_item_killer_t)kill_sched_resval); + zarray_init(&(w->resInUse), 128, (zarray_item_killer_t)kill_sched_resval); + + add_worker_resval(w, core, "slots", 4.0); + add_worker_resval(w, core, LIMIT_RESNAME, 1000.0); + + return w; +} + +static xce_sched_job_t* make_job(xce_scheduler_t* sched) +{ + job_submit_data_t submit; + xce_job_t j; + xce_sched_job_t* job; + int rc; + + memset(&submit, 0, sizeof(submit)); + submit.queue = (char*)QUEUE_NAME; + submit.user = (char*)"hdedup_user"; + submit.project_name = (char*)"default"; + submit.resreq = TEST_RESREQ; + submit.mintasks = 1; + submit.maxtasks = 1; + + memset(&j, 0, sizeof(j)); + j.jobid = JOB_ID; + j.state = JOB_STAT_PEND; + j.sub_time = 1000ULL; + j.queue_time = 1000ULL; + j.submit = &submit; + + job = make_sched_job(sched, &j); + TEST_ASSERT_NOT_NULL(job); + TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, job->schedobj->tag); + + rc = sched_add_job(&(sched->core), job); + TEST_ASSERT_TRUE(rc >= 0); + + return job; +} + +static int count_staged(const xvec_idval_t* v, int idx) +{ + int i; + int n = 0; + + for (i = 0; i < v->count; i++) { + int residx = -1; + + xvec_get((xvec_t*)&(v->idxs), i, &residx); + if (residx == idx) { + n++; + } + } + + return n; +} + +static xce_scheduler_t g_sched; + +static void test_repeated_host_attempts_stage_host_limit_once(void) +{ + xce_limit_val_t limit_val = { .resname = (char*)LIMIT_RESNAME, + .percent = 1, + .val = 50.0 }; + xce_limit_config_t limit_cfg = { .name = (char*)LIMIT_NAME, + .val = &limit_val }; + xce_limit_config_t* limits[1] = { &limit_cfg }; + xce_conf4sched_t config; + xce_sched_plugin_t* limit_plugin = NULL; + xce_sched_queue_t* queue; + xce_sched_worker_t* w; + xce_sched_job_t* job; + xce_sched_ctx_t ctx; + xlist_t* hlimits; + xce_vres_t* vres; + int vres_idx; + int attempt; + int rc; + + memset(&config, 0, sizeof(config)); + config.nlimits = 1; + config.limits = limits; + + memset(&g_sched, 0, sizeof(g_sched)); + rc = xce_sched_core_init(&(g_sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + register_slots_resource(g_sched.core.resmgr); + register_plain_resource(g_sched.core.resmgr, LIMIT_RESNAME); + + limit_plugin = limit_new_plugin(); + TEST_ASSERT_NOT_NULL(limit_plugin); + rc = plugin_register(&g_sched, limit_plugin); + TEST_ASSERT_TRUE(rc >= 0); + rc = limit_seed_config(limit_plugin, &(g_sched.core), &config); + TEST_ASSERT_TRUE(rc >= 0); + + queue = (xce_sched_queue_t*)calloc(1, sizeof(*queue)); + TEST_ASSERT_NOT_NULL(queue); + queue->name = makeString(QUEUE_NAME); + rc = sched_add_queue(&(g_sched.core), queue); + TEST_ASSERT_TRUE(rc >= 0); + + w = make_worker(&(g_sched.core)); + rc = sched_add_worker(&(g_sched.core), w); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_TRUE(w->idx >= 0); + + job = make_job(&g_sched); + + /* One allocation attempt per simulated pending round, all on the same + * worker, with no config change in between (so limit_job_election() + * never resets obj->hlimits). */ + for (attempt = 0; attempt < NATTEMPTS; attempt++) { + rc = sched_ctx_init(&ctx, &(g_sched.core), job->schedobj); + TEST_ASSERT_EQUAL_INT(0, rc); + ctx.worker = w; + ctx.nhosts = 1; + ctx.nslots = 1; + + rc = plugin_prepare_allocation(&(g_sched.core), &ctx); + TEST_ASSERT_TRUE(rc >= 0); + + hlimits = (xlist_t*)zarray_get(&(job->schedobj->hlimits), w->idx); + TEST_ASSERT_NOT_NULL_MESSAGE( + hlimits, "the host-scoped limit must match on this worker"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 1, hlimits->count, + "obj->hlimits[worker] must hold the matched limit exactly once, " + "however many attempts the job has made on this worker"); + + vres = (xce_vres_t*)xlist_get(hlimits, 0); + TEST_ASSERT_NOT_NULL(vres); + vres_idx = vres->vres->idx; + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 1, count_staged(&(ctx.alloc_req.host), vres_idx), + "alloc_req.host must stage the host limit's vres exactly once " + "-- every extra copy becomes an extra entry in the DISPATCH's " + "host allocation"); + + sched_ctx_fini(&ctx); + } + + printf("test_repeated_host_attempts_stage_host_limit_once: PASS\n"); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_repeated_host_attempts_stage_host_limit_once); + return UNITY_END(); +}