diff --git a/.kiro/specs/scheduler-allocation/tasks.md b/.kiro/specs/scheduler-allocation/tasks.md index a2539e3c..a8ac9c20 100644 --- a/.kiro/specs/scheduler-allocation/tasks.md +++ b/.kiro/specs/scheduler-allocation/tasks.md @@ -30,7 +30,7 @@ - Observable: a passing test suite covers select-expression filtering, availability filtering, exhaustion signaling, and the too-small-then-fits-smaller-search case - _Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6_ - _Boundary: host_iter.c_ -- [ ] 2.2 (P) Implement and test the unsatisfiable-resreq cache +- [x] 2.2 (P) Implement and test the unsatisfiable-resreq cache - Implement the cache's key type and its hash/comparison/copy/release operations, backed by the existing keyed-table infrastructure, storing a reason category (not a bare present/absent marker) as each entry's value - Implement lookup, insert, and clear operations against the cache - Write a unit test proving a lookup on an identical fingerprint-plus-identity is a hit that returns the same reason category given at insert time, a lookup on any differing field is a miss, and clear empties all entries @@ -106,6 +106,7 @@ ## Implementation Notes +- (2.2) `xce_scheduler_t.unsatisfiable_cache` (constructed in `sched_init()`/destroyed in `sched_fini()`, matching `xce_select.c`'s `sel_node_table_init()` construction convention exactly) lives on the outer `xce_scheduler_t`, per Req 7 AC1. Task 5.2's clear call site (`core_begin_session_internal()`, `core_session.c`) only receives `xce_sched_core_t* core` — since `core` is confirmed the first field of `xce_scheduler_t`, task 5.2 should recover the outer struct via `(xce_scheduler_t*)core` (valid, defined C behavior), not by widening the function's signature. - (2.1) `xce_alloc_plan_t` gained a `ctx` field (`xce_sched_ctx_t*`) so `host_iter_init()` can reach `ctx->sched`/`ctx->primary->select`/`ctx->alloc_req.host` — `host_iter_init()` rejects a plan with `ctx == NULL` with `-EINVAL` rather than crashing. **Tasks 3.1 and 4.1 must set `plan->ctx = ctx` on every plan their plan-iterator returns from `next()`** (not just once) before constructing a `host_iter` from it — a plan-iterator that reuses one internal buffer across calls/jobs must refresh this field each time, since a stale-but-non-NULL pointer from a torn-down `ctx` is not caught by the NULL guard. - (1.4) When a new scheduler test needs a real function from `schedobj.c`/`core.c` (e.g. `sched_ctx_get_uncomit_host()`), white-box `#include` the real source file rather than privately reimplementing its logic in the test — established precedent: `test_picker_config_fields.c`'s `#include "../src/core.c"` + `#include "../src/schedobj.c"` pairing. A private reimplementation may pass today but silently drifts from the real function with nothing to catch it later. - (1.1) `xce_host_iter_t.next()` returns `xce_sched_worker_t*` (`sched/types.h`, carries `resvals`/`resInUse`/`host_eval_ctx`) — never `xce_worker_t*` (`xce/types.h`, the wire/config type `make_sched_worker()` converts FROM). requirements.md/design.md briefly had this wrong; fixed in commit `d1a3e158`. Task 2.1 (`host_iter.c`'s real implementation) and any later task touching worker candidates must use `xce_sched_worker_t*` from the start. diff --git a/src/scheduler/include/sched/alloc_cache.h b/src/scheduler/include/sched/alloc_cache.h new file mode 100644 index 00000000..49d4c7e0 --- /dev/null +++ b/src/scheduler/include/sched/alloc_cache.h @@ -0,0 +1,134 @@ +#if !defined(_SCHED_ALLOC_CACHE_H_) + +#define _SCHED_ALLOC_CACHE_H_ 1 + +/* + * alloc_cache.h - session-level unsatisfiable-resreq cache (scheduler- + * allocation spec, task 2.2, Req 7.1, 7.3, 7.4, 7.5). + * + * A per-session memoization of resreqs this session already proved + * unsatisfiable: a job whose fingerprint + identity matches a previous + * exhausted allocation attempt can be pended immediately, without paying + * full plan/host iteration cost again. + * + * 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. + * + * 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 + * (cache_key_ops, below), alongside kv.c's int_key_ops/string_key_ops -- + * neither of those handles a composite struct key. See kv.c's + * kvtab_find()/kvtab_add(): key_find() returns >0 on a hit, -ENOENT on a + * miss, and does not allocate; key_add() allocates the heap-owned key copy + * (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. + */ + +#include + +#include "scheduler.h" + +/* + * xce_cache_key_t - full cache key: the pre-computed resreq fingerprint + * (xce_resreq_fingerprint_t, sched/types.h) plus job identity. Identity + * fields are xstring-interned pointers (xce_sched_obj_t.user/queue/ + * project) -- not owned, never freed by this cache; pointer equality + * already implies content equality for xstring-interned strings (see + * xstring.c), so no strcmp() is needed or performed. + */ +typedef struct { + xce_resreq_fingerprint_t resreq; + const char* user; /* xstring pointer, borrowed */ + const char* queue; /* xstring pointer, borrowed */ + const char* project; /* xstring pointer, borrowed */ +} xce_cache_key_t; + +/* + * 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. + */ +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 + * 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. + * + * 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). + * <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); + +/* + * 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). + * + * 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). + */ +extern int cache_insert(xce_scheduler_t* sched, xce_sched_obj_t* obj, + int reason_category); + +/* + * 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. + * + * 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. + * 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); + +#endif diff --git a/src/scheduler/include/scheduler.h b/src/scheduler/include/scheduler.h index feec1a6f..da3c0231 100644 --- a/src/scheduler/include/scheduler.h +++ b/src/scheduler/include/scheduler.h @@ -138,6 +138,27 @@ 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/meson.build b/src/scheduler/meson.build index 900196ea..d5cae2b0 100644 --- a/src/scheduler/meson.build +++ b/src/scheduler/meson.build @@ -28,6 +28,7 @@ scheduler_srcs = [ 'src/schedobj.c', 'src/plugin.c', 'src/resource.c', + 'src/alloc_cache.c', 'src/plugins/fcfs.c', 'src/plugins/builtin_fcfs.c', 'src/plugins/seqjob_allocator.c', diff --git a/src/scheduler/src/alloc_cache.c b/src/scheduler/src/alloc_cache.c new file mode 100644 index 00000000..61cd6168 --- /dev/null +++ b/src/scheduler/src/alloc_cache.c @@ -0,0 +1,222 @@ +/* + * alloc_cache.c - session-level unsatisfiable-resreq cache (scheduler- + * allocation spec, task 2.2, Req 7.1, 7.3, 7.4, 7.5). + * + * See sched/alloc_cache.h for the full design rationale. This file owns: + * + * - cache_key_hash_/cache_key_cmp_/cache_key_assign_/cache_key_destroy_ + * and the cache_key_ops (kv_key_ops_t) binding them together -- a + * THIRD kv_key_ops_t instance alongside kv.c's int_key_ops/ + * 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. + * + * 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 + * requirement is `cmp(...) == 0` for a match, but an earlier draft of this + * function (caught during design review; see requirements.md's "New types + * (Req 7)" section) returned a BOOLEAN (1 for a match, via &&-chained == + * comparisons) -- exactly inverted from this contract. As originally + * written, every lookup would have been a false miss, silently disabling + * the entire cache without failing any test that doesn't specifically + * assert a hit. This is exactly why test_alloc_cache.c includes a + * same-key-twice HIT assertion, not only an insert-then-miss-on-a- + * different-key round-trip (which the inverted polarity would still have + * passed). + */ +#include +#include +#include +#include + +#include "sched/alloc_cache.h" + +/* + * cache_key_hash_ - FNV-1a 32-bit over the fixed-size resreq fingerprint + * bytes (all fields are fixed-size integers/IDs, no pointers, no deep + * compare needed), folded with the three identity pointers. No existing + * hash-update helper in this codebase handles a composite struct key; this + * is new, self-contained code. + */ +static uint32_t cache_key_hash_(void* k) +{ + const xce_cache_key_t* key = (const xce_cache_key_t*)k; + uint32_t h = 2166136261u; /* FNV-1a 32-bit offset basis */ + const unsigned char* p = (const unsigned char*)&key->resreq; + size_t i; + + for (i = 0; i < sizeof(key->resreq); i++) { + h = (h ^ p[i]) * 16777619u; + } + + h = (h ^ (uint32_t)(uintptr_t)key->user) * 16777619u; + h = (h ^ (uint32_t)(uintptr_t)key->queue) * 16777619u; + h = (h ^ (uint32_t)(uintptr_t)key->project) * 16777619u; + + return h; +} + +/* + * cache_key_cmp_ - three-way comparator, 0 == equal. See this file's + * header comment for why this polarity (not a boolean) is required. + * memcmp() on the fixed-size resreq portion already returns a three-way + * result; the identity pointers use the same shape via plain pointer + * ordering (pointer equality already implies content equality for + * xstring-interned strings -- no strcmp() needed or performed). + */ +static int cache_key_cmp_(void* a, void* b) +{ + const xce_cache_key_t* ka = (const xce_cache_key_t*)a; + const xce_cache_key_t* kb = (const xce_cache_key_t*)b; + int rc; + + rc = memcmp(&ka->resreq, &kb->resreq, sizeof(ka->resreq)); + if (rc != 0) { + return rc; + } + + if (ka->user != kb->user) { + return (ka->user < kb->user) ? -1 : 1; + } + if (ka->queue != kb->queue) { + return (ka->queue < kb->queue) ? -1 : 1; + } + if (ka->project != kb->project) { + return (ka->project < kb->project) ? -1 : 1; + } + + return 0; +} + +/* + * cache_key_assign_ - kvtab keys are heap-owned by the table (see kv.c's + * string_key_ops strdup-on-insert precedent); *dest must be a fresh heap + * copy of the stack-constructed key passed at the lookup site, never an + * alias of it. + */ +static int cache_key_assign_(void** dest, void* src) +{ + xce_cache_key_t* copy = (xce_cache_key_t*)malloc(sizeof(xce_cache_key_t)); + + if (copy == NULL) { + return -ENOMEM; + } + + memcpy(copy, src, sizeof(xce_cache_key_t)); + *dest = copy; + + return 0; +} + +/* + * cache_key_destroy_ - frees only the key struct itself; user/queue/ + * project are borrowed xstring pointers (not owned, not freed here). + */ +static void cache_key_destroy_(void* k) { free(k); } + +kv_key_ops_t cache_key_ops = { + .hash = cache_key_hash_, + .assign = cache_key_assign_, + .cmp = cache_key_cmp_, + .destroy = cache_key_destroy_, +}; + +/* + * build_key_ - construct the full cache key on the stack from a primary + * scheduling object's pre-computed resreq fingerprint (Req 7.1, populated + * at job-ingest time) plus its identity fields. + * + * Returns 0 on success, -EINVAL if obj is not a populated primary resreq + * scheduling object. + */ +static int build_key_(xce_cache_key_t* key, xce_sched_obj_t* obj) +{ + if ((obj->tag != XCE_RESREQ_PRIMARY) || (obj->u.primary == NULL)) { + return -EINVAL; + } + + key->resreq = obj->u.primary->resreq_fp; + key->user = obj->user; + key->queue = obj->queue; + key->project = obj->project; + + return 0; +} + +int cache_lookup(xce_scheduler_t* sched, xce_sched_obj_t* obj, + int* out_reason_category) +{ + xce_cache_key_t key; + void* val = NULL; + int rc; + + if ((sched == NULL) || (obj == NULL) || (out_reason_category == NULL)) { + return -EINVAL; + } + + rc = build_key_(&key, obj); + if (rc < 0) { + return rc; + } + + if (sched->unsatisfiable_cache == NULL) { + /* Cache not (yet) constructed -- treat as an unconditional miss + * rather than an error. */ + return 0; + } + + rc = key_find(sched->unsatisfiable_cache, &key, &val); + if (rc <= 0) { + return 0; + } + + *out_reason_category = (int)(intptr_t)val; + + return 1; +} + +int cache_insert(xce_scheduler_t* sched, xce_sched_obj_t* obj, + int reason_category) +{ + xce_cache_key_t key; + int rc; + + if ((sched == NULL) || (obj == NULL)) { + return -EINVAL; + } + + rc = build_key_(&key, obj); + if (rc < 0) { + return rc; + } + + if (sched->unsatisfiable_cache == NULL) { + return -EINVAL; + } + + rc = key_add(sched->unsatisfiable_cache, &key, + (void*)(intptr_t)reason_category); + if (rc < 0) { + return rc; + } + + return 0; +} + +void cache_clear(xce_scheduler_t* sched) +{ + kv_params_t params = { .nslots = XCE_UNSAT_CACHE_NSLOTS, + .keyops = &cache_key_ops, + .datakiller = NULL }; + + if (sched == NULL) { + return; + } + + if (sched->unsatisfiable_cache != NULL) { + destroy(sched->unsatisfiable_cache); + } + + sched->unsatisfiable_cache = makeInstance(&kvtab_type, ¶ms); +} diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index e292549d..7a8bbba5 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -6,6 +6,8 @@ #include #include +#include + #include "client.h" #include "controller.capnp.h" #include "decision.h" @@ -15,6 +17,7 @@ #include "preason.h" #include "resobj.h" #include "resreq.h" +#include "sched/alloc_cache.h" #include "scheduler.h" #include "utils.h" #include "xcealloc.h" @@ -655,6 +658,24 @@ 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); + } + /* 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 * CLOCK_MONOTONIC-since-boot nanoseconds, not epoch time -- every @@ -812,6 +833,16 @@ 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) -- diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index f883c407..47262af7 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -1510,6 +1510,23 @@ sched_host_iter_test = executable( dependencies: [scheduler_test_deps], install: false) +# Unit test for the session-level unsatisfiable-resreq cache +# (scheduler-allocation spec, task 2.2, Req 7.1, 7.3, 7.4, 7.5). White-box +# TU (#includes ../src/alloc_cache.c directly, same convention as +# sched_host_iter_test above) -- never touches xce_sched_core_init(), since +# alloc_cache.c only depends on kv.h + scheduler.h/sched/types.h struct +# definitions, not on core.c's collections. Covers: same-key-twice hit +# (the cache_key_cmp_() three-way-vs-boolean polarity regression guard), +# a miss on every individually-differing fingerprint/identity field, +# reason-category round-trip across two distinct categories, and +# clear-empties-all-entries. +sched_alloc_cache_test = executable( + 'sched_alloc_cache_test', + ['test_alloc_cache.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + # Register as meson tests so `meson test --suite scheduler` runs them all. # detect_leaks=0 matches the suite convention (fixtures are not torn down). test('scheduler test', scheduler_test, suite: 'scheduler', @@ -1698,3 +1715,6 @@ test('scheduler alloc plan iter test', test('scheduler host iter test', sched_host_iter_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +test('scheduler alloc cache test', + sched_alloc_cache_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/scheduler/testsuite/test_alloc_cache.c b/src/scheduler/testsuite/test_alloc_cache.c new file mode 100644 index 00000000..0d2de76d --- /dev/null +++ b/src/scheduler/testsuite/test_alloc_cache.c @@ -0,0 +1,440 @@ +/* + * 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). + * + * White-box test TU: #includes ../src/alloc_cache.c directly (same + * convention test_host_iter.c/test_job_update.c/test_handlers.c already + * establish for this test suite), so it exercises the REAL + * 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. + * + * Covers, per this task's Observable line: + * - 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)" + * section -- would still have passed). This is deliberately the FIRST + * test below. + * - 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. + */ +#include +#include +#include +#include + +#include + +#include "sched/alloc_cache.h" +#include "scheduler.h" + +/* Pull in the real cache_key_ops/cache_lookup/cache_insert/cache_clear + * implementation under test. */ +#include "../src/alloc_cache.c" + +void setUp(void) { return; } +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) +{ + 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); +} + +static void fini_sched(xce_scheduler_t* sched) +{ + if (sched->unsatisfiable_cache != NULL) { + destroy(sched->unsatisfiable_cache); + sched->unsatisfiable_cache = NULL; + } +} + +/* + * init_primary_obj - a minimal, valid xce_sched_obj_t/xce_primary_schedobj_t + * pair: tag == XCE_RESREQ_PRIMARY, u.primary pointing at `primary`, with + * `primary`'s resreq_fp set to the given fingerprint fields and the + * object's own user/queue/project identity pointers set. Every other field + * (owner/resreq/reason on primary; glimits/hlimits/alloc/reason on obj) is + * left zeroed -- cache_lookup()/cache_insert() never touch them. + */ +static void init_primary_obj(xce_sched_obj_t* obj, xce_primary_schedobj_t* primary, + int min_tasks, int max_tasks, + uint32_t select_node_id, uint64_t rusage_hash, + const char* user, const char* queue, + const char* project) +{ + memset(obj, 0, sizeof(*obj)); + memset(primary, 0, sizeof(*primary)); + + primary->resreq_fp.min_tasks = min_tasks; + primary->resreq_fp.max_tasks = max_tasks; + primary->resreq_fp.select_node_id = select_node_id; + primary->resreq_fp.rusage_hash = rusage_hash; + + obj->tag = XCE_RESREQ_PRIMARY; + obj->u.primary = primary; + obj->user = (char*)user; + obj->queue = (char*)queue; + obj->project = (char*)project; +} + +/* 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 + * inequality is unambiguous regardless of any compiler string-literal + * pooling -- this cache's cmp is pointer-based, exactly mirroring the real + * xstring-interned-pointer contract identity fields carry in production. */ +#define BASE_MIN_TASKS 1 +#define BASE_MAX_TASKS 4 +#define BASE_SELECT_NODE_ID 100u +#define BASE_RUSAGE_HASH ((uint64_t)0xdeadbeefULL) +static const char BASE_USER[] = "alice"; +static const char BASE_QUEUE[] = "normal"; +static const char BASE_PROJECT[] = "proj1"; + +/* ---------------------------------------------------------------------- */ +/* Tests */ +/* ---------------------------------------------------------------------- */ + +/* + * THE critical test (design.md's own words: "a functionally dead cache + * 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 + * 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; + + init_sched(&sched); + + init_primary_obj(&obj1, &primary1, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + init_primary_obj(&obj2, &primary2, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + + rc = cache_insert(&sched, &obj1, category); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = cache_lookup(&sched, &obj2, &out); + TEST_ASSERT_EQUAL_INT(1, rc); + TEST_ASSERT_EQUAL_INT(category, out); + + fini_sched(&sched); +} + +/* + * A lookup before any insert is a plain miss. + */ +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; + + init_sched(&sched); + + 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); + TEST_ASSERT_EQUAL_INT(0, rc); + + fini_sched(&sched); +} + +/* + * 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. + */ +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; + + init_sched(&sched); + + init_primary_obj(&base_obj, &base_primary, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + 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); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = cache_lookup(&sched, &variant_obj, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + + fini_sched(&sched); +} + +static void test_miss_on_differing_min_tasks(void) +{ + assert_variant_misses(BASE_MIN_TASKS + 1, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); +} + +static void test_miss_on_differing_max_tasks(void) +{ + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS + 1, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); +} + +static void test_miss_on_differing_select_node_id(void) +{ + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID + 1, BASE_RUSAGE_HASH, + BASE_USER, BASE_QUEUE, BASE_PROJECT); +} + +static void test_miss_on_differing_rusage_hash(void) +{ + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH ^ 0x1ULL, + BASE_USER, BASE_QUEUE, BASE_PROJECT); +} + +static void test_miss_on_differing_user(void) +{ + static const char other_user[] = "bob"; + + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, other_user, + BASE_QUEUE, BASE_PROJECT); +} + +static void test_miss_on_differing_queue(void) +{ + static const char other_queue[] = "priority"; + + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + other_queue, BASE_PROJECT); +} + +static void test_miss_on_differing_project(void) +{ + static const char other_project[] = "proj2"; + + assert_variant_misses(BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, other_project); +} + +/* + * 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. + */ +static void test_reason_category_round_trip_two_distinct_categories(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; + + init_sched(&sched); + + 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); + + rc = cache_lookup(&sched, &obj_a, &out_a); + TEST_ASSERT_EQUAL_INT(1, rc); + TEST_ASSERT_EQUAL_INT(11, out_a); + + rc = cache_lookup(&sched, &obj_b, &out_b); + TEST_ASSERT_EQUAL_INT(1, rc); + TEST_ASSERT_EQUAL_INT(99, out_b); + + fini_sched(&sched); +} + +/* + * cache_clear() empties every entry: insert two distinct keys, clear, then + * confirm BOTH now miss (not just the one inserted last). + */ +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; + + init_sched(&sched); + + 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); + 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); + + TEST_ASSERT_EQUAL_INT(0, cache_insert(&sched, &obj_a, 1)); + TEST_ASSERT_EQUAL_INT(0, cache_insert(&sched, &obj_b, 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)); + + cache_clear(&sched); + TEST_ASSERT_NOT_NULL(sched.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); + + fini_sched(&sched); +} + +/* + * Re-inserting the identical key without an intervening clear propagates + * kvtab_add()'s own -EEXIST contract -- exercising cache_insert()'s error + * path. + */ +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; + + init_sched(&sched); + + init_primary_obj(&obj1, &primary1, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + init_primary_obj(&obj2, &primary2, 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(&sched, &obj1, 5)); + rc = cache_insert(&sched, &obj2, 6); + TEST_ASSERT_EQUAL_INT(-EEXIST, rc); + + fini_sched(&sched); +} + +/* + * Defensive argument checking: NULL sched/obj/out_reason_category, and a + * non-primary tag object, are all rejected rather than crashing. + */ +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; + + init_sched(&sched); + init_primary_obj(&obj, &primary, BASE_MIN_TASKS, BASE_MAX_TASKS, + BASE_SELECT_NODE_ID, BASE_RUSAGE_HASH, BASE_USER, + BASE_QUEUE, BASE_PROJECT); + + 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(&sched, &non_primary_obj, &out)); + + 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)); + + /* cache_clear(NULL) must not crash. */ + cache_clear(NULL); + + fini_sched(&sched); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_same_key_twice_is_a_hit); + RUN_TEST(test_lookup_before_any_insert_is_a_miss); + RUN_TEST(test_miss_on_differing_min_tasks); + RUN_TEST(test_miss_on_differing_max_tasks); + RUN_TEST(test_miss_on_differing_select_node_id); + RUN_TEST(test_miss_on_differing_rusage_hash); + 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_clear_empties_all_entries); + RUN_TEST(test_insert_duplicate_key_returns_eexist); + RUN_TEST(test_argument_validation); + return UNITY_END(); +}