From fc5599481346fe5d42ceb3a823a04a80e2dd38d5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 31 Aug 2026 04:20:58 +0000 Subject: [PATCH] fix(bug-1727): 'info worker'/'info resource' double-counted used capacity repl_info_worker_one()/repl_info_resource_one() (repl_info.c) printed value=avail+used for a CONSUMABLE resource, on the assumption that avail_v holds a live "remaining capacity" figure that shrinks as jobs run. That assumption was false: w->resvals[idx] (and core->sharedInst[idx] for shared resources) is the resource's STATIC TOTAL capacity, self-reported by the worker at join/heartbeat time and never decremented as jobs consume it -- confirmed against the actual admission-control formula this same field feeds (resource.c's unsatisfy_(): `total[i] < inuse[i] + uncomitted[i] + wanted[i]` treats it as a total, not a pre-reduced remaining figure). Printing avail+used therefore double-counted `used` on top of a value that already included it. Live-reproduced against a real 3-node cluster + worker: a fully-packed 22-slot worker showed "value=44.00 used=22.00" instead of the correct "value=22.00 used=22.00" (22 never-decremented total + 22 used = 44, matching this bug's mechanism exactly). This was previously investigated as bug-1727 ("recorded for future investigation only") -- confirmed its hypothesis (a) and closed it here rather than opening a new entry. Fixed by printing avail_v as-is (no addition) at both call sites. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA --- src/scheduler/src/repl_info.c | 76 +++++++++++++++---- .../test_repl_breakpoints_inspection.c | 38 ++++++---- 2 files changed, 86 insertions(+), 28 deletions(-) diff --git a/src/scheduler/src/repl_info.c b/src/scheduler/src/repl_info.c index ddcd888b..0ebc4ac2 100644 --- a/src/scheduler/src/repl_info.c +++ b/src/scheduler/src/repl_info.c @@ -278,19 +278,38 @@ static void repl_info_queue(xce_sched_sim_driver_t* sim, const char* arg) /* * repl_info_worker_one - AC 22's "info worker ": "Dump one worker: - * state, resources, assigned jobs." `w->resvals` holds currently-AVAILABLE - * values (the same zarray the picker/allocator accounting path reads/resets - * each session, core_events.c); `w->resInUse` holds the currently-committed - * amount at the same index. Only a resource flagged XCE_RESF_CONSUMABLE has - * a capacity/in-use split worth reporting -- its value= is available+inUse - * (a conservation invariant this codebase's own resource accounting already - * relies on) and used= is inUse; a non-consumable numeric resource (a - * static per-worker attribute nothing ever allocates against) gets a single - * value= line instead, mirroring repl_info_resource_one()'s identical - * CONSUMABLE gate for the cluster-wide case. Resource names/flags are - * resolved against a resmgr walk built the same way - * xce_sched_core_save_snapshot() builds its own `res_arr` (snapshot.c) -- - * not a new lookup mechanism. "assigned jobs" scans core->jobs for any + * state, resources, assigned jobs." `w->resvals` holds the resource's + * STATIC TOTAL capacity (as self-reported by the worker daemon at join/ + * heartbeat time -- WORKERS_PUSH/RESVALS_DELTA are the only writers, + * core_events.c -- and NEVER decremented as jobs consume it); `w->resInUse` + * holds the currently-committed amount at the same index. Only a resource + * flagged XCE_RESF_CONSUMABLE has a capacity/in-use split worth reporting -- + * its value= is the raw total (`w->resvals[idx]`, already inclusive of + * whatever is in use -- see resource.c's unsatisfy_(), whose admission + * formula `total[i] < inuse[i] + uncomitted[i] + wanted[i]` is the + * authoritative proof this field is a total, not a live remaining-capacity + * counter) and used= is inUse; a non-consumable numeric resource (a static + * per-worker attribute nothing ever allocates against) gets a single value= + * line instead, mirroring repl_info_resource_one()'s identical CONSUMABLE + * gate for the cluster-wide case. + * + * bug-1727 (2026-08-30, CONFIRMED + FIXED): this comment used to + * claim value= should be printed as available+inUse ("a conservation + * invariant this codebase's own resource accounting already relies on") -- + * that claim was FALSE for how w->resvals is actually populated (it is + * never decremented at charge time, so it already equals the total, not a + * live "available" residue), and printing avail+used double-counted `used` + * on top of a value that already included it. Live-reproduced against a + * real 3-node cluster + worker (50-element overlapping-runtime array job): + * a fully-packed 22-slot worker showed "value=44.00 used=22.00" instead of + * the correct "value=22.00 used=22.00" -- exactly 22 (never-decremented + * total) + 22 (used) = 44, matching this bug's mechanism precisely. Fixed + * by printing avail_v as-is (no addition) at both this call site and + * repl_info_resource_one()'s identical shared-resource case. + * + * Resource names/flags are resolved against a resmgr walk built the same + * way xce_sched_core_save_snapshot() builds its own `res_arr` (snapshot.c) + * -- not a new lookup mechanism. "assigned jobs" scans core->jobs for any * per-host allocation naming this worker (xce_job_alloc_t.allocs[]->host, * xce/types.h). */ @@ -375,6 +394,26 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, case Value_u_fvalue: if ((rdef != NULL) && (rdef->flags & XCE_RESF_CONSUMABLE)) { + /* bug-1727: w->resvals[idx] (avail_v here) is the resource's + * STATIC TOTAL capacity, not a live "remaining available" + * quantity that decreases as jobs consume it -- confirmed + * against the actual admission-control formula this exact + * field feeds, resource.c's unsatisfy_(): `total[i] < + * inuse[i] + uncomitted[i] + wanted[i]` treats + * get_resvals(&w->resvals, ...)'s output as `total` directly, + * comparing it against inuse (not total-inuse already + * subtracted from avail_v). Nothing else ever decrements + * w->resvals[idx] as jobs run (core_apply_worker_resvals() + * only overwrites it wholesale on a WORKERS_PUSH/RESVALS_ + * DELTA re-announce, and built-in load indices -- + * resource_sync.c's init_builtin_load_indices() -- never + * include "slots"). Printing avail+used therefore double- + * counts `used` on top of a value that is ALREADY the + * total, growing without bound as more jobs run -- + * live-reproduced and matching the user's exact reported + * numbers (value=44.00 used=22.00 on a 22-slot worker at + * full occupancy: 22 (never-decremented total) + 22 (used) + * = 44). Print avail_v as-is. */ double avail = avail_v->u.fvalue; double used = ((used_v != NULL) && (used_v->which == Value_u_fvalue)) @@ -383,7 +422,7 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, fprintf(stdout, "[sim] resource %s: value=%.2f used=%.2f\n", rname, - avail + used, used); + avail, used); } else { fprintf(stdout, "[sim] resource %s: value=%.2f\n", rname, avail_v->u.fvalue); @@ -616,8 +655,15 @@ static void repl_info_resource_one(xce_sched_sim_driver_t* sim, used = used_v->u.fvalue; } + /* bug-1727: same double-count fix as + * repl_info_worker_one() above -- core->sharedInst[idx] (v + * here) is already the resource's static total capacity, + * never decremented as jobs consume it (confirmed against + * resource.c's unsatisfy_() admission formula, which reads + * it as `total` and compares against `inuse` separately). + * Print it as-is instead of adding `used` on top. */ fprintf(stdout, "[sim] value=%.2f used=%.2f\n", - v->u.fvalue + used, used); + v->u.fvalue, used); } else { fprintf(stdout, "[sim] value=%.2f\n", v->u.fvalue); } diff --git a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c index fb162610..9c8dd252 100644 --- a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c +++ b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c @@ -759,9 +759,16 @@ static void test_info_worker_reports_resource_values(void) /* * New requirement (2026-08-20): a per-worker resource additionally flagged - * XCE_RESF_CONSUMABLE must report both value (available+inUse) and used - * (inUse); a non-consumable numeric resource keeps a single value= line - * (test_info_worker_reports_resource_values above covers that case). + * XCE_RESF_CONSUMABLE must report both value (its raw total capacity) and + * used (inUse); a non-consumable numeric resource keeps a single value= + * line (test_info_worker_reports_resource_values above covers that case). + * + * bug-1727 (2026-08-30): corrected from the original "value= + * avail+used" expectation -- w->resvals[idx] (avail_v) is ALREADY the + * total capacity (never decremented as jobs consume it; see resource.c's + * unsatisfy_() admission formula), so adding `used` on top double-counted + * it. See repl_info_worker_one()'s updated doc comment (repl_info.c) for + * the full live-reproduced evidence. */ static void test_info_worker_reports_consumable_value_and_used(void) { @@ -824,9 +831,9 @@ static void test_info_worker_reports_consumable_value_and_used(void) captured = capture_dispatch(&sim, "info worker hostA"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "resource slots: value=7.00 used=2.00"), - "a CONSUMABLE worker resource must report value=avail+used and " - "used=inUse"); + strstr(captured, "resource slots: value=5.00 used=2.00"), + "a CONSUMABLE worker resource must report value= and used=inUse (bug-1727)"); free(captured); @@ -1109,10 +1116,14 @@ static void test_info_resource_reports_string_value(void) /* * New requirement (2026-08-20): a GLOBAL/SHARED resource additionally - * flagged XCE_RESF_CONSUMABLE must report both its value (capacity, - * available+inUse) and how much is currently in use, read from - * core->sharedInUse -- the cluster-wide mirror of repl_info_worker_one()'s - * resInUse-derived value/used split. + * flagged XCE_RESF_CONSUMABLE must report both its value (raw total + * capacity) and how much is currently in use, read from core->sharedInUse + * -- the cluster-wide mirror of repl_info_worker_one()'s resInUse-derived + * value/used split. + * + * bug-1727 (2026-08-30): corrected from the original "value= + * avail+used" expectation -- same double-count fix as the per-worker case + * above. */ static void test_info_resource_reports_consumable_value_and_used(void) { @@ -1163,9 +1174,10 @@ static void test_info_resource_reports_consumable_value_and_used(void) captured = capture_dispatch(&sim, "info resource licenses"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "value=11.00 used=3.00"), - "a CONSUMABLE global resource must report value=avail+used and " - "used=inUse, sourced from core->sharedInUse"); + strstr(captured, "value=8.00 used=3.00"), + "a CONSUMABLE global resource must report value= and used=inUse, sourced from core->sharedInUse " + "(bug-1727)"); free(captured);