From 0317c7af9816a829d0d18e89631a981f7d261b3c Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 9 Oct 2026 08:02:18 +0000 Subject: [PATCH] fix(scheduler): cache hits keep the full pend reason; sim uses the cache (bug-1932) The session-level unsatisfiable-resreq cache stored only the exhausted attempt's main reason (an int). The allocators record shortfalls as per-host/per-resource entries and leave main_reason at 0, so every cache hit in the live scheduler pended its job with an empty reason. The simulator registered no cache at all (its table lived on xce_scheduler_t), so it searched every identical pending job again and its traces differed from production. - The cache table moves to xce_sched_core_t; cache_lookup/insert/clear and the hook adapters take the core, with no cast to a driver struct. sched_init() builds it, cache_clear() rebuilds it per session, xce_sched_core_fini() destroys it. - A cache entry is a deep copy of the attempt's full xce_job_preason_t plus the job that proved the request unsatisfiable (core->alloc_jobid, set by sched_allocator()). A hit moves a copy into ctx->reason; a miss leaves ctx->reason untouched. - main.c passes the same three adapters to the simulator through new sched_sim_args_t fields; sched_sim_driver_start() sets them on the sim core (plain field writes: white-box tests include sim.c alone). - Decision trace: a hit renders "no plan generated: skipped, same request as proved unsatisfiable earlier this session" (new XCE_TRACE_NOPLAN_KNOWN_UNSAT via sched_trace_known_unsat()); the outcome shows the restored reasons. - sim_verify keeps its main-only PEND compare (old recordings carry main-only hits); comments and the scheduler-sim-live-allocation design note updated. Tests: full-reason assertions in the cache dispatch tests (RED before: "a cache hit must restore the original attempt's FULL pend reason"), test_alloc_cache.c ported to the new API (deep copy, clear frees, -EEXIST frees), a sim-registered clear test, a renderer test, and a sim e2e scenario with two identical unsatisfiable jobs (204, 205) where 205 is skipped with 204's full reasons (RED before: no skip line). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../scheduler-sim-live-allocation/design.md | 2 + src/include/preason.h | 6 +- src/scheduler/include/core.h | 152 ++--- src/scheduler/include/sched/alloc_cache.h | 126 ++--- src/scheduler/include/sched/trace.h | 8 +- src/scheduler/include/scheduler.h | 21 - src/scheduler/include/sim.h | 9 + src/scheduler/src/alloc_cache.c | 238 ++++---- src/scheduler/src/core.c | 6 + src/scheduler/src/core_session.c | 6 +- src/scheduler/src/main.c | 42 +- src/scheduler/src/repl_trace_render.c | 16 +- src/scheduler/src/sched_election.c | 6 +- src/scheduler/src/sched_trace.c | 27 +- src/scheduler/src/scheduler.c | 128 ++--- src/scheduler/src/sim.c | 9 + src/scheduler/src/sim_verify.c | 31 +- src/scheduler/testsuite/test_alloc_cache.c | 519 +++++++++++------- .../testsuite/test_core_session_cache_clear.c | 86 ++- .../testsuite/test_maxtasks_wiring_ingest.c | 40 +- src/scheduler/testsuite/test_repl_trace_e2e.c | 66 ++- .../testsuite/test_repl_trace_e2e_fixture.c | 12 +- .../testsuite/test_scheduler_cache_dispatch.c | 31 +- .../test_scheduler_cache_dispatch_compound.c | 21 +- .../test_sim_cache_clear_isolation.c | 58 +- src/scheduler/testsuite/test_sim_verify.c | 7 +- src/scheduler/testsuite/test_trace_render.c | 46 ++ 27 files changed, 982 insertions(+), 737 deletions(-) diff --git a/.kiro/specs/scheduler-sim-live-allocation/design.md b/.kiro/specs/scheduler-sim-live-allocation/design.md index abfd57e6..210f6b49 100644 --- a/.kiro/specs/scheduler-sim-live-allocation/design.md +++ b/.kiro/specs/scheduler-sim-live-allocation/design.md @@ -35,6 +35,8 @@ - bug-1827 (the `core_clock.c` isolation-test coverage gap) — unrelated, tracked separately. - Any change to `.replay` file format, recording/capture (`recording.c`/`recording_events.c`), or snapshot format (`snapshot_wire.c`/`snapshot_load.c`) — confirmed field-complete and unaffected by the audit (dimension 4). - Giving simulation mode a working `unsatisfiable_cache` implementation of its own. + +> **Superseded (2026-10-09, bug-1932):** the two cache non-goals above no longer hold. The cache table moved from `xce_scheduler_t` onto `xce_sched_core_t`, the simulator registers the same three hooks as normal mode (passed in through `sched_sim_args_t`), and a hit now restores the original attempt's full pend reason instead of only its main reason. The seam itself is unchanged in role: no hook casts the core to a driver struct. - Simulation-mode thread lifecycle/teardown (`sched_fini()`-equivalent for sim mode does not exist today and this spec does not add one). ### Allowed Dependencies diff --git a/src/include/preason.h b/src/include/preason.h index 82d36eed..64659603 100644 --- a/src/include/preason.h +++ b/src/include/preason.h @@ -62,9 +62,9 @@ extern void erase_preason_index(xce_preason_index_t*); * * The scheduler simulator (--sim-verify, sim_verify.c) deliberately does NOT * use this function for its PEND outcome check: it compares main_reason only - * (bug-1845), because a recorded alloc-cache-hit PEND carries only the main - * reason while the simulator (which has no alloc cache) always builds the - * full detail lists. That loosening is a replay property, not object + * (bug-1845), because a recorded PEND and its replay can legitimately carry + * different detail (an alloc-cache hit recorded before bug-1932 carries only + * the main reason). That loosening is a replay property, not object * identity, so it stays simulator-local. * * NULL handling: identical pointers (including two NULLs) are equal; exactly diff --git a/src/scheduler/include/core.h b/src/scheduler/include/core.h index 81978578..79ed411f 100644 --- a/src/scheduler/include/core.h +++ b/src/scheduler/include/core.h @@ -73,97 +73,53 @@ typedef int (*xce_core_decision_cb_t)(void*, xce_decision_t*); /* * xce_core_cache_clear_cb_t - driver-registered per-session cache-clear * hook (scheduler-allocation spec, task 5.2 round 2, Req 7.2): - * `void (*)(xce_sched_core_t*)`. Mirrors xce_core_decision_cb_t's - * driver-registered-hook idiom just above, but generic on purpose -- no - * `void* driver_ctx` companion field, because the sole real - * implementation (alloc_cache.c's xce_alloc_cache_clear_cb() -- see that - * function's own doc comment for why it lives in alloc_cache.c rather - * than driver.c) recovers its owning xce_scheduler_t* via the - * pointer-to-first-member idiom this codebase already uses for this exact - * struct (scheduler.h: `xce_sched_core_t core` is xce_scheduler_t's first - * member), not via an opaque context pointer. + * `void (*)(xce_sched_core_t*)`, called by core_begin_session_internal() + * (core_session.c) when non-NULL. Both drivers register alloc_cache.c's + * xce_alloc_cache_clear_cb() (main.c; sched_sim_driver_start() from the + * caller's sched_sim_args_t), which empties THIS core's own + * unsatisfiable_cache (bug-1932). * - * WHY a hook instead of a direct call (task 5.2 round 1's CRITICAL defect): - * core_begin_session_internal() (core_session.c) is reached from BOTH - * driver.c's xce_sched_driver_run_normal_mode_round() (a real - * xce_scheduler_t*) and sim_loop.c's sched_sim_driver_run() (a - * structurally different xce_sched_sim_driver_t*, sim.h -- Req 5 AC 17 - * "Simulation isolation" -- which has NO unsatisfiable_cache field at - * all). Round 1 called alloc_cache.c's real, strong - * cache_clear(xce_scheduler_t*) unconditionally for both callers (routed - * through a same-named weak fallback that the strong definition always - * overrides at link time in the production binary) -- since link-time - * weak/strong resolution picks ONE winner process-wide, not per call - * site, every simulation-mode session reinterpreted whatever field sits - * at unsatisfiable_cache's byte offset in xce_sched_sim_driver_t - * (alt_events_file, a live FILE*) as a kvtab_t* and clobbered it. - * - * A per-core-instance function pointer fixes this at the data-ownership - * level: each xce_sched_core_t only ever carries the hook ITS OWN driver - * installed via xce_core_set_cache_clear_cb() below. main.c installs - * alloc_cache.c's xce_alloc_cache_clear_cb() on the real xce_scheduler_t's - * core; sim_loop.c never calls the setter at all, so a simulation-mode core's - * hook stays NULL (xce_sched_core_init()'s memset() default) and - * core_begin_session_internal() treats NULL as "this driver has no cache - * to clear" and skips the call entirely -- a true no-op, not an - * accidental type-confused one. + * WHY a hook instead of a direct call: task 5.2 round 1 called a + * cache_clear(xce_scheduler_t*) unconditionally for both drivers, so every + * simulation-mode session reinterpreted xce_sched_sim_driver_t's + * alt_events_file FILE* as the cache and clobbered it. The table now lives + * on the core and no hook casts the core to a driver struct, but the hook + * stays: white-box tests that link core_session.c without alloc_cache.c + * leave it NULL, and NULL means "no cache to clear". */ typedef void (*xce_core_cache_clear_cb_t)(xce_sched_core_t*); /* * xce_unsatisfiable_lookup_cb_t / xce_unsatisfiable_populate_cb_t - - * driver-registered, OPTIONAL per-session unsatisfiable-resreq cache - * consult/populate seam (scheduler-sim-live-allocation spec, task 1.1, - * Req 5.2). Mirrors xce_core_cache_clear_cb_t's own shape and rationale - * immediately above exactly: a bare function pointer taking - * xce_sched_core_t* directly, no wrapper struct, no opaque "impl"/ - * user-data pointer -- single registrant (main.c's normal-mode branch), - * no foreseeable second implementation. + * driver-registered, OPTIONAL unsatisfiable-resreq cache consult/populate + * seam (scheduler-sim-live-allocation spec, task 1.1, Req 5.2) around the + * allocation dispatch chokepoint, sched_make_primary_alloc_() + * (scheduler.c). `obj` is the object cache_lookup()/cache_insert() + * (sched/alloc_cache.h) key on: its resreq fingerprint plus user/queue/ + * project. * - * Parameter shape mirrors the REAL cache's own existing entry points - * (alloc_cache.c's cache_lookup()/cache_insert(), design.md's - * "Unsatisfiable-Cache Callback Seam" component) rather than inventing a - * new bare-integer "fingerprint" type: xce_sched_obj_t* is the exact - * object cache_lookup()/cache_insert() already key on (its embedded - * xce_resreq_fingerprint_t plus user/queue/project identity, sched/ - * alloc_cache.h) -- this seam adds no new type, only a NULL-safe - * indirection over the existing one. xce_sched_obj_t is already visible - * here as a forward-declared pointer type (sched/types.h, included - * above); only a pointer, never the full struct, is needed in this - * header. + * bug-1932: a hit must report the original attempt's FULL pend reason, + * not only its main reason (in production the main reason is usually 0 + * and the real cause is per-host, so a main-only hit pended later jobs + * with an empty reason). Lookup returns 1 on a hit and replaces + * *out_reasons with a copy of the stored reason, *out_jobid with the job + * whose search proved the request unsatisfiable; 0 on a miss (outputs + * untouched). Populate stores a copy of `reasons` (NULL: empty) for + * `jobid`. * - * WHY a hook instead of a direct call (mirrors xce_core_cache_clear_cb_t's - * own documented incident above): the allocation dispatch chokepoint this - * seam wraps (sched_make_primary_alloc_(), scheduler.c) is reached - * identically by both drivers once a later task's Core-Safe Allocation - * Chain migration lands -- but the real cache - * (xce_scheduler_t.unsatisfiable_cache) exists ONLY on a normal-mode - * xce_scheduler_t, never on a simulation-mode xce_sched_sim_driver_t. A - * NULL-checked, driver-registered callback keeps that difference behind - * an explicit seam instead of a struct-prefix cast into mode-specific - * state from code both drivers share -- the same anti-pattern - * xce_core_cache_clear_cb_t's own doc comment documents a real, prior - * memory-corruption incident from. - * - * NULL (the xce_sched_core_init() memset() default, unless a driver - * registers one) means "always compute fresh" for BOTH callbacks -- never - * an error, never a degraded-correctness path, only a degraded- - * performance one. Wiring the actual consult/populate call sites into - * sched_make_primary_alloc_() is a LATER task's job (tasks.md 2.2, - * boundary "Core-Safe Allocation Chain, Unsatisfiable-Cache Callback - * Seam") -- this task adds only the seam (these two fields, their - * adapters in alloc_cache.c, and main.c's normal-mode-only registration), - * not that call site. Every simulation-mode core leaves both fields NULL - * forever (sim_loop.c never registers them, Req 5 AC 17 "Simulation - * isolation"), so a simulation-mode session never touches the real cache - * implementation at all. + * Both drivers register alloc_cache.c's adapters (bug-1932: the + * simulator used to register none, so it searched every identical pending + * job again and its traces differed from production). NULL (the + * xce_sched_core_init() memset() default) means "always compute fresh" -- + * a performance difference only, never a correctness one. */ -typedef int (*xce_unsatisfiable_lookup_cb_t)(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int* out_reason_category); -typedef void (*xce_unsatisfiable_populate_cb_t)(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int reason_category); +typedef int (*xce_unsatisfiable_lookup_cb_t)(xce_sched_core_t* core, + xce_sched_obj_t* obj, + xce_job_preason_t* out_reasons, + uint64_t* out_jobid); +typedef void (*xce_unsatisfiable_populate_cb_t)( + xce_sched_core_t* core, xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, uint64_t jobid); /* Reserved jbuf sentinel for the Recorded-Decision Reconciliation * allocator-thread relay (bug-1837) -- see xce_sched_core_s's reconcile_req/ @@ -238,25 +194,25 @@ struct xce_sched_core_s { xce_core_decision_cb_t on_decision; /* Req 7.2 (scheduler-allocation spec, task 5.2 round 2): driver- * registered per-session cache-clear hook -- see - * xce_core_cache_clear_cb_t's own doc comment above for the full - * rationale. NULL until a driver registers one (xce_core_set_ - * cache_clear_cb()); core_begin_session_internal() (core_session.c) - * treats NULL as "this driver has no cache to clear" and skips the - * call -- true for every simulation-mode xce_sched_core_t, which never - * registers this hook. */ + * xce_core_cache_clear_cb_t above. NULL until a driver registers one + * (xce_core_set_cache_clear_cb()). */ xce_core_cache_clear_cb_t cache_clear_cb; - /* Req 5.2 (scheduler-sim-live-allocation spec, task 1.1): driver- - * registered, OPTIONAL unsatisfiable-resreq cache consult/populate - * seam -- see xce_unsatisfiable_lookup_cb_t/xce_unsatisfiable_ - * populate_cb_t's own doc comment above for the full rationale. NULL - * (xce_sched_core_init()'s memset() default) until main.c's - * normal-mode branch registers both, alongside cache_clear_cb just - * above. A simulation-mode core never registers either, so - * consulting/populating this seam is always a true no-op for sim - * mode ("always compute fresh"), never a fallback onto real cache - * state. */ + /* Req 5.2 (scheduler-sim-live-allocation spec, task 1.1): the + * unsatisfiable-resreq cache consult/populate seam -- see + * xce_unsatisfiable_lookup_cb_t above. NULL until a driver registers + * both (both drivers do, bug-1932). */ xce_unsatisfiable_lookup_cb_t unsatisfiable_lookup_cb; xce_unsatisfiable_populate_cb_t unsatisfiable_populate_cb; + /* bug-1932: the session-level unsatisfiable-resreq cache itself + * (sched/alloc_cache.h), owned by the core so both drivers have one. + * Built and emptied by cache_clear() at every session start, destroyed + * by xce_sched_core_fini(); NULL until the first clear. Allocator + * thread only, plus the clear between sessions. */ + var unsatisfiable_cache; + /* bug-1932: the job sched_make_allocation() is working on, set by + * sched_allocator() (sched_election.c) so a cache populate can record + * which job proved a request unsatisfiable. 0 when unknown. */ + uint64_t alloc_jobid; _Atomic int session_active; /* 1 during scheduling session, 0 between */ _Atomic int eval_in_progress; /* 1 while eval_select loop runs */ _Atomic int staging_merge_complete; /* 1 after staging merge completes */ diff --git a/src/scheduler/include/sched/alloc_cache.h b/src/scheduler/include/sched/alloc_cache.h index 05a13cd1..22e51ff1 100644 --- a/src/scheduler/include/sched/alloc_cache.h +++ b/src/scheduler/include/sched/alloc_cache.h @@ -13,11 +13,13 @@ * * Key shape: xce_resreq_fingerprint_t (sched/types.h, already populated on * xce_primary_schedobj_t.resreq_fp at job-ingest time -- see task 1.2) - * composed with job identity (user/queue/project). Value: the ORIGINAL - * exhausted attempt's pend-reason category (an int code), not a bare - * present/absent marker -- Req 7.5 requires a cache hit to report the same - * reason category the original miss determined, so operators are not - * misled about why a later job was skipped. + * composed with job identity (user/queue/project). Value: a deep copy of + * the ORIGINAL exhausted attempt's full pend reason (main, per-host and + * per-resource entries) plus the job whose search proved the request + * unsatisfiable -- Req 7.5 requires a cache hit to report the same reason + * the original miss determined, and bug-1932 found that keeping only the + * main-reason int pended later jobs with an empty reason whenever the real + * cause was per-host. * * Backed by this codebase's existing keyed-table infrastructure (kv.h's * kvtab_type/key_find()/key_add()) via a THIRD kv_key_ops_t instance @@ -28,23 +30,16 @@ * (cache_key_assign_(), below) and returns >=0 on success, -EEXIST if the * exact key is already present. * - * This header intentionally exposes only what design.md's Service - * Interface specifies (cache_lookup/cache_insert/cache_clear) plus the - * key type and key ops -- no separate cache-construct/-destroy wrapper. - * xce_scheduler_t.unsatisfiable_cache (scheduler.h) is constructed and - * destroyed directly by sched_init()/sched_fini() (scheduler.c) via - * makeInstance(&kvtab_type, ¶ms)/destroy(), using the exported - * cache_key_ops below and the XCE_UNSAT_CACHE_NSLOTS slot count -- the - * exact same convention xce_select.c's sel_node_table_init() already - * establishes for its own composite-struct-keyed kvtab_t - * (sel_node_key_ops). This cache's own unit test (test_alloc_cache.c) - * constructs/destroys its fixture's table identically, so the two never - * drift. + * The table is xce_sched_core_t.unsatisfiable_cache (core.h, bug-1932: + * moved off xce_scheduler_t so the simulation driver has one too). + * cache_clear() builds and empties it at every session start, through the + * per-session clear hook both drivers register; xce_sched_core_fini() + * destroys it. Its datakiller frees each entry. */ #include -#include "scheduler.h" +#include "core.h" /* * xce_cache_key_t - full cache key: the pre-computed resreq fingerprint @@ -63,100 +58,87 @@ typedef struct { /* * cache_key_ops - the kv_key_ops_t (kv.h) binding hash/assign/cmp/destroy - * for xce_cache_key_t. Exported so scheduler.c's sched_init()/sched_fini() - * (the real construct/destroy call sites for xce_scheduler_t. - * unsatisfiable_cache) and this cache's own test can build an identical - * kvtab_t independently. + * for xce_cache_key_t; cache_clear() builds the table with it. */ extern kv_key_ops_t cache_key_ops; /* * XCE_UNSAT_CACHE_NSLOTS - slot count for the kvtab_t backing - * xce_scheduler_t.unsatisfiable_cache. Shared by every construction call - * site (sched_init(), cache_clear()'s destroy+recreate, and this cache's - * own test) so they never drift apart. kv.c's kvtab_init() requires + * xce_sched_core_t.unsatisfiable_cache. kv.c's kvtab_init() requires * nslots > 1. */ #define XCE_UNSAT_CACHE_NSLOTS 257 /* * cache_lookup - Req 7.4: before starting full allocation for a job, look - * up its resreq fingerprint + identity in the cache. + * up its resreq fingerprint + identity in core's cache. * * Preconditions: obj->tag == XCE_RESREQ_PRIMARY; obj->u.primary->resreq_fp * already populated (job-ingest time, core_build_primary_schedobj(), * core_events.c -- task 1.2). * * Returns: - * 1 - hit; *out_reason_category is set to the reason category the - * ORIGINAL exhausted attempt stored at cache_insert() time (Req - * 7.5). Caller pends the job using this category, without - * invoking the plan/host iterators or preemption for this attempt. - * 0 - miss (including: cache not yet constructed). + * 1 - hit; *out is erased and refilled with a copy of the reason the + * ORIGINAL exhausted attempt stored (Req 7.5, bug-1932) and + * *out_jobid set to that attempt's job. Caller pends the job with + * it, without invoking the plan/host iterators for this attempt. + * A copy that runs out of memory part way keeps the entries copied + * so far. + * 0 - miss (including: cache not yet built); *out untouched. * <0 - error (NULL argument, or obj is not a populated primary resreq * scheduling object). */ -extern int cache_lookup(xce_scheduler_t* sched, xce_sched_obj_t* obj, - int* out_reason_category); +extern int cache_lookup(xce_sched_core_t* core, xce_sched_obj_t* obj, + xce_job_preason_t* out, uint64_t* out_jobid); /* * cache_insert - Req 7.3: when a job's primary resreq's plan iterator is * exhausted without a successful allocation, populate the cache with that - * resreq's fingerprint + identity and the exhausted attempt's own final - * pend-reason category (the exhausted plan iterator's own final pend - * reason's main_reason), so a later cache hit for the same key reports the - * same reason (Req 7.5). + * resreq's fingerprint + identity, a deep copy of the attempt's pend + * reason (`reasons`; NULL stores an empty one) and `jobid`, so a later hit + * for the same key reports the same reason (Req 7.5). * - * Returns 0 on success, <0 on error (NULL argument; obj is not a populated - * primary resreq scheduling object; -EEXIST if this exact key is already - * cached; the cache is not yet constructed). + * Returns 0 on success, <0 on error (NULL core/obj; obj is not a + * populated primary resreq scheduling object; -EEXIST if this exact key + * is already cached; -ENOMEM; the cache is not yet built). Nothing is + * stored on error. */ -extern int cache_insert(xce_scheduler_t* sched, xce_sched_obj_t* obj, - int reason_category); +extern int cache_insert(xce_sched_core_t* core, xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, uint64_t jobid); /* - * cache_clear - Req 7.2 (wiring into core_begin_session_internal() is a - * later task -- tasks.md 5.2, not this one): empty every entry from the - * cache. Called once at the start of every scheduling session. + * cache_clear - Req 7.2: empty every entry from core's cache, building the + * table on first use. Called once at the start of every scheduling + * session through xce_alloc_cache_clear_cb(). * * kv.h's intf_kv_t exposes only add_key/del_key/find_key -- no batch * clear-all primitive exists on kvtab_t. This implementation destroys the - * existing backing table and constructs a fresh, empty one with the same - * cache_key_ops/XCE_UNSAT_CACHE_NSLOTS in its place, which is functionally - * equivalent to clearing every entry. If reconstruction fails (e.g. + * existing backing table (its datakiller frees every entry) and constructs + * a fresh, empty one in its place. If reconstruction fails (e.g. * makeInstance() returns NULL on -ENOMEM), the cache is left NULL: * cache_lookup() treats a NULL cache as an unconditional miss and * cache_insert() returns an error, so a failed clear degrades to "cache * always misses" rather than crashing or operating on a stale table. */ -extern void cache_clear(xce_scheduler_t* sched); +extern void cache_clear(xce_sched_core_t* core); /* - * xce_alloc_cache_clear_cb - the xce_core_cache_clear_cb_t (core.h) - * adapter for cache_clear() above (scheduler-allocation spec, task 5.2 - * round 2, Req 7.2). Registered on the real xce_scheduler_t's core via - * xce_core_set_cache_clear_cb() (main.c). See alloc_cache.c's own doc - * comment on this function for the full rationale, including why it lives + * xce_alloc_cache_clear_cb / xce_alloc_cache_lookup_cb / + * xce_alloc_cache_populate_cb - the xce_core_cache_clear_cb_t / + * xce_unsatisfiable_lookup_cb_t / xce_unsatisfiable_populate_cb_t (core.h) + * adapters for cache_clear()/cache_lookup()/cache_insert() on the given + * core. Both drivers register all three (main.c directly; the simulation + * driver from its sched_sim_args_t). See alloc_cache.c for why they live * here rather than in driver.c. */ extern void xce_alloc_cache_clear_cb(xce_sched_core_t* core); - -/* - * xce_alloc_cache_lookup_cb / xce_alloc_cache_populate_cb - the - * xce_unsatisfiable_lookup_cb_t / xce_unsatisfiable_populate_cb_t (core.h) - * adapters for cache_lookup()/cache_insert() above (scheduler-sim-live- - * allocation spec, task 1.1, Req 5.2). Registered on the real - * xce_scheduler_t's core via direct field assignment in main.c's - * normal-mode-only startup branch, alongside xce_alloc_cache_clear_cb() - * just above. See alloc_cache.c's own doc comment on these functions for - * the full rationale, including why they live here rather than in - * driver.c. - */ -extern int xce_alloc_cache_lookup_cb(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int* out_reason_category); -extern void xce_alloc_cache_populate_cb(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int reason_category); +extern int xce_alloc_cache_lookup_cb(xce_sched_core_t* core, + xce_sched_obj_t* obj, + xce_job_preason_t* out_reasons, + uint64_t* out_jobid); +extern void xce_alloc_cache_populate_cb(xce_sched_core_t* core, + xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, + uint64_t jobid); #endif diff --git a/src/scheduler/include/sched/trace.h b/src/scheduler/include/sched/trace.h index 6eac461e..62756714 100644 --- a/src/scheduler/include/sched/trace.h +++ b/src/scheduler/include/sched/trace.h @@ -132,7 +132,8 @@ typedef struct { typedef enum { XCE_TRACE_NOPLAN_DECLINED, /* allocator accepts() declined */ XCE_TRACE_NOPLAN_GENERATOR_FAILED, /* plan_init failed */ - XCE_TRACE_NOPLAN_NONE_GENERATED /* generator produced no plan */ + XCE_TRACE_NOPLAN_NONE_GENERATED, /* generator produced no plan */ + XCE_TRACE_NOPLAN_KNOWN_UNSAT /* bug-1932: unsatisfiable-cache hit */ } xce_sched_trace_noplan_t; typedef enum { @@ -314,6 +315,7 @@ typedef struct sched_trace_entry_s { } check_end; struct { /* NO_PLAN */ xce_sched_trace_noplan_t why; + uint64_t src_jobid; /* KNOWN_UNSAT: the job that proved it */ } no_plan; struct { /* PLAN_BEGIN, PLAN_DISCARDED, PLAN_NOT_TRIED */ int plan_no; /* 1-based, in generation order per requirement */ @@ -630,6 +632,10 @@ extern void sched_trace_check_end(xce_sched_trace_rec_t* rec, int rc, * later in the same requirement makes no statement and counts nothing. */ extern void sched_trace_no_plan(xce_sched_trace_rec_t* rec, xce_sched_trace_noplan_t why); +/* bug-1932: the same, for a request src_jobid (0: unknown) proved + * unsatisfiable earlier this session -- the cache skipped the search. */ +extern void sched_trace_known_unsat(xce_sched_trace_rec_t* rec, + uint64_t src_jobid); /* Plans, numbered in generation order within the current requirement. */ extern void sched_trace_plan_begin(xce_sched_trace_rec_t* rec, diff --git a/src/scheduler/include/scheduler.h b/src/scheduler/include/scheduler.h index 2513bd30..8b4d2eca 100644 --- a/src/scheduler/include/scheduler.h +++ b/src/scheduler/include/scheduler.h @@ -175,27 +175,6 @@ typedef struct { typedef struct { xce_sched_core_t core; xcecli_ctx_t apictx; - /* Req 7.1 (scheduler-allocation spec, task 2.2): the per-session - * unsatisfiable-resreq fingerprint cache -- a kvtab_t (kv.h, RTTI `var`) - * keyed by xce_cache_key_t (sched/alloc_cache.h), value = a pend-reason - * category boxed as (void*)(intptr_t)category (Req 7.5). Constructed in - * sched_init(), destroyed in sched_fini() (scheduler.c) -- both minimal - * construct/destroy calls only, using the exported cache_key_ops - * (sched/alloc_cache.h), mirroring xce_select.c's sel_node_table_init() - * convention for a composite-struct-keyed kvtab_t. Lookup/insert - * (cache_lookup()/cache_insert(), alloc_cache.c) are confined to the - * allocator thread; NULL until sched_init() constructs it. - * - * NOTE (task 2.2 CONCERN, for a later task's attention): Req 7.2 - * requires cache_clear() to run inside core_begin_session_internal() - * (core_session.c), which only receives xce_sched_core_t* core, not - * xce_scheduler_t* -- this field lives on the outer driver struct per - * this task's own scope, not on `core` above. Wiring AC 2's clear call - * is explicitly a later task (tasks.md 5.2), not this one; whoever - * implements it will need to either reach the owning xce_scheduler_t - * from `core` (core is this struct's first member, so the standard - * pointer-to-first-member idiom applies) or relocate this field. */ - var unsatisfiable_cache; xce_cluster_config_t cluster_cfg; /* HA candidate list. Empty (candidates_count==0) only * transiently before sched_init() runs, when no diff --git a/src/scheduler/include/sim.h b/src/scheduler/include/sim.h index 1a75dd07..867f0acd 100644 --- a/src/scheduler/include/sim.h +++ b/src/scheduler/include/sim.h @@ -164,6 +164,15 @@ typedef struct { bool interactive; /* --interactive / -i (task 12) */ const char* log_path; /* --logfile FILE (optional; NULL means * SCHED_DEFAULT_LOG_PATH) */ + /* bug-1932: the unsatisfiable-resreq cache hooks (core.h), registered + * on the simulation core exactly as main.c registers them on the + * normal-mode one, so the simulator skips a request already proved + * unsatisfiable this session the way production does. main.c passes + * alloc_cache.c's adapters; NULL (white-box tests that do not link + * alloc_cache.c) leaves the core computing every attempt fresh. */ + xce_core_cache_clear_cb_t cache_clear_cb; + xce_unsatisfiable_lookup_cb_t unsat_lookup_cb; + xce_unsatisfiable_populate_cb_t unsat_populate_cb; } sched_sim_args_t; /* diff --git a/src/scheduler/src/alloc_cache.c b/src/scheduler/src/alloc_cache.c index e87f4548..a2776a50 100644 --- a/src/scheduler/src/alloc_cache.c +++ b/src/scheduler/src/alloc_cache.c @@ -10,7 +10,9 @@ * string_key_ops, since a composite struct key needs its own. * * - cache_lookup()/cache_insert()/cache_clear(), the three functions - * design.md's Service Interface specifies. + * design.md's Service Interface specifies, on the core's own table + * (bug-1932), and the entry type they store: a deep copy of the + * attempt's full pend reason plus the job that made it. * * cache_key_cmp_() is a THREE-WAY comparator (0 == equal), matching kv.c's * real int_key_ops.cmp (cmp_int_key) contract -- kvtab_get()'s only actual @@ -30,6 +32,7 @@ #include #include +#include "preason.h" #include "sched/alloc_cache.h" /* @@ -122,6 +125,70 @@ kv_key_ops_t cache_key_ops = { .destroy = cache_key_destroy_, }; +/* + * cache_entry_t - one cached verdict (bug-1932): the exhausted attempt's + * full pend reason, owned, and the job whose search produced it. + */ +typedef struct { + xce_job_preason_t reasons; + uint64_t jobid; +} cache_entry_t; + +/* The table's datakiller: frees an entry and every reason it owns. */ +static void cache_entry_free_(void* p) +{ + cache_entry_t* e = (cache_entry_t*)p; + + if (e == NULL) { + return; + } + erase_job_preason(&(e->reasons)); + free(e); +} + +/* + * copy_reasons_ - replaces *dst with a deep copy of *src (NULL: empty). + * The main reason goes first: add_job_preason()'s main-reason branch + * erases the lists. Each entry's strings are strdup()ed by + * add_job_preason(); on -ENOMEM the entries copied so far stay in *dst. + */ +static int copy_reasons_(xce_job_preason_t* dst, const xce_job_preason_t* src) +{ + int rc; + int i; + + rc = add_job_preason(dst, (src != NULL) ? src->main_reason : 0, NULL, NULL); + if ((rc < 0) || (src == NULL)) { + return rc; + } + + for (i = 0; i < src->nhost_reasons; i++) { + const xce_host_reason_t* h = src->host_based_reasons[i]; + + if (h == NULL) { + continue; + } + rc = add_job_preason(dst, h->reason, h->host, NULL); + if (rc < 0) { + return rc; + } + } + + for (i = 0; i < src->nres_reasons; i++) { + const xce_res_reason_t* r = src->res_based_reasons[i]; + + if (r == NULL) { + continue; + } + rc = add_job_preason(dst, r->reason, r->host, r->resource); + if (rc < 0) { + return rc; + } + } + + return 0; +} + /* * build_key_ - construct the full cache key on the stack from a primary * scheduling object's pre-computed resreq fingerprint (Req 7.1, populated @@ -144,14 +211,16 @@ static int build_key_(xce_cache_key_t* key, xce_sched_obj_t* obj) return 0; } -int cache_lookup(xce_scheduler_t* sched, xce_sched_obj_t* obj, - int* out_reason_category) +int cache_lookup(xce_sched_core_t* core, xce_sched_obj_t* obj, + xce_job_preason_t* out, uint64_t* out_jobid) { xce_cache_key_t key; - void* val = NULL; - int rc; + void* val = NULL; + cache_entry_t* e; + int rc; - if ((sched == NULL) || (obj == NULL) || (out_reason_category == NULL)) { + if ((core == NULL) || (obj == NULL) || (out == NULL) + || (out_jobid == NULL)) { return -EINVAL; } @@ -160,29 +229,34 @@ int cache_lookup(xce_scheduler_t* sched, xce_sched_obj_t* obj, return rc; } - if (sched->unsatisfiable_cache == NULL) { - /* Cache not (yet) constructed -- treat as an unconditional miss - * rather than an error. */ + if (core->unsatisfiable_cache == NULL) { + /* Cache not (yet) built -- treat as an unconditional miss rather + * than an error. */ return 0; } - rc = key_find(sched->unsatisfiable_cache, &key, &val); - if (rc <= 0) { + rc = key_find(core->unsatisfiable_cache, &key, &val); + if ((rc <= 0) || (val == NULL)) { return 0; } - *out_reason_category = (int)(intptr_t)val; + e = (cache_entry_t*)val; + *out_jobid = e->jobid; + /* still a hit when the copy runs short of memory: the job is pended + * either way, with whatever reasons were copied */ + (void)copy_reasons_(out, &(e->reasons)); return 1; } -int cache_insert(xce_scheduler_t* sched, xce_sched_obj_t* obj, - int reason_category) +int cache_insert(xce_sched_core_t* core, xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, uint64_t jobid) { xce_cache_key_t key; - int rc; + cache_entry_t* e; + int rc; - if ((sched == NULL) || (obj == NULL)) { + if ((core == NULL) || (obj == NULL)) { return -EINVAL; } @@ -191,126 +265,76 @@ int cache_insert(xce_scheduler_t* sched, xce_sched_obj_t* obj, return rc; } - if (sched->unsatisfiable_cache == NULL) { + if (core->unsatisfiable_cache == NULL) { return -EINVAL; } - rc = key_add(sched->unsatisfiable_cache, &key, - (void*)(intptr_t)reason_category); + e = (cache_entry_t*)calloc(1, sizeof(*e)); + if (e == NULL) { + return -ENOMEM; + } + e->jobid = jobid; + + rc = copy_reasons_(&(e->reasons), reasons); + if (rc >= 0) { + rc = key_add(core->unsatisfiable_cache, &key, e); + } if (rc < 0) { + /* kvtab_add() never runs the datakiller on a failed add */ + cache_entry_free_(e); return rc; } return 0; } -void cache_clear(xce_scheduler_t* sched) +void cache_clear(xce_sched_core_t* core) { kv_params_t params = { .nslots = XCE_UNSAT_CACHE_NSLOTS, .keyops = &cache_key_ops, - .datakiller = NULL }; + .datakiller = cache_entry_free_ }; - if (sched == NULL) { + if (core == NULL) { return; } - if (sched->unsatisfiable_cache != NULL) { - destroy(sched->unsatisfiable_cache); + if (core->unsatisfiable_cache != NULL) { + destroy(core->unsatisfiable_cache); } - sched->unsatisfiable_cache = makeInstance(&kvtab_type, ¶ms); + core->unsatisfiable_cache = makeInstance(&kvtab_type, ¶ms); } /* - * xce_alloc_cache_clear_cb (scheduler-allocation spec, task 5.2 round 2, - * Req 7.2) - the xce_core_cache_clear_cb_t (core.h) adapter for - * cache_clear() above, registered on the real xce_scheduler_t's core via - * xce_core_set_cache_clear_cb() (main.c, alongside the existing - * xce_core_set_decision_cb() registration). `core` is xce_scheduler_t's - * first member (scheduler.h: `typedef struct { xce_sched_core_t core; - * ... } xce_scheduler_t;`), so per C11 6.7.2.1p15 (a pointer to a - * structure's first member is validly convertible to a pointer to the - * enclosing structure, and vice versa) this cast recovers the correct, - * real xce_scheduler_t whenever this function is called through the - * hook -- valid ONLY because the sole registration call site (main.c) - * installs this exact function on a core instance genuinely embedded in - * a real xce_scheduler_t. + * xce_alloc_cache_clear_cb / xce_alloc_cache_lookup_cb / + * xce_alloc_cache_populate_cb - the core.h hook adapters both drivers + * register (main.c; sched_sim_driver_start() from its sched_sim_args_t). + * Since bug-1932 the table is the core's own, so they pass `core` straight + * through -- no cast to a driver struct (the round-1 cast of a simulation + * core to xce_scheduler_t* is what clobbered alt_events_file, see core.h). * - * Deliberately defined HERE (alloc_cache.c), not in driver.c, even though - * driver.c is the file that legitimately owns a real xce_scheduler_t* - * elsewhere: alloc_cache.c already owns cache_clear()'s xce_scheduler_t* - * cast, and several white-box test TUs in this suite #include - * ../src/driver.c directly WITHOUT linking alloc_cache.c (test_driver_ - * translate.c, test_driver_decision_cb.c, test_resval_ownership.c, every - * test_sim_*.c/test_admin_request.c/test_record_lifecycle.c that pulls - * driver.c in) -- had this adapter lived in driver.c and called - * cache_clear() directly, every one of those TUs would fail to LINK (a - * reference to an unresolved external symbol is a link error regardless - * of whether the referencing function is ever actually invoked). Keeping - * this adapter in alloc_cache.c means those TUs are entirely unaffected: - * none of them link alloc_cache.c, none of them reference this function, - * and none of them ever call xce_core_set_cache_clear_cb() either -- their - * cache_clear_cb stays NULL, matching production simulation-mode - * behavior exactly (see xce_core_cache_clear_cb_t's own doc comment, - * core.h, for the full "why NULL is correct for sim mode" rationale). - * - * Round 1's CRITICAL defect is NOT reintroduced by this design: round 1's - * bug was calling cache_clear() UNCONDITIONALLY from inside - * core_begin_session_internal() (core_session.c) for every caller, - * indiscriminately. This function is never called unconditionally by - * anything -- it is only ever invoked as core->cache_clear_cb(core), and - * only for a core instance that main.c's own registration explicitly - * installed it on. + * Deliberately defined HERE, not in driver.c or sim.c: several white-box + * test TUs #include ../src/driver.c or ../src/sim.c directly WITHOUT + * linking alloc_cache.c (test_driver_translate.c, + * test_driver_decision_cb.c, test_resval_ownership.c, every test_sim_*.c + * that pulls those in). A reference to these functions from there would + * fail to link even if never called; those TUs leave the hooks NULL and + * compute every attempt fresh. For the same reason the simulation driver + * receives them as function pointers in sched_sim_args_t (sim.h) rather + * than naming them. */ -void xce_alloc_cache_clear_cb(xce_sched_core_t* core) -{ - cache_clear((xce_scheduler_t*)core); -} +void xce_alloc_cache_clear_cb(xce_sched_core_t* core) { cache_clear(core); } -/* - * xce_alloc_cache_lookup_cb / xce_alloc_cache_populate_cb - - * scheduler-sim-live-allocation spec, task 1.1 (Req 5.2): the - * xce_unsatisfiable_lookup_cb_t / xce_unsatisfiable_populate_cb_t (core.h) - * adapters for cache_lookup()/cache_insert() above, mirroring - * xce_alloc_cache_clear_cb()'s own shape and rationale exactly. - * - * Deliberately defined HERE (alloc_cache.c), not in driver.c or - * scheduler.c, for the IDENTICAL white-box-test-linkage reason - * xce_alloc_cache_clear_cb()'s own doc comment above documents: several - * white-box test TUs #include ../src/driver.c directly WITHOUT linking - * alloc_cache.c. Had either adapter lived anywhere those TUs don't - * already link, every one of them would fail to LINK the moment a future - * task wires a call site (main.c/a shared allocation-chain function) - * against it. Keeping both adapters here means those TUs stay entirely - * unaffected: none of them link alloc_cache.c, none of them reference - * either adapter, and none of them ever assign core->unsatisfiable_ - * lookup_cb/unsatisfiable_populate_cb either -- both fields stay NULL, - * matching production simulation-mode behavior exactly (see - * xce_unsatisfiable_lookup_cb_t/xce_unsatisfiable_populate_cb_t's own doc - * comment, core.h, for the full "why NULL is correct for sim mode" - * rationale). - * - * The (xce_scheduler_t*)core cast below is safe for the identical reason - * xce_alloc_cache_clear_cb()'s own cast is safe just above: these two - * adapters are normal-mode-exclusive -- only ever assigned onto - * core->unsatisfiable_lookup_cb/unsatisfiable_populate_cb by main.c's - * normal-mode branch, only ever invoked with a `core` pointer genuinely - * embedded in an xce_scheduler_t (xce_sched_core_t core is - * xce_scheduler_t's first member, scheduler.h) -- a simulation-mode core - * never registers either callback, so this cast is never reached with a - * xce_sched_sim_driver_t*-embedded core. This is exactly the "small - * adapter function... itself normal-mode-exclusive" allowance design.md's - * "Unsatisfiable-Cache Callback Seam" component documents -- the cast - * must never appear in code shared by both drivers, but is safe here. - */ int xce_alloc_cache_lookup_cb(xce_sched_core_t* core, xce_sched_obj_t* obj, - int* out_reason_category) + xce_job_preason_t* out_reasons, + uint64_t* out_jobid) { - return cache_lookup((xce_scheduler_t*)core, obj, out_reason_category); + return cache_lookup(core, obj, out_reasons, out_jobid); } void xce_alloc_cache_populate_cb(xce_sched_core_t* core, xce_sched_obj_t* obj, - int reason_category) + const xce_job_preason_t* reasons, + uint64_t jobid) { - cache_insert((xce_scheduler_t*)core, obj, reason_category); + (void)cache_insert(core, obj, reasons, jobid); } diff --git a/src/scheduler/src/core.c b/src/scheduler/src/core.c index 0326b6c1..92e150ba 100644 --- a/src/scheduler/src/core.c +++ b/src/scheduler/src/core.c @@ -381,6 +381,12 @@ void xce_sched_core_fini(xce_sched_core_t* core) destroy(core->plugins); core->plugins = NULL; } + /* bug-1932: the unsatisfiable-resreq cache (cache_clear() built it with + * a datakiller, so this frees every entry too) */ + if (core->unsatisfiable_cache != NULL) { + destroy(core->unsatisfiable_cache); + core->unsatisfiable_cache = NULL; + } core_destroy_and_free_(&(core->jobs), (void (*)(void*))core_job_free_); core_destroy_and_free_(&(core->workers), (void (*)(void*))core_worker_free_); diff --git a/src/scheduler/src/core_session.c b/src/scheduler/src/core_session.c index 9e635f6b..c7368e8b 100644 --- a/src/scheduler/src/core_session.c +++ b/src/scheduler/src/core_session.c @@ -191,10 +191,8 @@ static void core_begin_session_internal(xce_sched_core_t* core) * per-session state this function already resets above, via the * driver-registered hook (core.h's xce_core_cache_clear_cb_t) rather * than an unconditional call -- see that typedef's own doc comment for - * why round 1's unconditional weak-symbol cache_clear() call was a - * CRITICAL type-confusion bug for simulation-mode sessions, and why a - * NULL hook (sim_loop.c never registers one) must be a true no-op - * here, not a fallthrough to any cache-clearing behavior. */ + * round 1's type-confusion bug. A NULL hook (a core whose driver + * registered no cache) is a true no-op. */ if (core->cache_clear_cb != NULL) { core->cache_clear_cb(core); } diff --git a/src/scheduler/src/main.c b/src/scheduler/src/main.c index 49edec1a..13ba3eb9 100644 --- a/src/scheduler/src/main.c +++ b/src/scheduler/src/main.c @@ -388,6 +388,11 @@ int main(int argc, char* argv[]) sim_args.output_path = arguments.sim_output; sim_args.interactive = (arguments.interactive != 0); sim_args.log_path = logfile; + /* bug-1932: the simulator uses the same unsatisfiable-resreq cache + * as normal mode (registered below for the live core) */ + sim_args.cache_clear_cb = xce_alloc_cache_clear_cb; + sim_args.unsat_lookup_cb = xce_alloc_cache_lookup_cb; + sim_args.unsat_populate_cb = xce_alloc_cache_populate_cb; rc = sched_sim_driver_run(&sim, &sim_args); if (rc == -ENOSYS) { @@ -492,38 +497,13 @@ int main(int argc, char* argv[]) xce_core_set_decision_cb(&(sched.core), xce_sched_driver_on_decision, &sched); - /* Req 7.2 (scheduler-allocation spec, task 5.2 round 2): register the - * per-session cache-clear hook -- see xce_core_cache_clear_cb_t's doc - * comment (core.h) for why this replaced round 1's unconditional - * weak-symbol cache_clear() call, and alloc_cache.c's own doc comment - * on xce_alloc_cache_clear_cb() for why the adapter lives there - * rather than in driver.c. Registered once, alongside the decision-cb - * registration just above (the only other driver-registered core - * hook), before the first scheduling round. The simulation driver - * (sim_loop.c) deliberately never makes this call -- a - * simulation-mode core has no cache to clear (Req 5 AC 17 - * "Simulation isolation"). */ + /* Req 7.2 / Req 5.2 (scheduler-allocation task 5.2, scheduler-sim- + * live-allocation task 1.1): register the unsatisfiable-resreq cache's + * per-session clear hook and its consult/populate seam, once, before + * the first scheduling round -- see core.h's xce_core_cache_clear_cb_t + * and xce_unsatisfiable_lookup_cb_t. The simulation branch above passes + * the same three adapters through sched_sim_args_t (bug-1932). */ xce_core_set_cache_clear_cb(&(sched.core), xce_alloc_cache_clear_cb); - - /* Req 5.2 (scheduler-sim-live-allocation spec, task 1.1): register the - * optional unsatisfiable-resreq cache consult/populate seam -- see - * xce_unsatisfiable_lookup_cb_t/xce_unsatisfiable_populate_cb_t's own - * doc comment (core.h) for the full rationale. Direct field - * assignment, not a setter call (unlike cache_clear_cb just above): - * xce_sched_core_t is not opaque, and these two fields carry no - * locking/session-active discipline beyond the plain NULL-check the - * seam's own contract already gives them -- main.c already writes - * xce_sched_core_t fields directly elsewhere (e.g. sig_handler()'s - * `sched->core.stop = 1;`, above), so this is a precedented idiom, not - * a new one. Registered once, alongside every other driver-registered - * core hook, before the first scheduling round. The simulation driver - * (sim_loop.c) deliberately never sets either field -- a - * simulation-mode core has no cache to consult or populate (Req 5 AC - * 17 "Simulation isolation", same rationale as cache_clear_cb just - * above). Wiring the actual consult/populate call site into - * sched_make_primary_alloc_() (scheduler.c) is a LATER task's job - * (tasks.md 2.2) -- until then, both fields being non-NULL here has - * no observable effect, since nothing yet reads them. */ sched.core.unsatisfiable_lookup_cb = xce_alloc_cache_lookup_cb; sched.core.unsatisfiable_populate_cb = xce_alloc_cache_populate_cb; diff --git a/src/scheduler/src/repl_trace_render.c b/src/scheduler/src/repl_trace_render.c index 430806f1..a0c263e6 100644 --- a/src/scheduler/src/repl_trace_render.c +++ b/src/scheduler/src/repl_trace_render.c @@ -307,16 +307,26 @@ static void put_check_end(FILE* f, int rc) } } -static void put_no_plan(FILE* f, xce_sched_trace_noplan_t why) +static void put_no_plan(FILE* f, const sched_trace_entry_t* e) { + char job[64]; + fputs("no plan generated: ", f); - switch (why) { + switch (e->u.no_plan.why) { case XCE_TRACE_NOPLAN_DECLINED: fputs("allocator declined the requirement", f); break; case XCE_TRACE_NOPLAN_GENERATOR_FAILED: fputs("plan generator failed to start", f); break; + case XCE_TRACE_NOPLAN_KNOWN_UNSAT: /* bug-1932 */ + fputs("skipped, same request ", f); + if ((e->u.no_plan.src_jobid != 0) + && (fmt_jobid(job, sizeof(job), e->u.no_plan.src_jobid) >= 0)) { + fprintf(f, "as %s ", job); + } + fputs("proved unsatisfiable earlier this session", f); + break; case XCE_TRACE_NOPLAN_NONE_GENERATED: default: fputs("generator produced no plan", f); @@ -534,7 +544,7 @@ static void put_entry(FILE* f, const sched_trace_entry_t* e, put_check_end(f, e->u.check_end.rc); break; case SCHED_TRACE_E_NO_PLAN: - put_no_plan(f, e->u.no_plan.why); + put_no_plan(f, e); break; case SCHED_TRACE_E_PLAN_BEGIN: put_plan(f, e); diff --git a/src/scheduler/src/sched_election.c b/src/scheduler/src/sched_election.c index 7c83580b..6ff1e5c7 100644 --- a/src/scheduler/src/sched_election.c +++ b/src/scheduler/src/sched_election.c @@ -317,7 +317,11 @@ void* sched_allocator(void* arg) /* calculate decision for jobid */ /* scheduler-sim-live-allocation spec, task 2.2: sched_make_ * allocation() is core-typed -- pass core directly. */ - alloc_rc = sched_make_allocation(core, job->schedobj); + /* bug-1932: a cache populate records the job that proved its + * request unsatisfiable, for later hits' trace lines */ + core->alloc_jobid = jobid; + alloc_rc = sched_make_allocation(core, job->schedobj); + core->alloc_jobid = 0; if (alloc_rc < 0) { /* something wrong */ SCHED_TRACE(core, diff --git a/src/scheduler/src/sched_trace.c b/src/scheduler/src/sched_trace.c index d5e00628..bd9aac1b 100644 --- a/src/scheduler/src/sched_trace.c +++ b/src/scheduler/src/sched_trace.c @@ -574,8 +574,10 @@ void sched_trace_check_end(xce_sched_trace_rec_t* rec, int rc, e->u.check_end.plans_follow = plans_follow; } -void sched_trace_no_plan(xce_sched_trace_rec_t* rec, - xce_sched_trace_noplan_t why) +/* The NO_PLAN entry, or NULL when it is not stored (over the line bound, + * or the record is incomplete). */ +static sched_trace_entry_t* no_plan_add(xce_sched_trace_rec_t* rec, + xce_sched_trace_noplan_t why) { sched_trace_entry_t* e; @@ -588,10 +590,25 @@ void sched_trace_no_plan(xce_sched_trace_rec_t* rec, } e = sti_entry_add(rec, SCHED_TRACE_E_NO_PLAN); - if (e == NULL) { - return; + if (e != NULL) { + e->u.no_plan.why = why; + } + return e; +} + +void sched_trace_no_plan(xce_sched_trace_rec_t* rec, + xce_sched_trace_noplan_t why) +{ + (void)no_plan_add(rec, why); +} + +void sched_trace_known_unsat(xce_sched_trace_rec_t* rec, uint64_t src_jobid) +{ + sched_trace_entry_t* e = no_plan_add(rec, XCE_TRACE_NOPLAN_KNOWN_UNSAT); + + if (e != NULL) { + e->u.no_plan.src_jobid = src_jobid; } - e->u.no_plan.why = why; } /* Fills a PLAN_BEGIN / PLAN_DISCARDED / PLAN_NOT_TRIED payload. */ diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index bae5a8c2..360737ed 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -158,29 +158,51 @@ sched_primary_cache_obj_(xce_sched_ctx_t* ctx, } /* - * sched_pend_from_cached_reason_ - Req 7.4/7.5: on a cache hit, attach the - * STORED reason category to ctx->reason using the identical mechanism - * every allocator-exhaustion path already uses to create/populate a pend - * reason (make_pendreason() + add_job_preason(), never a direct calloc()/ - * field write -- see preason.h/preason.c and e.g. resource.c's - * sched_try_alloc_on_host_() or alloc_runner.c's sched_allocator_run(), - * all of which follow this exact "if (ctx->reason == NULL) { - * ctx->reason = make_pendreason(0); }" pattern). add_job_preason(reasons, - * code, NULL, NULL) is that same function's own "set main reason" branch - * (preason.c) -- the identical call shape design.md's cache_insert() - * precondition ties reason_category to (xce_job_preason_t.main_reason). + * sched_pend_from_cache_ - Req 7.4/7.5, bug-1932: consult the + * driver-registered unsatisfiable-cache lookup hook. On a hit, ctx->reason + * (created the way every allocator-exhaustion path creates it) is replaced + * by a copy of the FULL reason the original exhausted attempt recorded -- + * main, per-host and per-resource -- and the decision trace names the job + * that proved the request unsatisfiable. Keeping only the main reason + * (the pre-bug-1932 behavior) pended later jobs with an empty reason + * whenever the real cause was per-host. + * + * 1 on a hit (the caller pends without running either allocator), 0 on a + * miss, a NULL hook, or no memory for the reason (all compute fresh). */ -static void sched_pend_from_cached_reason_(xce_sched_ctx_t* ctx, int category) +static int sched_pend_from_cache_(xce_sched_core_t* core, xce_sched_ctx_t* ctx, + xce_sched_obj_t* cache_obj) { + xce_job_preason_t found; + uint64_t src_jobid = 0; + + if (core->unsatisfiable_lookup_cb == NULL) { + return 0; + } + + /* looked up into a local, so a miss leaves ctx->reason as it was */ + memset(&found, 0, sizeof(found)); + if (core->unsatisfiable_lookup_cb(core, cache_obj, &found, &src_jobid) + != 1) { + return 0; + } + if (ctx->reason == NULL) { ctx->reason = make_pendreason(0); } - - if (ctx->reason == NULL) { - return; + if ((ctx->reason == NULL) || (ctx->reason->reasons == NULL)) { + /* no storage for the reason: compute fresh instead */ + erase_job_preason(&found); + return 0; } + /* the copy's lists move into ctx's reason */ + erase_job_preason(ctx->reason->reasons); + *(ctx->reason->reasons) = found; - add_job_preason(ctx->reason->reasons, category, NULL, NULL); + /* Decision trace: no check and no plan for this requirement */ + SCHED_TRACE(core, sched_trace_known_unsat(core->trace_rec, src_jobid)); + + return 1; } /* @@ -240,7 +262,6 @@ static int sched_make_primary_alloc_(xce_sched_core_t* core, xce_primary_schedobj_t scratch_primary; xce_sched_obj_t scratch_obj; xce_sched_obj_t* cache_obj; - int reason_category = 0; int rc; cache_obj = sched_primary_cache_obj_(ctx, &scratch_primary, &scratch_obj); @@ -248,14 +269,10 @@ static int sched_make_primary_alloc_(xce_sched_core_t* core, /* Req 7.4 / scheduler-sim-live-allocation spec, task 2.2 * (Unsatisfiable-Cache Callback Seam): consult the driver-registered * lookup hook instead of calling cache_lookup() directly -- a NULL - * hook (simulation mode, which never registers one) means "always - * compute fresh", never an error, so this is the one primary-resreq - * dispatch chokepoint the seam exists for. A cache hit pends the job - * immediately, without invoking either allocator for this attempt. */ - if ((core->unsatisfiable_lookup_cb != NULL) - && (core->unsatisfiable_lookup_cb(core, cache_obj, &reason_category) - == 1)) { - sched_pend_from_cached_reason_(ctx, reason_category); + * hook means "always compute fresh", never an error. A cache hit pends + * the job immediately, with the original attempt's full reason + * (bug-1932), without invoking either allocator for this attempt. */ + if (sched_pend_from_cache_(core, ctx, cache_obj)) { return 0; } @@ -274,19 +291,17 @@ static int sched_make_primary_alloc_(xce_sched_core_t* core, /* Req 7.3: once the chosen allocator has exhausted every candidate * plan (the not-satisfied signal, rc == 0), populate the cache with - * this resreq's fingerprint + identity and the exhausted attempt's own - * final pend-reason category, before the caller pends the job. A - * negative rc is an infrastructure error, not an unsatisfiable - * determination, so it is never cached; a positive rc is success. - * Routed through the same NULL-safe seam as the lookup above -- a NULL - * populate hook is simply a no-op, not an error. */ - if (rc == 0) { - int category - = (ctx->reason != NULL) ? ctx->reason->reasons->main_reason : 0; - - if (core->unsatisfiable_populate_cb != NULL) { - core->unsatisfiable_populate_cb(core, cache_obj, category); - } + * this resreq's fingerprint + identity, a copy of the exhausted + * attempt's full pend reason and the job it was for (bug-1932), before + * the caller pends the job. A negative rc is an infrastructure error, + * not an unsatisfiable determination, so it is never cached; a + * positive rc is success. Routed through the same NULL-safe seam as + * the lookup above -- a NULL populate hook is simply a no-op. */ + if ((rc == 0) && (core->unsatisfiable_populate_cb != NULL)) { + core->unsatisfiable_populate_cb( + core, cache_obj, + (ctx->reason != NULL) ? ctx->reason->reasons : NULL, + core->alloc_jobid); } return rc; @@ -791,23 +806,11 @@ int sched_init(xce_scheduler_t* sched, xce_client_config_t* config, return rc; } - /* Req 7.1 (scheduler-allocation spec, task 2.2): construct the - * per-session unsatisfiable-resreq cache's backing kvtab_t, using the - * exported cache_key_ops (sched/alloc_cache.h) -- same - * makeInstance(&kvtab_type, ¶ms) convention xce_select.c's - * sel_node_table_init() already establishes for a composite-struct- - * keyed kvtab_t. Left NULL on -ENOMEM: cache_lookup()/cache_insert() - * (alloc_cache.c) both treat a NULL cache defensively (unconditional - * miss / error respectively) rather than crashing. */ - { - kv_params_t unsat_cache_params - = { .nslots = XCE_UNSAT_CACHE_NSLOTS, - .keyops = &cache_key_ops, - .datakiller = NULL }; - - sched->unsatisfiable_cache - = makeInstance(&kvtab_type, &unsat_cache_params); - } + /* Req 7.1: build the unsatisfiable-resreq cache now, so it is usable + * before the first session start empties it again (bug-1932: it lives + * on the core). Left NULL on -ENOMEM: cache_lookup()/cache_insert() + * treat a NULL cache as a miss / an error, never a crash. */ + cache_clear(&(sched->core)); /* bug-1584: xce_sched_core_init() alone leaves the clock atoms at the * neutral (0,0,1,1) default, under which xce_core_now() returns raw @@ -969,20 +972,11 @@ void sched_fini(xce_scheduler_t* sched) xcecli_stop(sched->apictx); xcecli_ctx_fini(sched->apictx); - /* Req 7.1 (scheduler-allocation spec, task 2.2): destroy the - * unsatisfiable-resreq cache's backing kvtab_t. Safe here -- both the - * jobpicker and allocator threads (the only threads that ever touch - * this cache, confined to the allocator side) have already been - * cancelled and joined above. */ - if (sched->unsatisfiable_cache != NULL) { - destroy(sched->unsatisfiable_cache); - sched->unsatisfiable_cache = NULL; - } - /* Req 5.1/5.2 (scheduler-core spec, task 4.1): the remaining teardown * (cbufs, pend_updates dataqs, resmgr/group managers, the CSE intern - * table, the core lock) is xce_sched_core_fini()'s job (core.c) -- - * same order as before this task, just relocated. */ + * table, the core lock -- and, since bug-1932, the unsatisfiable-resreq + * cache, now that the threads that touch it are joined above) is + * xce_sched_core_fini()'s job (core.c). */ xce_sched_core_fini(&(sched->core)); /* scheduler-core spec, task 9.4 fix round: pairs with sched_init()'s diff --git a/src/scheduler/src/sim.c b/src/scheduler/src/sim.c index fb199131..49867957 100644 --- a/src/scheduler/src/sim.c +++ b/src/scheduler/src/sim.c @@ -550,6 +550,15 @@ int sched_sim_driver_start(xce_sched_sim_driver_t* sim, return rc; } + /* bug-1932: the same unsatisfiable-resreq cache hooks normal mode + * registers (main.c), passed in by the caller -- see sched_sim_args_t. + * The cache is the core's own, so no hook reaches driver state. Plain + * field writes, not xce_core_set_cache_clear_cb(): white-box tests + * #include this file without core_session.c. */ + sim->core.cache_clear_cb = args->cache_clear_cb; + sim->core.unsatisfiable_lookup_cb = args->unsat_lookup_cb; + sim->core.unsatisfiable_populate_cb = args->unsat_populate_cb; + /* xce_sched_core_init() deliberately leaves core->pend_updates[0]/[1] * NULL (core.c's own file header comment: their dataq_init() killer * callback stays driver-owned) -- but xce_sched_core_fini() diff --git a/src/scheduler/src/sim_verify.c b/src/scheduler/src/sim_verify.c index d8a02185..9fa21655 100644 --- a/src/scheduler/src/sim_verify.c +++ b/src/scheduler/src/sim_verify.c @@ -129,26 +129,17 @@ static void sim_verify_why(char** why, const char* fmt, ...) * pending-reason objects identical") than this one ("is this the same * replay outcome"). * - * bug-1845: core.h documents, as a deliberate "simulation isolation" - * design decision, that "a simulation-mode session never touches the real - * cache implementation at all" -- core->unsatisfiable_lookup_cb/ - * unsatisfiable_populate_cb are registered only by main.c's normal-mode - * branch (alloc_cache.c), never by sim_loop.c. sched_make_primary_alloc_() - * (scheduler.c) consults that cache FIRST; on a hit it records only - * main_reason via sched_pend_from_cached_reason_() (no host/resource - * detail at all -- that is the entire point of the cache, skipping the - * expensive per-host iteration that would normally produce that detail). - * Replay, having no cache, always falls through to the full per-host - * computation and therefore always produces full detail. Two independent, - * architecturally-intentional detail levels for the exact same semantic - * outcome (this job is PEND, for this main reason) -- not a decision - * correctness signal, and requiring them to match produced systematic - * false MISMATCH reports on every cache-hit PEND decision in the original - * recording. Live-verified against a real captured .replay file: expected - * (recorded, cache-hit) reason showed - * {main_reason=0, nhost_reasons=0, nres_reasons=0}; actual (replay, full - * computation) showed {main_reason=0, nhost_reasons=0, nres_reasons=2} -- - * main_reason matched, detail did not, and never architecturally can. + * bug-1845: a recorded PEND and its replay can carry the same outcome at + * different detail levels. Until bug-1932, a live unsatisfiable-cache hit + * recorded only main_reason ({main 0, nhost 0, nres 0} in a real captured + * .replay) while the simulator, which then had no cache, computed the full + * per-host detail ({main 0, nres 2}); requiring the detail to match + * reported a false MISMATCH on every cache-hit PEND. Since bug-1932 both + * modes use the cache and a hit copies the full reason, but main-only + * stays: recordings made before the fix still carry main-only hits, and a + * hit legitimately carries the reasons of the job that proved the request + * unsatisfiable, which a recording made without a hit for this job need + * not share. */ static int sim_verify_pend_main_reason_eq(const xce_job_preason_t* a, const xce_job_preason_t* b, diff --git a/src/scheduler/testsuite/test_alloc_cache.c b/src/scheduler/testsuite/test_alloc_cache.c index 6f6482b3..12435155 100644 --- a/src/scheduler/testsuite/test_alloc_cache.c +++ b/src/scheduler/testsuite/test_alloc_cache.c @@ -1,7 +1,7 @@ /* * test_alloc_cache.c - unit test for the session-level unsatisfiable- * resreq cache (scheduler-allocation spec, task 2.2, Req 7.1, 7.3, 7.4, - * 7.5). + * 7.5; bug-1932). * * White-box test TU: #includes ../src/alloc_cache.c directly (same * convention test_host_iter.c/test_job_update.c/test_handlers.c already @@ -9,14 +9,12 @@ * cache_key_hash_/cache_key_cmp_/cache_key_assign_/cache_key_destroy_ and * cache_lookup()/cache_insert()/cache_clear() implementations, not a stub. * - * alloc_cache.c only depends on kv.h (libxce, already linked) and - * scheduler.h/sched/types.h (header-only struct definitions) -- it never - * touches xce_scheduler_t.core, so this fixture never calls - * xce_sched_core_init(); a bare memset()'d xce_scheduler_t with its own - * unsatisfiable_cache field constructed directly (mirroring sched_init()'s - * own construction call, scheduler.c) is sufficient. + * bug-1932: the cache lives on xce_sched_core_t (both drivers own one), so + * the fixture is a bare memset()'d core whose table cache_clear() builds -- + * the same call both drivers' per-session clear hook makes. core.c is not + * linked here, so the fixture destroys the table itself. * - * Covers, per this task's Observable line: + * Covers: * - same-key-twice HIT (not just insert-then-miss-on-a-different-key, * which the cache_key_cmp_() inverted-polarity bug already found and * fixed during design review -- requirements.md's "New types (Req 7)" @@ -25,10 +23,10 @@ * - a lookup differing in any single field (each of the four resreq * fingerprint fields, and each of the three identity fields) is a * miss. - * - reason-category round-trip: insert with category X, hit later - * returns X -- tested with two distinct category values to rule out a - * trivial always-returns-0 implementation. - * - clear() empties all entries. + * - bug-1932: a hit returns the FULL stored pend reason (main, per-host + * and per-resource entries) and the job that proved it, as a deep copy + * independent of both the inserted original and later lookups. + * - clear() empties all entries (and frees them: ASAN). */ #include #include @@ -37,6 +35,7 @@ #include +#include "preason.h" #include "sched/alloc_cache.h" #include "scheduler.h" @@ -51,28 +50,20 @@ void tearDown(void) { return; } /* Fixture helpers */ /* ---------------------------------------------------------------------- */ -/* - * init_sched - a bare xce_scheduler_t with only unsatisfiable_cache - * constructed (this cache never touches sched->core). Mirrors sched_init()'s - * own construction call (scheduler.c) exactly, so the fixture and - * production code can never silently drift. - */ -static void init_sched(xce_scheduler_t* sched) +/* init_core - a bare core with only its cache table built, by the same + * cache_clear() call the drivers' per-session hook makes. */ +static void init_core(xce_sched_core_t* core) { - kv_params_t params = { .nslots = XCE_UNSAT_CACHE_NSLOTS, - .keyops = &cache_key_ops, - .datakiller = NULL }; - - memset(sched, 0, sizeof(*sched)); - sched->unsatisfiable_cache = makeInstance(&kvtab_type, ¶ms); - TEST_ASSERT_NOT_NULL(sched->unsatisfiable_cache); + memset(core, 0, sizeof(*core)); + cache_clear(core); + TEST_ASSERT_NOT_NULL(core->unsatisfiable_cache); } -static void fini_sched(xce_scheduler_t* sched) +static void fini_core(xce_sched_core_t* core) { - if (sched->unsatisfiable_cache != NULL) { - destroy(sched->unsatisfiable_cache); - sched->unsatisfiable_cache = NULL; + if (core->unsatisfiable_cache != NULL) { + destroy(core->unsatisfiable_cache); + core->unsatisfiable_cache = NULL; } } @@ -105,6 +96,22 @@ static void init_primary_obj(xce_sched_obj_t* obj, xce_primary_schedobj_t* prima obj->project = (char*)project; } +/* A pend reason with every kind of entry: main `main_reason`, one host + * reason and two resource reasons (one host-scoped, one cluster-wide). */ +static void make_reasons(xce_job_preason_t* r, int main_reason, + const char* host) +{ + memset(r, 0, sizeof(*r)); + TEST_ASSERT_EQUAL_INT(0, add_job_preason(r, main_reason, NULL, NULL)); + TEST_ASSERT_TRUE(add_job_preason(r, PREASON_MATCH_FAIL, (char*)host, NULL) + > 0); + TEST_ASSERT_TRUE( + add_job_preason(r, PREASON_NO_RESOURCE, (char*)host, (char*)"slots") + > 0); + TEST_ASSERT_TRUE( + add_job_preason(r, PREASON_NO_RESOURCE, NULL, (char*)"license") > 0); +} + /* Baseline fixture fingerprint + identity, reused (with one field varied * at a time) across the miss tests below. Distinct string literals (not * merely distinct content) are used for the identity fields so pointer @@ -115,6 +122,7 @@ static void init_primary_obj(xce_sched_obj_t* obj, xce_primary_schedobj_t* prima #define BASE_MAX_TASKS 4 #define BASE_SELECT_NODE_ID 100u #define BASE_RUSAGE_HASH ((uint64_t)0xdeadbeefULL) +#define BASE_JOBID ((uint64_t)4242ULL) static const char BASE_USER[] = "alice"; static const char BASE_QUEUE[] = "normal"; static const char BASE_PROJECT[] = "proj1"; @@ -128,21 +136,24 @@ static const char BASE_PROJECT[] = "proj1"; * from day one would not fail any test that doesn't specifically assert a * hit"): insert once, then look up an INDEPENDENTLY constructed key with * every field identical (including pointer-identical identity fields) -- - * must be a hit, returning the exact category given at insert time. A - * cache_key_cmp_() with the earlier, inverted boolean polarity would fail + * must be a hit, returning the exact reasons and job given at insert time. + * A cache_key_cmp_() with the earlier, inverted boolean polarity would fail * this test (every lookup a false miss) while still passing a weaker * insert-then-miss-on-a-different-key round-trip. */ static void test_same_key_twice_is_a_hit(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj1, obj2; - xce_primary_schedobj_t primary1, primary2; - int category = 42; - int out = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj1, obj2; + xce_primary_schedobj_t primary1, primary2; + xce_job_preason_t in; + xce_job_preason_t out; + uint64_t jobid = 0; + int rc; - init_sched(&sched); + init_core(&core); + make_reasons(&in, 0, "host1"); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj1, &primary1, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, @@ -151,56 +162,73 @@ static void test_same_key_twice_is_a_hit(void) BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - rc = cache_insert(&sched, &obj1, category); + rc = cache_insert(&core, &obj1, &in, BASE_JOBID); TEST_ASSERT_EQUAL_INT(0, rc); - rc = cache_lookup(&sched, &obj2, &out); + rc = cache_lookup(&core, &obj2, &out, &jobid); TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(category, out); + TEST_ASSERT_EQUAL_UINT64(BASE_JOBID, jobid); + TEST_ASSERT_TRUE_MESSAGE( + job_preason_eq(&in, &out, NULL), + "bug-1932: a hit must restore the full stored pend reason -- main, " + "per-host and per-resource entries -- not just the main reason"); + TEST_ASSERT_EQUAL_INT(1, out.nhost_reasons); + TEST_ASSERT_EQUAL_INT(2, out.nres_reasons); - fini_sched(&sched); + erase_job_preason(&in); + erase_job_preason(&out); + fini_core(&core); } /* - * A lookup before any insert is a plain miss. + * A lookup before any insert is a plain miss, and leaves `out` alone. */ static void test_lookup_before_any_insert_is_a_miss(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj; - xce_primary_schedobj_t primary; - int out = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_job_preason_t out; + uint64_t jobid = 7; + int rc; - init_sched(&sched); + init_core(&core); + memset(&out, 0, sizeof(out)); + out.main_reason = 9; init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - rc = cache_lookup(&sched, &obj, &out); + rc = cache_lookup(&core, &obj, &out, &jobid); TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(9, out.main_reason); + TEST_ASSERT_EQUAL_UINT64(7, jobid); - fini_sched(&sched); + fini_core(&core); } /* - * differing_field_misses - shared body for "a lookup differing in exactly - * one field is a miss": insert the base key with `category`, then look up - * a second key built from the given (possibly-varied) fields -- must miss. + * assert_variant_misses - shared body for "a lookup differing in exactly + * one field is a miss": insert the base key, then look up a second key + * built from the given (possibly-varied) fields -- must miss. */ static void assert_variant_misses(int min_tasks, int max_tasks, uint32_t select_node_id, uint64_t rusage_hash, const char* user, const char* queue, const char* project) { - xce_scheduler_t sched; - xce_sched_obj_t base_obj, variant_obj; - xce_primary_schedobj_t base_primary, variant_primary; - int out = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t base_obj, variant_obj; + xce_primary_schedobj_t base_primary, variant_primary; + xce_job_preason_t in; + xce_job_preason_t out; + uint64_t jobid = 0; + int rc; - init_sched(&sched); + init_core(&core); + make_reasons(&in, 0, "host1"); + memset(&out, 0, sizeof(out)); init_primary_obj(&base_obj, &base_primary, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, @@ -208,13 +236,14 @@ static void assert_variant_misses(int min_tasks, int max_tasks, init_primary_obj(&variant_obj, &variant_primary, min_tasks, max_tasks, select_node_id, rusage_hash, user, queue, project); - rc = cache_insert(&sched, &base_obj, 7); + rc = cache_insert(&core, &base_obj, &in, BASE_JOBID); TEST_ASSERT_EQUAL_INT(0, rc); - rc = cache_lookup(&sched, &variant_obj, &out); + rc = cache_lookup(&core, &variant_obj, &out, &jobid); TEST_ASSERT_EQUAL_INT(0, rc); - fini_sched(&sched); + erase_job_preason(&in); + fini_core(&core); } static void test_miss_on_differing_min_tasks(void) @@ -273,63 +302,128 @@ static void test_miss_on_differing_project(void) } /* - * Req 7.5: a cache hit reports the SAME reason category the original - * miss's exhausted attempt stored -- tested with two distinct category - * values (on two distinct keys, since re-inserting the identical key - * would hit kvtab_add()'s own -EEXIST contract) to rule out a trivial - * always-returns-0 (or always-returns-the-first-inserted-value) - * implementation. + * Req 7.5 / bug-1932: two distinct keys keep their own reasons and jobs -- + * rules out an implementation returning the first-inserted (or a fixed) + * value for every hit. */ -static void test_reason_category_round_trip_two_distinct_categories(void) +static void test_two_entries_keep_their_own_reasons_and_jobs(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj_a, obj_b; - xce_primary_schedobj_t primary_a, primary_b; - static const char user_b[] = "carol"; - int out_a = -1, out_b = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj_a, obj_b; + xce_primary_schedobj_t primary_a, primary_b; + static const char user_b[] = "carol"; + xce_job_preason_t in_a, in_b, out; + uint64_t jobid = 0; - init_sched(&sched); + init_core(&core); + make_reasons(&in_a, 0, "host-a"); + make_reasons(&in_b, PREASON_BAD_RESREQ, "host-b"); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj_a, &primary_a, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - /* Distinct key (differs only in identity) so this doesn't collide with - * obj_a's own insert. */ init_primary_obj(&obj_b, &primary_b, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, user_b, BASE_QUEUE, BASE_PROJECT); - rc = cache_insert(&sched, &obj_a, 11); - TEST_ASSERT_EQUAL_INT(0, rc); - rc = cache_insert(&sched, &obj_b, 99); - TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj_a, &in_a, 11)); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj_b, &in_b, 99)); - rc = cache_lookup(&sched, &obj_a, &out_a); - TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(11, out_a); + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj_a, &out, &jobid)); + TEST_ASSERT_EQUAL_UINT64(11, jobid); + TEST_ASSERT_TRUE(job_preason_eq(&in_a, &out, NULL)); - rc = cache_lookup(&sched, &obj_b, &out_b); - TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(99, out_b); + /* a hit replaces whatever `out` held: no merge with a's entries */ + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj_b, &out, &jobid)); + TEST_ASSERT_EQUAL_UINT64(99, jobid); + TEST_ASSERT_TRUE(job_preason_eq(&in_b, &out, NULL)); - fini_sched(&sched); + erase_job_preason(&in_a); + erase_job_preason(&in_b); + erase_job_preason(&out); + fini_core(&core); } /* - * cache_clear() empties every entry: insert two distinct keys, clear, then - * confirm BOTH now miss (not just the one inserted last). + * bug-1932: the cache owns a deep copy. Erasing (freeing) the inserted + * original, and erasing a previous lookup's output, must leave the stored + * entry intact -- the original attempt's reason is freed when its decision + * is published, long before a later job's lookup. + */ +static void test_entry_is_a_deep_copy(void) +{ + xce_sched_core_t core; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_job_preason_t in, expect, out; + uint64_t jobid = 0; + + init_core(&core); + make_reasons(&in, 0, "host1"); + make_reasons(&expect, 0, "host1"); + memset(&out, 0, sizeof(out)); + init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj, &in, BASE_JOBID)); + erase_job_preason(&in); + + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj, &out, &jobid)); + TEST_ASSERT_TRUE(job_preason_eq(&expect, &out, NULL)); + erase_job_preason(&out); + + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj, &out, &jobid)); + TEST_ASSERT_TRUE(job_preason_eq(&expect, &out, NULL)); + + erase_job_preason(&expect); + erase_job_preason(&out); + fini_core(&core); +} + +/* + * A NULL reason (an attempt that pended without recording any) is stored + * as an empty reason; a hit then reports it as empty, not as garbage. + */ +static void test_null_reasons_stored_as_empty(void) +{ + xce_sched_core_t core; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_job_preason_t empty, out; + uint64_t jobid = 0; + + init_core(&core); + memset(&empty, 0, sizeof(empty)); + make_reasons(&out, 0, "stale"); + init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj, NULL, BASE_JOBID)); + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj, &out, &jobid)); + TEST_ASSERT_TRUE(job_preason_eq(&empty, &out, NULL)); + + fini_core(&core); +} + +/* + * cache_clear() empties all entries (freeing them -- ASAN reports a leak + * otherwise) and leaves a usable table. */ static void test_clear_empties_all_entries(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj_a, obj_b; - xce_primary_schedobj_t primary_a, primary_b; - static const char user_b[] = "dave"; - int out = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj_a, obj_b; + xce_primary_schedobj_t primary_a, primary_b; + static const char user_b[] = "dave"; + xce_job_preason_t in, out; + uint64_t jobid = 0; - init_sched(&sched); + init_core(&core); + make_reasons(&in, 0, "host1"); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj_a, &primary_a, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, @@ -338,37 +432,42 @@ static void test_clear_empties_all_entries(void) BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, user_b, BASE_QUEUE, BASE_PROJECT); - TEST_ASSERT_EQUAL_INT(0, cache_insert(&sched, &obj_a, 1)); - TEST_ASSERT_EQUAL_INT(0, cache_insert(&sched, &obj_b, 2)); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj_a, &in, 1)); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj_b, &in, 2)); /* Sanity: both are hits before clear. */ - TEST_ASSERT_EQUAL_INT(1, cache_lookup(&sched, &obj_a, &out)); - TEST_ASSERT_EQUAL_INT(1, cache_lookup(&sched, &obj_b, &out)); + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj_a, &out, &jobid)); + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj_b, &out, &jobid)); - cache_clear(&sched); - TEST_ASSERT_NOT_NULL(sched.unsatisfiable_cache); + cache_clear(&core); + TEST_ASSERT_NOT_NULL(core.unsatisfiable_cache); - rc = cache_lookup(&sched, &obj_a, &out); - TEST_ASSERT_EQUAL_INT(0, rc); - rc = cache_lookup(&sched, &obj_b, &out); - TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(0, cache_lookup(&core, &obj_a, &out, &jobid)); + TEST_ASSERT_EQUAL_INT(0, cache_lookup(&core, &obj_b, &out, &jobid)); - fini_sched(&sched); + erase_job_preason(&in); + erase_job_preason(&out); + fini_core(&core); } /* * Re-inserting the identical key without an intervening clear propagates * kvtab_add()'s own -EEXIST contract -- exercising cache_insert()'s error - * path. + * path, which must free the rejected copy (ASAN) and keep the first entry. */ static void test_insert_duplicate_key_returns_eexist(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj1, obj2; - xce_primary_schedobj_t primary1, primary2; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj1, obj2; + xce_primary_schedobj_t primary1, primary2; + xce_job_preason_t in1, in2, out; + uint64_t jobid = 0; + int rc; - init_sched(&sched); + init_core(&core); + make_reasons(&in1, 0, "host1"); + make_reasons(&in2, 0, "host2"); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj1, &primary1, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, @@ -377,26 +476,39 @@ static void test_insert_duplicate_key_returns_eexist(void) BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - TEST_ASSERT_EQUAL_INT(0, cache_insert(&sched, &obj1, 5)); - rc = cache_insert(&sched, &obj2, 6); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&core, &obj1, &in1, 5)); + rc = cache_insert(&core, &obj2, &in2, 6); TEST_ASSERT_EQUAL_INT(-EEXIST, rc); - fini_sched(&sched); + TEST_ASSERT_EQUAL_INT(1, cache_lookup(&core, &obj1, &out, &jobid)); + TEST_ASSERT_EQUAL_UINT64(5, jobid); + TEST_ASSERT_TRUE(job_preason_eq(&in1, &out, NULL)); + + erase_job_preason(&in1); + erase_job_preason(&in2); + erase_job_preason(&out); + fini_core(&core); } /* - * Defensive argument checking: NULL sched/obj/out_reason_category, and a - * non-primary tag object, are all rejected rather than crashing. + * Defensive argument checking: NULL core/obj/out/out_jobid, and a + * non-primary tag object, are all rejected rather than crashing. A core + * whose table was never built is a plain miss on lookup and an error on + * insert. */ static void test_argument_validation(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj; - xce_primary_schedobj_t primary; - xce_sched_obj_t non_primary_obj; - int out = -1; + xce_sched_core_t core; + xce_sched_core_t bare; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_sched_obj_t non_primary_obj; + xce_job_preason_t out; + uint64_t jobid = 0; - init_sched(&sched); + init_core(&core); + memset(&bare, 0, sizeof(bare)); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); @@ -404,35 +516,37 @@ static void test_argument_validation(void) memset(&non_primary_obj, 0, sizeof(non_primary_obj)); non_primary_obj.tag = XCE_RESREQ_PRIMARY + 1; /* anything != PRIMARY */ - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(NULL, &obj, &out)); - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(&sched, NULL, &out)); - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(&sched, &obj, NULL)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(NULL, &obj, &out, &jobid)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(&core, NULL, &out, &jobid)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(&core, &obj, NULL, &jobid)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_lookup(&core, &obj, &out, NULL)); TEST_ASSERT_EQUAL_INT(-EINVAL, - cache_lookup(&sched, &non_primary_obj, &out)); + cache_lookup(&core, &non_primary_obj, &out, &jobid)); - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(NULL, &obj, 1)); - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(&sched, NULL, 1)); - TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(&sched, &non_primary_obj, 1)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(NULL, &obj, NULL, 1)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(&core, NULL, NULL, 1)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + cache_insert(&core, &non_primary_obj, NULL, 1)); + + TEST_ASSERT_EQUAL_INT(0, cache_lookup(&bare, &obj, &out, &jobid)); + TEST_ASSERT_EQUAL_INT(-EINVAL, cache_insert(&bare, &obj, NULL, 1)); /* cache_clear(NULL) must not crash. */ cache_clear(NULL); - fini_sched(&sched); + fini_core(&core); } /* ---------------------------------------------------------------------- */ -/* Task 1.1 (scheduler-sim-live-allocation spec, Req 5.2): the */ -/* unsatisfiable-cache callback seam. */ +/* The unsatisfiable-cache callback seam (scheduler-sim-live-allocation */ +/* Req 5.2; bug-1932: both drivers register it). */ /* ---------------------------------------------------------------------- */ /* * A freshly-zeroed xce_sched_core_t (the same memset() default * xce_sched_core_init() applies -- core.c is not linked into this - * white-box TU, so this test zeroes a bare instance directly rather than - * calling that function) leaves BOTH new callback fields NULL. This is - * the exact state every simulation-mode xce_sched_core_t is in forever -- - * sim_loop.c never registers either callback (Req 5 AC 17 "Simulation - * isolation"). + * white-box TU) leaves BOTH callback fields NULL: a core whose driver + * registered no cache computes every attempt fresh. */ static void test_seam_fields_default_null_on_zeroed_core(void) { @@ -442,116 +556,93 @@ static void test_seam_fields_default_null_on_zeroed_core(void) TEST_ASSERT_NULL(core.unsatisfiable_lookup_cb); TEST_ASSERT_NULL(core.unsatisfiable_populate_cb); + TEST_ASSERT_NULL(core.unsatisfiable_cache); } /* - * THE Observable this task's own TDD note names: "with both callbacks - * left NULL, the allocation dispatch chokepoint always computes fresh - * with no crash and no incorrect skip." The real chokepoint - * (sched_make_primary_alloc_(), scheduler.c) is migrated to actually - * consult this seam by a LATER task (tasks.md 2.2, boundary "Core-Safe - * Allocation Chain, Unsatisfiable-Cache Callback Seam") -- this task adds - * only the seam itself, not that call site. This test proves the seam's - * own NULL-safety contract in isolation, using the identical NULL-check - * shape design.md's own invariant specifies for that future call site - * ("the allocation chain ... only ever NULL-checks and calls these"): - * with both fields NULL, a NULL-checked consult never dereferences either - * function pointer (no crash), is never mistaken for a cache HIT (no - * incorrect skip -- `hit` stays 0, meaning "compute fresh"), and a - * NULL-checked populate attempt is silently skipped, not a fatal error. + * With both callbacks left NULL, a NULL-checked consult never dereferences + * either function pointer (no crash), is never mistaken for a cache HIT + * (no incorrect skip), and a NULL-checked populate attempt is silently + * skipped -- the same NULL-check shape sched_make_primary_alloc_() + * (scheduler.c) uses. */ static void test_null_callbacks_mean_always_compute_fresh(void) { - xce_sched_core_t core; - xce_sched_obj_t obj; - xce_primary_schedobj_t primary; - int category = -1; - int hit = -1; /* poison: neither 0 nor 1 yet */ + xce_sched_core_t core; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_job_preason_t out; + uint64_t jobid = 0; + int hit = -1; /* poison: neither 0 nor 1 yet */ memset(&core, 0, sizeof(core)); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - /* Prove the guard's PRECONDITION explicitly, immediately before relying - * on it below -- not just that the end result happens to be "compute - * fresh", but that the NULL-ness this test's two guarded calls depend - * on genuinely holds at the exact point each guard is evaluated. A - * design that accidentally called through a non-NULL stale/garbage - * pointer here (e.g. an uninitialized field instead of a true NULL) - * could still coincidentally return 0/not crash on some platforms; - * asserting NULL directly, right where each guard fires, rules that - * out. */ TEST_ASSERT_NULL(core.unsatisfiable_lookup_cb); if (core.unsatisfiable_lookup_cb != NULL) { - hit = core.unsatisfiable_lookup_cb(&core, &obj, &category); + hit = core.unsatisfiable_lookup_cb(&core, &obj, &out, &jobid); } else { hit = 0; /* no cache registered => always a miss => compute fresh */ } TEST_ASSERT_EQUAL_INT(0, hit); - /* A NULL-checked populate attempt must be a true no-op, not a crash -- - * same "assert the guard's own precondition, right at the guard" - * discipline as the lookup check just above. */ TEST_ASSERT_NULL(core.unsatisfiable_populate_cb); if (core.unsatisfiable_populate_cb != NULL) { - core.unsatisfiable_populate_cb(&core, &obj, 7); + core.unsatisfiable_populate_cb(&core, &obj, NULL, 7); } } /* - * Normal-mode observable (this task's own note): "a normal-mode test - * confirming the cache is still consulted/populated exactly as before - * this change." xce_alloc_cache_lookup_cb()/xce_alloc_cache_populate_cb() - * (alloc_cache.c) are the two adapters main.c registers on the real - * xce_scheduler_t's core (xce_sched_core_t core is xce_scheduler_t's - * first member, scheduler.h -- the same pointer-to-first-member idiom - * xce_alloc_cache_clear_cb() already uses). This test calls them through - * core.unsatisfiable_lookup_cb/unsatisfiable_populate_cb exactly as a - * normal-mode driver would, and confirms the result is identical to - * calling cache_lookup()/cache_insert() directly -- proving the adapters - * are a transparent passthrough, not a behavior change. + * xce_alloc_cache_lookup_cb()/xce_alloc_cache_populate_cb() (alloc_cache.c) + * are the adapters both drivers register. Called through the core's seam + * exactly as sched_make_primary_alloc_() does, they must be a transparent + * passthrough to cache_lookup()/cache_insert() on that same core -- no + * cast to a driver struct, so they work on any core (bug-1932). */ static void test_lookup_and_populate_cb_adapters_match_direct_cache_calls(void) { - xce_scheduler_t sched; - xce_sched_obj_t obj; - xce_primary_schedobj_t primary; - int out_direct = -1; - int out_via_cb = -1; - int rc; + xce_sched_core_t core; + xce_sched_obj_t obj; + xce_primary_schedobj_t primary; + xce_job_preason_t in, out_direct, out_via_cb; + uint64_t jobid = 0; + int rc; - init_sched(&sched); - sched.core.unsatisfiable_lookup_cb = xce_alloc_cache_lookup_cb; - sched.core.unsatisfiable_populate_cb = xce_alloc_cache_populate_cb; + init_core(&core); + core.unsatisfiable_lookup_cb = xce_alloc_cache_lookup_cb; + core.unsatisfiable_populate_cb = xce_alloc_cache_populate_cb; + make_reasons(&in, 0, "host1"); + memset(&out_direct, 0, sizeof(out_direct)); + memset(&out_via_cb, 0, sizeof(out_via_cb)); init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, BASE_QUEUE, BASE_PROJECT); - /* Miss before any insert, via the callback -- same as a direct - * cache_lookup() call (test_lookup_before_any_insert_is_a_miss - * above). */ - TEST_ASSERT_NOT_NULL(sched.core.unsatisfiable_lookup_cb); - rc = sched.core.unsatisfiable_lookup_cb(&(sched.core), &obj, &out_via_cb); + rc = core.unsatisfiable_lookup_cb(&core, &obj, &out_via_cb, &jobid); TEST_ASSERT_EQUAL_INT(0, rc); - /* Populate via the callback... */ - TEST_ASSERT_NOT_NULL(sched.core.unsatisfiable_populate_cb); - sched.core.unsatisfiable_populate_cb(&(sched.core), &obj, 55); + core.unsatisfiable_populate_cb(&core, &obj, &in, 55); - /* ...then confirm a DIRECT cache_lookup() call sees the SAME entry -- - * the adapter genuinely populated the real cache, not a shadow copy. */ - rc = cache_lookup(&sched, &obj, &out_direct); + /* the adapter populated the real cache, not a shadow copy */ + rc = cache_lookup(&core, &obj, &out_direct, &jobid); TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(55, out_direct); + TEST_ASSERT_EQUAL_UINT64(55, jobid); + TEST_ASSERT_TRUE(job_preason_eq(&in, &out_direct, NULL)); - /* And the callback-based lookup now also reports the same hit. */ - rc = sched.core.unsatisfiable_lookup_cb(&(sched.core), &obj, &out_via_cb); + jobid = 0; + rc = core.unsatisfiable_lookup_cb(&core, &obj, &out_via_cb, &jobid); TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(55, out_via_cb); + TEST_ASSERT_EQUAL_UINT64(55, jobid); + TEST_ASSERT_TRUE(job_preason_eq(&in, &out_via_cb, NULL)); - fini_sched(&sched); + erase_job_preason(&in); + erase_job_preason(&out_direct); + erase_job_preason(&out_via_cb); + fini_core(&core); } int main(void) @@ -566,7 +657,9 @@ int main(void) RUN_TEST(test_miss_on_differing_user); RUN_TEST(test_miss_on_differing_queue); RUN_TEST(test_miss_on_differing_project); - RUN_TEST(test_reason_category_round_trip_two_distinct_categories); + RUN_TEST(test_two_entries_keep_their_own_reasons_and_jobs); + RUN_TEST(test_entry_is_a_deep_copy); + RUN_TEST(test_null_reasons_stored_as_empty); RUN_TEST(test_clear_empties_all_entries); RUN_TEST(test_insert_duplicate_key_returns_eexist); RUN_TEST(test_argument_validation); diff --git a/src/scheduler/testsuite/test_core_session_cache_clear.c b/src/scheduler/testsuite/test_core_session_cache_clear.c index c04e731f..977a2a39 100644 --- a/src/scheduler/testsuite/test_core_session_cache_clear.c +++ b/src/scheduler/testsuite/test_core_session_cache_clear.c @@ -18,19 +18,19 @@ * if non-NULL, and this fixture explicitly registers the REAL production * adapter (alloc_cache.c's xce_alloc_cache_clear_cb() -- see that * function's own doc comment for why it lives in alloc_cache.c rather - * than driver.c) on init_sched()'s xce_scheduler_t, the same call main.c - * makes in production. + * than driver.c) on init_core()'s core, the same call both drivers make + * (bug-1932). * * This file still does NOT use the white-box #include convention most of * this test suite uses for core_session.c; its meson.build entry still * lists core.c/core_clock.c/core_session.c/decision.c/alloc_cache.c as * genuinely separate compiled sources so this fixture links against the - * REAL cache_clear(xce_scheduler_t*) (alloc_cache.c) that its registered + * REAL cache_clear() (alloc_cache.c) that its registered * hook calls into -- not a stub. * * ── What this proves ───────────────────────────────────────────────────── * A job's fingerprint+identity is inserted into a REAL, live - * xce_scheduler_t.unsatisfiable_cache (cache_insert(), simulating what + * core->unsatisfiable_cache (cache_insert(), simulating what * task 5.1's allocator-exhaustion path would have done); a lookup for the * identical key is confirmed a HIT; the REAL xce_core_begin_session() * (core_session.c's public entry point, exercising the real @@ -87,36 +87,28 @@ void tearDown(void) { return; } /* ---------------------------------------------------------------------- */ /* - * init_sched - a real xce_scheduler_t with BOTH core (xce_sched_core_init(), - * core.c) and unsatisfiable_cache (mirroring sched_init()'s own - * construction call, scheduler.c, and test_alloc_cache.c's own - * init_sched()) constructed -- everything core_begin_session_internal() - * touches, without sched_init()'s heavier networking/threading setup - * (this test never calls sched_init() at all). Also registers the REAL - * production cache_clear_cb (round 2, xce_alloc_cache_clear_cb() -- - * alloc_cache.c, linked into this exact test binary, see this file's own - * header comment) -- the same one-time-at-startup registration main.c - * performs in production (xce_core_set_cache_clear_cb(), right alongside - * xce_core_set_decision_cb()), so this fixture's own registration is not - * a stand-in wrapper but the identical call production makes. + * init_core - a real core (xce_sched_core_init(), core.c) with the REAL + * production cache_clear_cb registered (xce_alloc_cache_clear_cb(), + * alloc_cache.c, linked into this exact test binary) -- the same + * one-time-at-startup registration both drivers make (main.c for normal + * mode, sched_sim_driver_start() for simulation, bug-1932). The cache + * table lives on the core (bug-1932); cache_clear() builds it here, the + * same call the hook makes at every session start. */ -static void init_sched(xce_scheduler_t* sched) +static void init_core(xce_sched_core_t* core) { - kv_params_t params = { .nslots = XCE_UNSAT_CACHE_NSLOTS, - .keyops = &cache_key_ops, - .datakiller = NULL }; - int rc; + int rc; - memset(sched, 0, sizeof(*sched)); + memset(core, 0, sizeof(*core)); - rc = xce_sched_core_init(&(sched->core)); + rc = xce_sched_core_init(core); TEST_ASSERT_EQUAL_INT(0, rc); - rc = xce_core_set_cache_clear_cb(&(sched->core), xce_alloc_cache_clear_cb); + rc = xce_core_set_cache_clear_cb(core, xce_alloc_cache_clear_cb); TEST_ASSERT_EQUAL_INT(0, rc); - sched->unsatisfiable_cache = makeInstance(&kvtab_type, ¶ms); - TEST_ASSERT_NOT_NULL(sched->unsatisfiable_cache); + cache_clear(core); + TEST_ASSERT_NOT_NULL(core->unsatisfiable_cache); } /* @@ -157,40 +149,40 @@ static void init_primary_obj(xce_sched_obj_t* obj, */ static void test_begin_session_clears_previously_cached_entry(void) { - xce_scheduler_t sched; + xce_sched_core_t core; xce_sched_obj_t obj; xce_primary_schedobj_t primary; - int category = 7; - int out = -1; + xce_job_preason_t out; + uint64_t jobid = 0; int rc; - init_sched(&sched); + init_core(&core); + memset(&out, 0, sizeof(out)); init_primary_obj(&obj, &primary, 1, 4, 100u, 0xdeadbeefULL, "alice", "normal", "proj1"); - rc = cache_insert(&sched, &obj, category); + rc = cache_insert(&core, &obj, NULL, 7); TEST_ASSERT_EQUAL_INT(0, rc); /* Baseline: proves the fixture/insert are sound before relying on them * below -- same "prove the harness works first" discipline * test_core_session.c's own worker-reset tests already follow. */ - rc = cache_lookup(&sched, &obj, &out); + rc = cache_lookup(&core, &obj, &out, &jobid); TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(category, out); + TEST_ASSERT_EQUAL_UINT64(7, jobid); - rc = xce_core_begin_session(&(sched.core)); + rc = xce_core_begin_session(&core); TEST_ASSERT_EQUAL_INT(0, rc); - out = -1; - rc = cache_lookup(&sched, &obj, &out); + rc = cache_lookup(&core, &obj, &out, &jobid); TEST_ASSERT_EQUAL_INT_MESSAGE( 0, rc, "Req 7.2: a job cached as unsatisfiable in one session must be " "re-evaluated fresh (a cache MISS), not immediately pended from " "the cache, in the next session -- core_begin_session_internal() " - "must clear xce_scheduler_t.unsatisfiable_cache"); + "must clear the core's unsatisfiable_cache"); - rc = xce_core_end_session(&(sched.core)); + rc = xce_core_end_session(&core); TEST_ASSERT_EQUAL_INT(0, rc); } @@ -203,28 +195,30 @@ static void test_begin_session_clears_previously_cached_entry(void) */ static void test_cache_still_usable_after_begin_session_clear(void) { - xce_scheduler_t sched; + xce_sched_core_t core; xce_sched_obj_t obj; xce_primary_schedobj_t primary; - int out = -1; + xce_job_preason_t out; + uint64_t jobid = 0; int rc; - init_sched(&sched); + init_core(&core); + memset(&out, 0, sizeof(out)); - rc = xce_core_begin_session(&(sched.core)); + rc = xce_core_begin_session(&core); TEST_ASSERT_EQUAL_INT(0, rc); init_primary_obj(&obj, &primary, 2, 8, 200u, 0xfeedfaceULL, "bob", "batch", "proj2"); - rc = cache_insert(&sched, &obj, 3); + rc = cache_insert(&core, &obj, NULL, 3); TEST_ASSERT_EQUAL_INT(0, rc); - rc = cache_lookup(&sched, &obj, &out); + rc = cache_lookup(&core, &obj, &out, &jobid); TEST_ASSERT_EQUAL_INT(1, rc); - TEST_ASSERT_EQUAL_INT(3, out); + TEST_ASSERT_EQUAL_UINT64(3, jobid); - rc = xce_core_end_session(&(sched.core)); + rc = xce_core_end_session(&core); TEST_ASSERT_EQUAL_INT(0, rc); } diff --git a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c index e78585ff..77ac5a1d 100644 --- a/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c +++ b/src/scheduler/testsuite/test_maxtasks_wiring_ingest.c @@ -1065,26 +1065,28 @@ test_resource_request_update_never_changes_dispatched_task_count(void) static void test_differing_task_counts_do_not_collide_in_unsatisfiable_cache(void) { - xce_scheduler_t sched; - xce_queue_t* queue; - xce_core_event_t queue_event; - xce_job_t* job_a; - xce_job_t* job_b; - xce_job_t* job_ptrs[2]; - xce_core_event_t job_event; - xce_sched_job_t* sjob_a; - xce_sched_job_t* sjob_b; - int out_reason = -1; - int rc; + xce_scheduler_t sched; + xce_queue_t* queue; + xce_core_event_t queue_event; + xce_job_t* job_a; + xce_job_t* job_b; + xce_job_t* job_ptrs[2]; + xce_core_event_t job_event; + xce_sched_job_t* sjob_a; + xce_sched_job_t* sjob_b; + xce_job_preason_t out_reason; + uint64_t out_jobid = 0; + int rc; + memset(&out_reason, 0, sizeof(out_reason)); memset(&sched, 0, sizeof(sched)); rc = xce_sched_core_init(&(sched.core)); TEST_ASSERT_TRUE_MESSAGE(rc >= 0, "fixture: xce_sched_core_init() must " "succeed"); - cache_clear(&sched); + cache_clear(&(sched.core)); TEST_ASSERT_NOT_NULL_MESSAGE( - sched.unsatisfiable_cache, + sched.core.unsatisfiable_cache, "fixture: cache_clear() must construct a real unsatisfiable_cache " "table (mirrors the real per-session construction, main.c's " "xce_core_set_cache_clear_cb() registration)"); @@ -1147,24 +1149,26 @@ test_differing_task_counts_do_not_collide_in_unsatisfiable_cache(void) /* ── The Req 5.4 proof: an unsatisfiable outcome cached for job A * must NOT produce a cache hit for job B. ──────────────────────────── */ - rc = cache_insert(&sched, sjob_a->schedobj, 7 /* arbitrary category */); + rc = cache_insert(&(sched.core), sjob_a->schedobj, NULL, + 7 /* arbitrary job */); TEST_ASSERT_EQUAL_INT_MESSAGE( 0, rc, "fixture: cache_insert() for job A must succeed"); - rc = cache_lookup(&sched, sjob_b->schedobj, &out_reason); + rc = cache_lookup(&(sched.core), sjob_b->schedobj, &out_reason, &out_jobid); TEST_ASSERT_EQUAL_INT_MESSAGE( 0, rc, "Req 5.4: job B (a different requested task count, otherwise " "identical) must NOT hit job A's cached unsatisfiable outcome"); /* Sanity: the cache mechanism itself genuinely works -- job A's OWN - * key IS a hit, with the SAME reason category given at insert time. + * key IS a hit, with the SAME job given at insert time. * Without this, the miss above could just as easily mean the cache is * dead altogether, not that it correctly distinguishes task counts. */ - rc = cache_lookup(&sched, sjob_a->schedobj, &out_reason); + rc = cache_lookup(&(sched.core), sjob_a->schedobj, &out_reason, &out_jobid); TEST_ASSERT_EQUAL_INT_MESSAGE( 1, rc, "sanity: job A's own key must hit its own cached entry"); - TEST_ASSERT_EQUAL_INT(7, out_reason); + TEST_ASSERT_EQUAL_UINT64(7, out_jobid); + erase_job_preason(&out_reason); } /* diff --git a/src/scheduler/testsuite/test_repl_trace_e2e.c b/src/scheduler/testsuite/test_repl_trace_e2e.c index 3a404e6d..35c8e411 100644 --- a/src/scheduler/testsuite/test_repl_trace_e2e.c +++ b/src/scheduler/testsuite/test_repl_trace_e2e.c @@ -23,8 +23,9 @@ * - extra.replay, generated per run, for what the baseline lacks: a * satisfied compound requirement (201), a compound requirement that * fails at its first member (202), a job array with two elements - * (0-0-0-32768[1] and [2]) and a host-selection mismatch (203 selects - * mem>20000, which only w02 has). + * (0-0-0-32768[1] and [2]), a host-selection mismatch (203 selects + * mem>20000, which only w02 has) and two identical unsatisfiable jobs + * (204, 205; bug-1932). * * `break DECISION jobid=105` (a raw packed decimal id) names 0-0-0-105; * since bug-1926 the dashed form 0-0-0-105 names it too. @@ -42,6 +43,10 @@ * selection mismatch, host-limit rejection, shortfall next to a limit, * `info trace` for an array, an element and an unknown job, and the * file equal to the inline console traces. + * 4. bug-1932 (extra replay): 205's request equals 204's, which a full + * search proved unsatisfiable earlier in the same session; 205 pends + * from the unsatisfiable-request cache with no search, its trace names + * 204, and it pends with 204's full reasons (per-host included). */ #include #include @@ -665,11 +670,68 @@ static void test_compound_array_selection_and_info_trace(void) run_done(&r); } +/* ---------------------------------------------------------------------- */ +/* 4. A request already proved unsatisfiable this session (bug-1932) */ +/* ---------------------------------------------------------------------- */ + +/* The outcome section of a trace block: from its " outcome: " line on. */ +static const char* outcome_of(const char* block) +{ + const char* o = strstr(block, "\n outcome: "); + + TEST_ASSERT_NOT_NULL_MESSAGE(o, block); + return o; +} + +static void test_known_unsatisfiable_request_skipped_with_full_reasons(void) +{ + run_t r; + char* b204; + char* b205; + + run_repl(&r, EXTRA_REPLAY, + "trace on job 0-0-0-204 0-0-0-205 level full\n" + "step\n" + "step\n" + "step\n" + "quit\n"); + + /* Session 3: nothing else runs, so 204 passes the cluster-wide check + * and every host rejects it for mem: the reason is per-host only. */ + b204 = block_of(r.out, "[trace] job 0-0-0-204 session 3"); + IN_ORDER(b204, "\n cluster-wide check passed\n", + "\n host w01 rejected (per-host check): resource " + "shortfall\n", + "\n host w02 rejected (per-host check): resource " + "shortfall\n", + "\n outcome: PEND\n", "\n host w01: "); + + /* 205: no check and no plan, the trace names the job that proved the + * request unsatisfiable, and the pend reasons are 204's, in full. */ + b205 = block_of(r.out, "[trace] job 0-0-0-205 session 3"); + IN_ORDER(b205, + "\n no plan generated: skipped, same request as 0-0-0-204 " + "proved unsatisfiable earlier this session\n", + "\n outcome: PEND\n"); + TEST_ASSERT_NULL_MESSAGE(strstr(b205, "cluster-wide check"), b205); + TEST_ASSERT_NULL_MESSAGE(strstr(b205, "\n cluster-wide:"), b205); + TEST_ASSERT_NULL_MESSAGE(strstr(b205, " plan 1 "), b205); + TEST_ASSERT_EQUAL_STRING_MESSAGE( + outcome_of(b204), outcome_of(b205), + "bug-1932: a cache hit must pend with the full reasons of the job " + "that proved the request unsatisfiable"); + + free(b204); + free(b205); + run_done(&r); +} + int main(void) { UNITY_BEGIN(); RUN_TEST(test_step_mode_full_all_jobs_and_file_copy); RUN_TEST(test_continue_breakpoint_summary_and_info_trace); RUN_TEST(test_compound_array_selection_and_info_trace); + RUN_TEST(test_known_unsatisfiable_request_skipped_with_full_reasons); return UNITY_END(); } diff --git a/src/scheduler/testsuite/test_repl_trace_e2e_fixture.c b/src/scheduler/testsuite/test_repl_trace_e2e_fixture.c index e585fb2b..e14ea397 100644 --- a/src/scheduler/testsuite/test_repl_trace_e2e_fixture.c +++ b/src/scheduler/testsuite/test_repl_trace_e2e_fixture.c @@ -386,6 +386,9 @@ static uint64_t array_elem(uint32_t k) * 203 selects mem>20000 (w01 mismatches), pended by the * cluster-wide limit (6 of 6 in use) * 3000 after 201 and both elements finish: 202 and 203 are tried again + * 4000 after 202 and 203 finish: 204 and 205 (identical requests, more + * mem than any host has; bug-1932) pend in every session -- 204 + * by a full search, 205 by the unsatisfiable-request cache */ void write_extra_replay(const char* dest) { @@ -393,7 +396,7 @@ void write_extra_replay(const char* dest) char path[PATH_MAX]; FILE* f; xce_worker_t* workers[2]; - xce_job_t* jobs[5]; + xce_job_t* jobs[7]; snprintf(record_dir, sizeof(record_dir), "/tmp/sched_tre2e_gen_XXXXXX"); TEST_ASSERT_NOT_NULL(mkdtemp(record_dir)); @@ -420,7 +423,9 @@ void write_extra_replay(const char* dest) 1, 1); jobs[4] = make_push_job(203, 1104, "select[mem>20000] rusage[mem=512]", "alice", 1, 1); - append_jobs(f, 1003ULL, jobs, 5); + jobs[5] = make_push_job(204, 1105, "rusage[mem=999999]", "carol", 1, 1); + jobs[6] = make_push_job(205, 1106, "rusage[mem=999999]", "carol", 1, 1); + append_jobs(f, 1003ULL, jobs, 7); append_session(f, 2000ULL); append_state_change(f, 2500ULL, 201, JOB_STAT_RUN, JOB_STAT_DONE); @@ -429,6 +434,9 @@ void write_extra_replay(const char* dest) append_state_change(f, 2700ULL, array_elem(2), JOB_STAT_RUN, JOB_STAT_DONE); append_session(f, 3000ULL); + append_state_change(f, 3500ULL, 202, JOB_STAT_RUN, JOB_STAT_DONE); + append_state_change(f, 3600ULL, 203, JOB_STAT_RUN, JOB_STAT_DONE); + append_session(f, 4000ULL); append_end_of_stream(f); TEST_ASSERT_EQUAL_INT(0, fclose(f)); diff --git a/src/scheduler/testsuite/test_scheduler_cache_dispatch.c b/src/scheduler/testsuite/test_scheduler_cache_dispatch.c index d52a8cb4..3f4d7ff5 100644 --- a/src/scheduler/testsuite/test_scheduler_cache_dispatch.c +++ b/src/scheduler/testsuite/test_scheduler_cache_dispatch.c @@ -173,22 +173,24 @@ int __wrap_sched_meet_host_selection(xce_sched_core_t* core, static int g_lookup_cb_calls = 0; static int g_populate_cb_calls = 0; -static int spy_unsatisfiable_lookup_cb(xce_sched_core_t* core, +static int spy_unsatisfiable_lookup_cb(xce_sched_core_t* core, xce_sched_obj_t* obj, - int* out_reason_category) + xce_job_preason_t* out_reasons, + uint64_t* out_jobid) { g_lookup_cb_calls++; - return xce_alloc_cache_lookup_cb(core, obj, out_reason_category); + return xce_alloc_cache_lookup_cb(core, obj, out_reasons, out_jobid); } -static void spy_unsatisfiable_populate_cb(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int reason_category) +static void spy_unsatisfiable_populate_cb(xce_sched_core_t* core, + xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, + uint64_t jobid) { g_populate_cb_calls++; - xce_alloc_cache_populate_cb(core, obj, reason_category); + xce_alloc_cache_populate_cb(core, obj, reasons, jobid); } void setUp(void) { return; } @@ -451,6 +453,21 @@ static void test_standalone_primary_cache_hit_skips_allocator(void) decision2->u.reason->reasons->main_reason, "a cache hit must attribute the SAME pend reason category as the " "original unsatisfiable determination (Req 7.5)"); + /* bug-1932: job 1's real cause is per-host (the worker's slots + * shortfall), with main reason 0; a hit that keeps only the main + * reason pends job 2 with an EMPTY reason. */ + TEST_ASSERT_TRUE_MESSAGE( + (decision1->u.reason->reasons->nhost_reasons + + decision1->u.reason->reasons->nres_reasons) + > 0, + "fixture: job 1's exhausted attempt must record a per-host or " + "per-resource reason (its one worker has no free slots)"); + TEST_ASSERT_TRUE_MESSAGE( + job_preason_eq(decision1->u.reason->reasons, + decision2->u.reason->reasons, NULL), + "bug-1932: a cache hit must restore the original attempt's FULL " + "pend reason (main, per-host and per-resource), not only its main " + "reason"); TEST_ASSERT_EQUAL_INT_MESSAGE( 2, g_lookup_cb_calls, "job 2's cache hit must also be consulted through core->" diff --git a/src/scheduler/testsuite/test_scheduler_cache_dispatch_compound.c b/src/scheduler/testsuite/test_scheduler_cache_dispatch_compound.c index 706598c7..e2d5a790 100644 --- a/src/scheduler/testsuite/test_scheduler_cache_dispatch_compound.c +++ b/src/scheduler/testsuite/test_scheduler_cache_dispatch_compound.c @@ -132,22 +132,24 @@ int __wrap_sched_meet_host_selection(xce_sched_core_t* core, static int g_lookup_cb_calls = 0; static int g_populate_cb_calls = 0; -static int spy_unsatisfiable_lookup_cb(xce_sched_core_t* core, +static int spy_unsatisfiable_lookup_cb(xce_sched_core_t* core, xce_sched_obj_t* obj, - int* out_reason_category) + xce_job_preason_t* out_reasons, + uint64_t* out_jobid) { g_lookup_cb_calls++; - return xce_alloc_cache_lookup_cb(core, obj, out_reason_category); + return xce_alloc_cache_lookup_cb(core, obj, out_reasons, out_jobid); } -static void spy_unsatisfiable_populate_cb(xce_sched_core_t* core, - xce_sched_obj_t* obj, - int reason_category) +static void spy_unsatisfiable_populate_cb(xce_sched_core_t* core, + xce_sched_obj_t* obj, + const xce_job_preason_t* reasons, + uint64_t jobid) { g_populate_cb_calls++; - xce_alloc_cache_populate_cb(core, obj, reason_category); + xce_alloc_cache_populate_cb(core, obj, reasons, jobid); } void setUp(void) { return; } @@ -412,6 +414,11 @@ static void test_compound_nested_primary_cache_hit_skips_allocator(void) "a cache hit must attribute the SAME pend reason category as the " "original unsatisfiable determination (Req 7.5), even across a " "compound-nested primary"); + TEST_ASSERT_TRUE_MESSAGE( + job_preason_eq(decisionA->u.reason->reasons, + decisionB->u.reason->reasons, NULL), + "bug-1932: a cache hit must restore the original attempt's FULL " + "pend reason, even across a compound-nested primary"); TEST_ASSERT_EQUAL_INT_MESSAGE( 2, g_lookup_cb_calls, "job B's cache hit must also be consulted through core->" diff --git a/src/scheduler/testsuite/test_sim_cache_clear_isolation.c b/src/scheduler/testsuite/test_sim_cache_clear_isolation.c index bb46244f..407ac83b 100644 --- a/src/scheduler/testsuite/test_sim_cache_clear_isolation.c +++ b/src/scheduler/testsuite/test_sim_cache_clear_isolation.c @@ -37,6 +37,13 @@ * REAL xce_core_begin_session() (core_session.c's public entry point), * and asserts alt_events_file is bit-for-bit unchanged across the call. * + * bug-1932 update: the table now lives on xce_sched_core_t and the + * simulator registers the same hook as normal mode (from its + * sched_sim_args_t). The NULL-hook test below still guards the no-hook + * path; test_sim_registered_cache_clear_uses_the_core_table guards the + * registered one: clearing builds the core's table and leaves + * alt_events_file alone. + * * ── Link technique ─────────────────────────────────────────────────────── * Same technique as the sibling test_core_session_cache_clear.c (round 1): * this fixture's meson.build entry lists core.c/core_clock.c/ @@ -107,10 +114,9 @@ static void test_sim_begin_session_never_corrupts_alt_events_file(void) TEST_ASSERT_NOT_NULL(f); sim.alt_events_file = f; - /* Deliberately NOT calling xce_core_set_cache_clear_cb() here -- - * mirrors sim_loop.c exactly, which never registers this hook (Req 5 - * AC 17 "Simulation isolation": sim mode has no cache to clear). This - * is the crux of the regression: before round 2's fix, + /* Deliberately NOT calling xce_core_set_cache_clear_cb() here -- the + * no-hook path (a core whose driver registered no cache). This is the + * crux of the regression: before round 2's fix, * core_begin_session_internal() called cache_clear() unconditionally, * so linking alloc_cache.c's real, strong cache_clear(xce_scheduler_t*) * into this binary was enough to corrupt alt_events_file below even @@ -132,9 +138,53 @@ static void test_sim_begin_session_never_corrupts_alt_events_file(void) fclose(f); } +/* + * bug-1932: the simulator now registers the SAME cache hooks as normal mode + * (sched_sim_driver_start(), from the driver's sched_sim_args_t). That is + * safe only because the cache table lives on xce_sched_core_t and no hook + * casts the core to a driver struct: clearing at session start must build + * the core's own table and still leave every sim-only field alone. + */ +static void test_sim_registered_cache_clear_uses_the_core_table(void) +{ + xce_sched_sim_driver_t sim; + FILE* f; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + f = tmpfile(); + TEST_ASSERT_NOT_NULL(f); + sim.alt_events_file = f; + + rc = xce_core_set_cache_clear_cb(&(sim.core), xce_alloc_cache_clear_cb); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = xce_core_begin_session(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + TEST_ASSERT_EQUAL_PTR_MESSAGE( + f, sim.alt_events_file, + "the registered cache_clear_cb must clear the core's own table, " + "never reinterpret a simulation driver field"); + TEST_ASSERT_NOT_NULL_MESSAGE( + sim.core.unsatisfiable_cache, + "bug-1932: a session start must build the core's cache table"); + + rc = xce_core_end_session(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + fclose(f); + /* core fini needs the driver-owned pend_updates queues this fixture + * never builds; free the one thing this test made */ + destroy(sim.core.unsatisfiable_cache); +} + int main(void) { UNITY_BEGIN(); RUN_TEST(test_sim_begin_session_never_corrupts_alt_events_file); + RUN_TEST(test_sim_registered_cache_clear_uses_the_core_table); return UNITY_END(); } diff --git a/src/scheduler/testsuite/test_sim_verify.c b/src/scheduler/testsuite/test_sim_verify.c index 9041c297..1c240388 100644 --- a/src/scheduler/testsuite/test_sim_verify.c +++ b/src/scheduler/testsuite/test_sim_verify.c @@ -733,11 +733,8 @@ static void test_pend_decisions_differing_main_reason_mismatch(void) /* * test_pend_decisions_differing_reason_detail_still_match - bug-1845 * regression: the recorded (expected) side represents a live cache-hit - * PEND (core.h's documented "simulation isolation" -- sim/replay never - * registers core->unsatisfiable_lookup_cb, so it can never itself produce - * this shape, only ever receive it from a recorded normal-mode run) -- - * main_reason set, zero host/resource reason detail - * (sched_pend_from_cached_reason_(), scheduler.c, never adds any). The + * PEND recorded before bug-1932 -- main_reason set, zero host/resource + * reason detail (the pre-fix cache stored only the main reason). The * live (actual) side represents replay's own full per-host computation for * the SAME job/round -- same main_reason, but with real host- and * resource-based reason detail attached (sched_try_alloc_on_host_(), diff --git a/src/scheduler/testsuite/test_trace_render.c b/src/scheduler/testsuite/test_trace_render.c index 1dc8cf84..10795658 100644 --- a/src/scheduler/testsuite/test_trace_render.c +++ b/src/scheduler/testsuite/test_trace_render.c @@ -1021,6 +1021,51 @@ static void test_no_decision_pend_without_reason_keeps_hierarchy(void) sched_trace_rec_free(rec); } +/* bug-1932: a request the unsatisfiable-request cache already holds is + * skipped; the line names the job whose search proved it, or says so + * without a name when that job is unknown (0). */ +static void test_known_unsatisfiable_names_source_job(void) +{ + static const char* const rn[] = { "slots" }; + static const float rv[] = { 1.0f }; + xce_sched_trace_rec_t* rec; + fx_primary_t p; + char* out; + + rec = new_rec(XCE_TRACE_LEVEL_FULL, XCE_TRACE_LINE_BOUND_DEFAULT, + XCE_TRACE_REC_DECISION); + TEST_ASSERT_NOT_NULL(rec); + set_selection(rec); + set_glimits(rec, 0, NULL, NULL); + make_primary(&p, 1, 1, 1, rn, rv, NULL); + + sched_trace_path_push(rec, XCE_TRACE_PATH_SINGLE, 1, 1); + sched_trace_resreq_begin(rec, &p.p, 0); + /* the hit returns before the plan search: no search end follows */ + sched_trace_known_unsat(rec, (uint64_t)204); + sched_trace_known_unsat(rec, 0); + sched_trace_resreq_end(rec, 0, 0); + sched_trace_path_pop(rec); + sched_trace_rec_finish(rec, XCE_TRACE_REC_PEND_NO_REASON, 0); + + TEST_ASSERT_EQUAL_INT(1, rec->counters.reqs_none_generated); + + out = render(rec, XCE_REPL_UNITS_RAW); + assert_golden( + "[trace] job 1-2-1000-42 session 7 position 3 level full\n" + SEL_LINE NO_LIMITS_LINE + " requirement single tasks 1..1 requests slots=1\n" + " no plan generated: skipped, same request as 0-0-0-204 proved " + "unsatisfiable earlier this session\n" + " no plan generated: skipped, same request proved unsatisfiable " + "earlier this session\n" + " single requirement not satisfied\n" + " no decision: job pended without any pend reason\n", + out); + free(out); + sched_trace_rec_free(rec); +} + static void test_reconstructed_record(void) { xce_sched_trace_rec_t* rec; @@ -1318,6 +1363,7 @@ int main(void) RUN_TEST(test_no_decision_no_schedobj); RUN_TEST(test_no_decision_internal_error_keeps_hierarchy); RUN_TEST(test_no_decision_pend_without_reason_keeps_hierarchy); + RUN_TEST(test_known_unsatisfiable_names_source_job); RUN_TEST(test_reconstructed_record); RUN_TEST(test_selection_and_limits_unavailable); RUN_TEST(test_entry_kinds_and_edge_cases);