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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
This commit is contained in:
parent
0e78f86377
commit
fc55994813
2 changed files with 86 additions and 28 deletions
|
|
@ -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 <name>": "Dump one worker:
|
* repl_info_worker_one - AC 22's "info worker <name>": "Dump one worker:
|
||||||
* state, resources, assigned jobs." `w->resvals` holds currently-AVAILABLE
|
* state, resources, assigned jobs." `w->resvals` holds the resource's
|
||||||
* values (the same zarray the picker/allocator accounting path reads/resets
|
* STATIC TOTAL capacity (as self-reported by the worker daemon at join/
|
||||||
* each session, core_events.c); `w->resInUse` holds the currently-committed
|
* heartbeat time -- WORKERS_PUSH/RESVALS_DELTA are the only writers,
|
||||||
* amount at the same index. Only a resource flagged XCE_RESF_CONSUMABLE has
|
* core_events.c -- and NEVER decremented as jobs consume it); `w->resInUse`
|
||||||
* a capacity/in-use split worth reporting -- its value= is available+inUse
|
* holds the currently-committed amount at the same index. Only a resource
|
||||||
* (a conservation invariant this codebase's own resource accounting already
|
* flagged XCE_RESF_CONSUMABLE has a capacity/in-use split worth reporting --
|
||||||
* relies on) and used= is inUse; a non-consumable numeric resource (a
|
* its value= is the raw total (`w->resvals[idx]`, already inclusive of
|
||||||
* static per-worker attribute nothing ever allocates against) gets a single
|
* whatever is in use -- see resource.c's unsatisfy_(), whose admission
|
||||||
* value= line instead, mirroring repl_info_resource_one()'s identical
|
* formula `total[i] < inuse[i] + uncomitted[i] + wanted[i]` is the
|
||||||
* CONSUMABLE gate for the cluster-wide case. Resource names/flags are
|
* authoritative proof this field is a total, not a live remaining-capacity
|
||||||
* resolved against a resmgr walk built the same way
|
* counter) and used= is inUse; a non-consumable numeric resource (a static
|
||||||
* xce_sched_core_save_snapshot() builds its own `res_arr` (snapshot.c) --
|
* per-worker attribute nothing ever allocates against) gets a single value=
|
||||||
* not a new lookup mechanism. "assigned jobs" scans core->jobs for any
|
* 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,
|
* per-host allocation naming this worker (xce_job_alloc_t.allocs[]->host,
|
||||||
* xce/types.h).
|
* xce/types.h).
|
||||||
*/
|
*/
|
||||||
|
|
@ -375,6 +394,26 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim,
|
||||||
|
|
||||||
case Value_u_fvalue:
|
case Value_u_fvalue:
|
||||||
if ((rdef != NULL) && (rdef->flags & XCE_RESF_CONSUMABLE)) {
|
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 avail = avail_v->u.fvalue;
|
||||||
double used = ((used_v != NULL) &&
|
double used = ((used_v != NULL) &&
|
||||||
(used_v->which == Value_u_fvalue))
|
(used_v->which == Value_u_fvalue))
|
||||||
|
|
@ -383,7 +422,7 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim,
|
||||||
|
|
||||||
fprintf(stdout,
|
fprintf(stdout,
|
||||||
"[sim] resource %s: value=%.2f used=%.2f\n", rname,
|
"[sim] resource %s: value=%.2f used=%.2f\n", rname,
|
||||||
avail + used, used);
|
avail, used);
|
||||||
} else {
|
} else {
|
||||||
fprintf(stdout, "[sim] resource %s: value=%.2f\n", rname,
|
fprintf(stdout, "[sim] resource %s: value=%.2f\n", rname,
|
||||||
avail_v->u.fvalue);
|
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;
|
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",
|
fprintf(stdout, "[sim] value=%.2f used=%.2f\n",
|
||||||
v->u.fvalue + used, used);
|
v->u.fvalue, used);
|
||||||
} else {
|
} else {
|
||||||
fprintf(stdout, "[sim] value=%.2f\n", v->u.fvalue);
|
fprintf(stdout, "[sim] value=%.2f\n", v->u.fvalue);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -759,9 +759,16 @@ static void test_info_worker_reports_resource_values(void)
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* New requirement (2026-08-20): a per-worker resource additionally flagged
|
* New requirement (2026-08-20): a per-worker resource additionally flagged
|
||||||
* XCE_RESF_CONSUMABLE must report both value (available+inUse) and used
|
* XCE_RESF_CONSUMABLE must report both value (its raw total capacity) and
|
||||||
* (inUse); a non-consumable numeric resource keeps a single value= line
|
* used (inUse); a non-consumable numeric resource keeps a single value=
|
||||||
* (test_info_worker_reports_resource_values above covers that case).
|
* 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)
|
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");
|
captured = capture_dispatch(&sim, "info worker hostA");
|
||||||
|
|
||||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
strstr(captured, "resource slots: value=7.00 used=2.00"),
|
strstr(captured, "resource slots: value=5.00 used=2.00"),
|
||||||
"a CONSUMABLE worker resource must report value=avail+used and "
|
"a CONSUMABLE worker resource must report value=<raw total, "
|
||||||
"used=inUse");
|
"not avail+used> and used=inUse (bug-1727)");
|
||||||
|
|
||||||
free(captured);
|
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
|
* New requirement (2026-08-20): a GLOBAL/SHARED resource additionally
|
||||||
* flagged XCE_RESF_CONSUMABLE must report both its value (capacity,
|
* flagged XCE_RESF_CONSUMABLE must report both its value (raw total
|
||||||
* available+inUse) and how much is currently in use, read from
|
* capacity) and how much is currently in use, read from core->sharedInUse
|
||||||
* core->sharedInUse -- the cluster-wide mirror of repl_info_worker_one()'s
|
* -- the cluster-wide mirror of repl_info_worker_one()'s resInUse-derived
|
||||||
* resInUse-derived value/used split.
|
* 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)
|
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");
|
captured = capture_dispatch(&sim, "info resource licenses");
|
||||||
|
|
||||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
strstr(captured, "value=11.00 used=3.00"),
|
strstr(captured, "value=8.00 used=3.00"),
|
||||||
"a CONSUMABLE global resource must report value=avail+used and "
|
"a CONSUMABLE global resource must report value=<raw total, not "
|
||||||
"used=inUse, sourced from core->sharedInUse");
|
"avail+used> and used=inUse, sourced from core->sharedInUse "
|
||||||
|
"(bug-1727)");
|
||||||
|
|
||||||
free(captured);
|
free(captured);
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue