From f4103e0efd2bf6260dbc49b2ec82b95c04bb304d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 31 Aug 2026 07:22:16 +0000 Subject: [PATCH] fix(scheduler): charge unreserved jobs' real memory usage too A job with no rusage[mem=...] request (reserved=0) was being SKIPPED entirely during reservation-vs-usage reconciliation, on the mistaken assumption that "no reservation" meant "nothing needs charging". It doesn't: this codebase's admission check has no live OS read at decision time (sched_check_host_satisfy_(), resource.c) -- it compares the worker's last-reported "mem" value against this scheduler's OWN tracked resInUse tally, built entirely from summed xce_res_account_t entries. Skipping an unreserved job left its real, live memory consumption permanently invisible to that tally -- an actual over-subscription risk (the scheduler could admit more onto a host than it can support, believing memory was free that a live process was genuinely using), not merely a display gap. Fix: charged = max(reserved, actual) applies to EVERY job, reserved or not. ensure_host_account_by_resname() (resource.c) now creates a fresh reserved=0 account entry (resolving residx via key_find() against core->resmgr, same pattern sched_update_res_accounts() already uses for a fresh account) instead of skipping when a job has no existing entry for a resource. Returns NULL (a genuine no-op) only when the resource isn't registered in the cluster's config at all. Rewrote resource-accounting.md to correct the mechanism it documented: "available" is NOT a live OS read (mem vs the similarly-named but static maxmem was worth naming explicitly too) -- it's worker's last-reported mem minus this scheduler's own resInUse tally, and that tally must include every job's real usage regardless of reservation. Also documented a known, pre-existing property surfaced while correcting this: since mem's own worker report already reflects real consumption and the scheduler's charge is a separate layer that doesn't get cleared on a fresh report, tracking can become conservative over time (under-utilization risk, never over- subscription) -- the same direction slots already accepts, and a separate, larger question than this fix's scope. Replaced test_reconcile_job_resource_usage_skips_when_no_mem_request (asserted the now-wrong skip behavior) with two tests: one proving an unreserved job's real usage is created and charged, one proving a genuine no-op when "mem" isn't a registered resource at all. Full scheduler suite: 55/55 passing, zero regressions. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA --- .kiro/steering/resource-accounting.md | 230 ++++++++++++------ src/scheduler/src/resource.c | 93 ++++++- .../testsuite/test_resource_accounting.c | 74 +++++- 3 files changed, 304 insertions(+), 93 deletions(-) diff --git a/.kiro/steering/resource-accounting.md b/.kiro/steering/resource-accounting.md index 2a92a5da..19e12935 100644 --- a/.kiro/steering/resource-accounting.md +++ b/.kiro/steering/resource-accounting.md @@ -24,16 +24,83 @@ to a cumulative counter like `cpu_time` (a usage metric reported for accounting/billing, not a capacity resource the allocator schedules against). +**`mem` is not `maxmem`.** These are two distinct, similarly-named +per-host resources, and it is easy to reach for the wrong one: + +- `maxmem` is a **static** value, self-reported once by the worker at + join time and never refreshed — the host's total installed physical + memory. +- `mem` is a **dynamic load index** (`resource_sync.c`'s + `resource_sync_register_load_index("mem", ...)`, refreshed on the + worker's regular load-report cadence, same family as `r1m`/`r15m`/`swp`). + Its value is `node_info_get_mem()` -> `read_memory_info()`'s `mem_free` + — the OS's own currently-available-physical-memory reading, in bytes. + +This whole document is about `mem`, never `maxmem`. Admission control cares +about what's free *right now*, not the host's total capacity. + +## How admission control actually reads "available" (read this before the formula) + +The scheduler's admission check does **not** query the OS live at +decision time. `sched_check_host_satisfy_()` (`resource.c`) reads two +already-in-memory arrays and hands them to `unsatisfy_()`: + +```c +total = get_resvals(&(w->resvals), ...); // the worker's last-reported "mem" value +inuse = get_resvals(&(w->resInUse), ...); // THIS scheduler's own running charge tally +... +if (total[i] < inuse[i] + uncomitted[i] + wanted[i]) { /* can't admit */ } +``` + +`total` for `mem` is whatever the worker last pushed (`mem_free` as of that +report — already a live OS reading, just not read synchronously at +decision time). `inuse` is built *entirely* from this scheduler's own +bookkeeping: `sched_account_jobs()` re-derives it every scheduling round by +summing `used + reserved` (`sched_update_res_accounts()`) across every +`xce_res_account_t` belonging to every currently-`RUN` job, for the job's +**entire lifetime** — not just until the worker's next report catches up. +This is the same mechanism `slots` has always used; `mem` uses it too, it +just also happens to have a `total` that independently drifts on its own +schedule. + +**Available memory for scheduling, precisely:** + +``` +available = w->resvals["mem"] (worker's last-reported mem_free) + - w->resInUse["mem"] (this scheduler's own running charge tally) +``` + +where `w->resInUse["mem"]` is the sum, over every job on that host, of +`max(reserved_j, used_j)` (see the policy below for how each job's term is +computed). + +**Known, pre-existing property worth naming, not a bug this document is +introducing:** because `mem`'s own worker-reported figure already reflects +real OS-level consumption from every process on the host (dispatched by +this scheduler or not), and the scheduler's reservation charge is a +*separate, additive* layer that isn't cleared when a fresh `mem` report +arrives (`core_apply_worker_resvals()`/`RESVALS_DELTA`, `core_events.c`, +overwrites `w->resvals` wholesale but never touches `w->resInUse`), a +running job's footprint can end up reflected twice — once because the +worker's next `mem` report is already lower, and again because this +scheduler's own charge for that job hasn't gone anywhere. This makes +admission control *more conservative* over time (under-utilization risk), +never less (it cannot cause over-subscription) — the same direction this +codebase already accepts for `slots`, which has no live ground truth to +reconcile against at all. Untangling that fully (e.g. reconciling +`resInUse` against a fresh worker push instead of letting both layers +accumulate independently) is a separate, larger architectural question, +out of scope here. + ## The policy 1. **Request.** A job may submit `rusage[mem=4M]`. If it doesn't, treat the - reservation as 0 — the job made no promise about memory and gets none of - the protection described below. + reservation as 0 — the job made no promise about memory, but see step 4: + a reservation of 0 does **not** mean nothing gets charged. 2. **Admission.** Before dispatching to a candidate host, the scheduler - checks the host has at least 4MB of *available* memory (see the - available-memory formula below — this is not simply the OS's raw free - figure). + checks the host has at least 4MB of *available* `mem` per the formula + above. 3. **Reservation charge.** Once dispatched, 4MB is charged against the host's in-use memory — this is the reservation, and it holds regardless @@ -41,89 +108,94 @@ against). otherwise. 4. **Reconciliation against real usage.** Once the worker starts reporting - the job's actual memory usage (rusage ticks), two cases: - - **actual < reserved (4MB):** keep charging the full 4MB reservation. - The estimate might have been conservative and the job may still grow - into it later in its lifecycle — the unused slack must not be handed - to another job on the strength of a snapshot that could be stale a - moment later. - - **actual >= reserved (4MB):** charge the real usage instead. The - estimate was wrong in the other direction; admission control must - reflect what the host is *actually* carrying, not a now-obsolete - promise, or the host can be driven into genuine oversubscription. + the job's actual memory usage (rusage ticks), for **every** job on the + host — including one that requested no `rusage[mem=...]` at all, whose + reservation is 0: + - **actual < reserved:** keep charging the full reservation (0 for an + unreserved job means: charge nothing extra beyond its actual usage, + which is exactly what the max() below produces). The estimate might + have been conservative and the job may still grow into it later — the + unused slack must not be handed to another job on the strength of a + snapshot that could be stale a moment later. + - **actual >= reserved:** charge the real usage instead. For a job that + never reserved anything, this is *always* this branch (reserved = 0, + actual >= 0 always) — its full real consumption must be charged, + because nothing else in this codebase's admission path (see above — + there is no live OS read at decision time) will ever account for it + otherwise. Skipping an unreserved job's real usage would leave the + scheduler's own `inuse` tally blind to memory a live process is + genuinely sitting on — an actual over-subscription risk, not merely a + display gap. - In short: **charged = max(reserved, actual)**. - -5. **Available memory for scheduling:** - - ``` - available = host_os_reported_available_memory - - sum over every job on that host of: max(0, reserved_j - used_j) - ``` - - Only the *unused slack of the reservation* (`reserved - used`, floored at - 0) is subtracted — never the job's actual usage on top of that. The - job's real consumption is already reflected in the OS's own - available-memory figure by the time it's read; subtracting it again - would double-count it. This is also why case 4's "actual >= reserved" - branch contributes nothing extra here: `max(0, reserved - actual)` is - already 0 once actual has caught up to or passed the reservation, so the - OS figure alone is authoritative once a job is running hot. + In short: **charged = max(reserved, actual)** — for every job, reserved + or not. ## Existing infrastructure this policy builds on -This is not a green-field design — the account-level data model already has -almost exactly the right shape for it, it's just never been wired up to -real usage data: +The account-level data model already had almost exactly the right shape +for this — it just wasn't wired up to real usage data, and didn't create +an account entry at all for a job that made no reservation: -- `xce_res_account_t` (`types.h`) already carries three separate fields per - job-resource pairing: `reserved`, `allocated` (both set to the job's - request at dispatch time, `make_res_account()`, `xcealloc.c`), and `used`. - `used` is initialized to `0.0` and — as of this writing — **nothing in the - codebase ever updates it again**. It is a fully dormant field. -- The resource-pool charge/uncharge path, `sched_update_res_accounts()` - (`resource.c`), sums `accounts[i]->used + accounts[i]->reserved` as the - amount charged into `w->resInUse`/`core->sharedInUse`. +- `xce_res_account_t` (`types.h`) carries three fields per job-resource + pairing: `reserved`, `allocated` (both set to the job's request at + dispatch time, `make_res_account()`, `xcealloc.c`), and `used`. +- `sched_update_res_accounts()` (`resource.c`) sums + `accounts[i]->used + accounts[i]->reserved` as the amount charged into + `w->resInUse`/`core->sharedInUse`. -Substitute `used = max(0, actual - reserved)` (the *overage* beyond the -reservation, not the raw actual value) into that existing sum and it falls -out exactly right, with no formula change needed at the charge site: +Setting `used = max(0, actual - reserved)` (the *overage* beyond the +reservation, not the raw actual value) makes that existing sum come out +exactly right, with no change needed to the charge formula itself: ``` used + reserved = reserved + max(0, actual - reserved) = max(reserved, actual) ``` -— precisely step 4's `charged = max(reserved, actual)` above. The field -names and the charge formula strongly suggest this reconciliation was the -original intent of splitting `used` out from `reserved`/`allocated` in the -first place; only the piece that keeps `used` current from real rusage -reports was never built. +For a job with no reservation at all (`reserved = 0`), there is no existing +account entry to update in the first place — `make_res_account()` is only +ever called for resources the job's own `resreq` specified. The +implementation below creates one on first use rather than skipping. -### The missing wiring (not yet implemented — scope for a future task) +## Implementation (`resource.c`, `schedobj.c`) -Job rusage already reaches the scheduler (`XCE_SCHED_OP_JOB_RUSAGE` -> -`sched_apply_job_rusage()`, `schedobj.c`) — but today that function only -stores the raw report on `job->rusages` and fires a picker-plugin hook -(`plugin_job_rusage_update`); it never touches `job->accounts` or -re-charges the resource pool. Implementing this policy means, at that same -integration point, for each `XCE_RESF_DYNAMIC | XCE_RESF_CONSUMABLE` -resource the report covers: +Implemented, not just designed: -1. Look up the job's existing `reserved` value for that resource from - `job->accounts`. -2. Compute `overage = max(0, actual_usage - reserved)`. -3. Update `xce_res_account_t.used = overage`. -4. Re-charge the pool: uncharge the account's old total, recharge the new - total, so `w->resInUse`/`core->sharedInUse` — and therefore - `unsatisfy_()`'s admission check (`total < inuse + uncommitted + - wanted`, `resource.c`) — reflects the update immediately, not just at - the next full re-account pass (`sched_account_jobs()`). +- **`ensure_host_account_by_resname()`** — finds a job's existing + `xce_res_account_t` for a resname, or creates a fresh `reserved = 0` + entry (resolving `residx` via `key_find()` against `core->resmgr`, same + pattern `sched_update_res_accounts()` already uses for a fresh account) + if the job never requested that resource. Returns `NULL` only if the + resource isn't registered in the cluster's config at all — nothing to + charge, not an error. +- **`sched_reconcile_host_resource_usage()`** — the core formula: sets one + `xce_res_account_t.used` to `max(0, actual - reserved)` and applies just + the resulting **delta** to the worker's `resInUse` (never a full + uncharge/recharge of the whole job account, which would needlessly + re-touch sibling resources like `slots` charged in the same account). +- **`sched_reconcile_job_resource_usage()`** — the wrapper driven from a + raw rusage report (`xce_job_rusages_t`, the worker's own wire shape, + forwarded verbatim to the scheduler by `sched_job_rusage_update_fun()`, + `controller/sched.c`). Maps the worker's real wire resname + (`"memory_rss"` — same worker-resname-vs-scheduler-resource-name split + bug-1735 already established for the controller side) to the scheduler's + `"mem"` resource. Scoped to **single-host jobs only**: the wire report + carries no host field at all (`update_job_rusages()`, `controller/job.c`, + derives the reporting host locally from the endpoint the request arrived + on and never attaches it to the forwarded struct), so there is no way to + attribute a report to one specific host of a multi-host job today. That + is a separate, undocumented-until-now wire-protocol gap — a multi-host + job's memory stays charged at its flat per-host reservation, exactly as + it always has; this feature does not make that worse, and does not fix + it either. +- Wired into **`sched_apply_job_rusage()`** (`schedobj.c`), which already + received every job rusage tick but previously only stored it for + display/plugin hooks with zero resource-accounting effect. -This has not been implemented yet. When it is, it should land as its own -spec (TDD: a regression test proving the reconciliation formula before the -wiring, following this project's testing discipline) rather than a quick -patch, since it changes live admission-control behavior for every -`rusage[mem=...]`-bearing job in the cluster. +Compatible with `sched_account_jobs()`'s existing per-round `resInUse` +reset-and-re-derive cycle (bug-1624): the reconciled `used` value is +persisted on the job's own account record, which that re-derivation reads +from directly — not a transient pool-level delta a round boundary would +wipe. ## Why estimates in either direction are expected, not exceptional @@ -132,15 +204,17 @@ prediction of peak lifetime memory, not a measurement. Both directions of error are routine, not edge cases to special-case away: - **Over-estimated** (the common conservative-user case): the job never - grows into the memory it asked for. Case 4's first branch protects the + grows into the memory it asked for. The reservation still protects the host from another job being packed into that never-realized headroom — a job that legitimately reserved memory it turns out not to need for most of its run can still burst into it later without being starved by a neighbor that was admitted on the strength of a low-water-mark reading. -- **Under-estimated**: the job genuinely needs more than it asked for. - Case 4's second branch keeps admission control honest about what the - host is really carrying once that happens, rather than continuing to - trust a promise the job has already broken. +- **Under-estimated**: the job genuinely needs more than it asked for (or + never asked for anything at all). Reconciliation keeps admission control + honest about what the host is really carrying once that happens, rather + than continuing to trust a promise the job has already broken — or, for + an unreserved job, rather than trusting no promise at all as if that + meant no consumption. Neither case is a failure mode to reject or warn about at the accounting layer — the reservation is advisory input to admission control, not a hard diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index 2cdeccf0..f9c4fb87 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -605,9 +605,7 @@ int sched_commit_job_alloc(xce_sched_core_t* core, xce_sched_job_t* job) /* * find_host_account_by_resname - the xce_res_account_t within `hacct` - * matching `resname` (e.g. "mem"), or NULL if the job never requested that - * resource (see resource-accounting.md: no rusage[] clause for a resource - * means no account entry for it at all, not a zero-valued one). + * matching `resname` (e.g. "mem"), or NULL if no such entry exists yet. */ static xce_res_account_t* find_host_account_by_resname(xce_host_account_t* hacct, const char* resname) @@ -630,6 +628,79 @@ find_host_account_by_resname(xce_host_account_t* hacct, const char* resname) return NULL; } +/* + * ensure_host_account_by_resname - find_host_account_by_resname(), or + * (resource-accounting.md correction) CREATE a fresh reserved=0 entry for + * `resname` if the job never requested it via rusage[]. + * + * A job with no rusage[mem=...] clause genuinely reserves nothing -- but + * this codebase's admission check (sched_check_host_satisfy_(), above) + * never reads a live OS "how much is actually free" figure: it compares + * the worker's self-reported STATIC total (w->resvals) against this + * scheduler's OWN running tally (w->resInUse), built entirely by summing + * every job's xce_res_account_t.used+reserved. A reservation of 0 does NOT + * mean nothing needs charging -- charged = max(reserved, actual) = actual + * when reserved is 0, same formula, and that actual usage is real memory + * no other job can also use. Skipping it (as an earlier version of this + * function did) let an unreserved job's genuine consumption stay + * permanently invisible to w->resInUse -- the scheduler would believe + * memory was free that a live, unreserved job was actually sitting on, a + * real over-subscription risk, not merely a cosmetic gap. + * + * The new entry's residx is resolved the same way + * sched_update_res_accounts() resolves a fresh account's residx above -- + * key_find() against core->resmgr, same lock discipline. Returns NULL + * (nothing to charge, not an error) if `resname` isn't even a registered + * resource in this cluster's config; the caller must treat that as a + * no-op, matching find_host_account_by_resname()'s own contract. + */ +static xce_res_account_t* +ensure_host_account_by_resname(xce_sched_core_t* core, + xce_host_account_t* hacct, const char* resname) +{ + xce_res_account_t* entry; + xce_res_account_t** grown; + xce_resource_t* r = NULL; + int rc = 0; + + entry = find_host_account_by_resname(hacct, resname); + if (entry != NULL) { + return entry; + } + + lock(core->resmgr, RTTI_LOCK_READ); + rc = key_find(core->resmgr, (void*)resname, (void**)&r); + unlock(core->resmgr); + + if ((rc < 0) || (r == NULL)) { + return NULL; + } + + entry = (xce_res_account_t*)calloc(1, sizeof(*entry)); + if (entry == NULL) { + return NULL; + } + entry->resname = strdup(resname); + entry->residx = r->idx; + entry->used = 0.0; + entry->reserved = 0.0; + entry->allocated = 0.0; + + grown = (xce_res_account_t**)realloc( + hacct->accounts, (size_t)(hacct->naccount + 1) * sizeof(*grown)); + if (grown == NULL) { + FREEUP(entry->resname); + FREEUP(entry); + return NULL; + } + + hacct->accounts = grown; + hacct->accounts[hacct->naccount] = entry; + hacct->naccount++; + + return entry; +} + /* * sched_reconcile_host_resource_usage - resource-accounting.md's * reservation-vs-actual-usage policy for one REAL (gauge-style, e.g. "mem") @@ -710,11 +781,14 @@ int sched_reconcile_host_resource_usage(xce_sched_core_t* core, * controller side, mapped explicitly here rather than unified across the * codebase (out of this fix's scope too). * - * A no-op (0, nothing charged) if the job never requested rusage[mem=...] - * at all (find_host_account_by_resname() then returns NULL, per that - * function's own doc comment) -- resource-accounting.md's "no rusage - * means reserved=0" case, which needs no reconciliation since there was - * never anything held back. + * A job that never requested rusage[mem=...] gets reserved=0, NOT skipped + * entirely: ensure_host_account_by_resname() creates the missing account + * entry on first use rather than treating "no reservation" as "nothing to + * charge" -- see that function's own doc comment for why this codebase's + * admission check specifically requires it (no live OS "available" read + * anywhere in the path; every job's real usage must be reflected in + * w->resInUse, reserved or not). The only genuine no-op is when "mem" + * isn't even a registered resource in this cluster's config at all. */ #define SCHED_RUSAGE_MEM_RESNAME "memory_rss" #define SCHED_MEM_RESOURCE_NAME "mem" @@ -741,7 +815,8 @@ int sched_reconcile_job_resource_usage(xce_sched_core_t* core, return 0; } - mem_acct = find_host_account_by_resname(hacct, SCHED_MEM_RESOURCE_NAME); + mem_acct = ensure_host_account_by_resname(core, hacct, + SCHED_MEM_RESOURCE_NAME); if (mem_acct == NULL) { return 0; } diff --git a/src/scheduler/testsuite/test_resource_accounting.c b/src/scheduler/testsuite/test_resource_accounting.c index b1057689..8e62d614 100644 --- a/src/scheduler/testsuite/test_resource_accounting.c +++ b/src/scheduler/testsuite/test_resource_accounting.c @@ -65,6 +65,7 @@ #include +#include "resobj.h" #include "scheduler.h" #include "sim.h" @@ -623,20 +624,44 @@ static void test_reconcile_job_resource_usage_skips_multihost_jobs(void) TEST_ASSERT_EQUAL_INT(0, rc); } +/* + * test_reconcile_job_resource_usage_charges_unreserved_job_full_usage - + * resource-accounting.md's correction: a job with NO rusage[mem=...] at + * all is reserved=0, but that must NOT mean "nothing to charge". This + * codebase's admission check has no live OS read at decision time (see + * the steering doc) -- w->resInUse is the ONLY place a job's real memory + * footprint is ever reflected, so an unreserved job's actual usage must + * still be charged in full (max(0, actual) == actual), or the scheduler's + * own capacity tracking silently under-counts real consumption and can + * over-admit new jobs onto memory a live, unreserved job is genuinely + * sitting on. + */ static void -test_reconcile_job_resource_usage_skips_when_no_mem_request(void) +test_reconcile_job_resource_usage_charges_unreserved_job_full_usage(void) { xce_scheduler_t sched; + xce_sched_worker_t* w; xce_job_account_t* jacct; - xce_job_rusages_t* rusages = make_mem_rusages(99.0f); + xce_job_rusages_t* rusages = make_mem_rusages(5.0f); + xce_resource_t mem_res = { 0 }; + xce_resource_t* registered; int rc; memset(&sched, 0, sizeof(sched)); setup_core_with_worker(&(sched.core), WORKER_NAME); + w = sched_get_worker(&(sched.core), WORKER_NAME); + TEST_ASSERT_NOT_NULL(w); - /* A single-host account with NO "mem" entry at all -- resource- - * accounting.md's "no rusage[mem=...] means reserved=0, nothing to - * reconcile" case. */ + /* "mem" must be a real, registered resource for ensure_host_account_ + * by_resname()'s key_find() lookup to resolve a residx -- mirrors how + * a real cluster config would already have it registered long before + * any job ever runs. */ + mem_res.name = strdup(SCHED_MEM_RESOURCE_NAME); + registered = makeResource(sched.core.resmgr, &mem_res); + TEST_ASSERT_NOT_NULL(registered); + + /* A single-host account with NO "mem" entry at all -- the job never + * requested rusage[mem=...]. */ jacct = (xce_job_account_t*)calloc(1, sizeof(*jacct)); jacct->nhosts = 1; jacct->accounts = (xce_host_account_t**)calloc(1, sizeof(*jacct->accounts)); @@ -646,6 +671,42 @@ test_reconcile_job_resource_usage_skips_when_no_mem_request(void) rc = sched_reconcile_job_resource_usage(&(sched.core), jacct, rusages); TEST_ASSERT_EQUAL_INT(0, rc); + + /* An account entry now exists (created on first use), reserved=0, + * used == the full actual usage, and the pool reflects it. */ + TEST_ASSERT_EQUAL_INT(1, jacct->accounts[0]->naccount); + TEST_ASSERT_EQUAL_STRING(SCHED_MEM_RESOURCE_NAME, + jacct->accounts[0]->accounts[0]->resname); + TEST_ASSERT_EQUAL_FLOAT(0.0, + (float)jacct->accounts[0]->accounts[0]->reserved); + TEST_ASSERT_EQUAL_FLOAT(5.0, (float)jacct->accounts[0]->accounts[0]->used); + TEST_ASSERT_EQUAL_FLOAT( + 5.0, get_worker_resinuse(w, jacct->accounts[0]->accounts[0]->residx)); +} + +static void +test_reconcile_job_resource_usage_noop_when_mem_not_a_registered_resource(void) +{ + xce_scheduler_t sched; + xce_job_account_t* jacct; + xce_job_rusages_t* rusages = make_mem_rusages(99.0f); + int rc; + + memset(&sched, 0, sizeof(sched)); + setup_core_with_worker(&(sched.core), WORKER_NAME); + /* Deliberately do NOT register "mem" into sched.core.resmgr -- a + * misconfigured cluster with no such resource defined at all. */ + + jacct = (xce_job_account_t*)calloc(1, sizeof(*jacct)); + jacct->nhosts = 1; + jacct->accounts = (xce_host_account_t**)calloc(1, sizeof(*jacct->accounts)); + jacct->accounts[0] = (xce_host_account_t*)calloc(1, sizeof(xce_host_account_t)); + jacct->accounts[0]->host = strdup(WORKER_NAME); + jacct->accounts[0]->naccount = 0; + + rc = sched_reconcile_job_resource_usage(&(sched.core), jacct, rusages); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(0, jacct->accounts[0]->naccount); } static void test_reconcile_job_resource_usage_null_args_return_einval(void) @@ -673,7 +734,8 @@ int main(void) RUN_TEST(test_reconcile_host_resource_usage_null_args_return_einval); RUN_TEST(test_reconcile_job_resource_usage_maps_memory_rss_to_mem_account); RUN_TEST(test_reconcile_job_resource_usage_skips_multihost_jobs); - RUN_TEST(test_reconcile_job_resource_usage_skips_when_no_mem_request); + RUN_TEST(test_reconcile_job_resource_usage_charges_unreserved_job_full_usage); + RUN_TEST(test_reconcile_job_resource_usage_noop_when_mem_not_a_registered_resource); RUN_TEST(test_reconcile_job_resource_usage_null_args_return_einval); return UNITY_END(); }