fix(scheduler,controller): worker resource values always display as 0.00 (bug-1568)
xce_scheduler -i -s <replay>: "info worker <name>" showed total=0.00 available=0.00 for every resource, even though the live cluster (xce_test resval/workerlist) reported real non-zero values for the same worker. Two independent root causes, both live-traced end-to-end against a real deployed cluster (value-level TRACE at every hop: controller push, scheduler apply, snapshot save, snapshot load/decode): 1. src/scheduler/src/core_events.c: XCE_CORE_EVENT_WORKERS_PUSH called sched_add_worker() -- a plain key_add(), add-only -- unconditionally on every push. A worker connecting triggers multiple WORKERS_PUSH events in quick succession (an early push before the controller finishes writing real capacity values, carrying stale/zero data, followed moments later by a push carrying the real values). The FIRST push always won the race; every push after it for the same worker name silently failed -EEXIST and its resource values were discarded -- not just at join, for the rest of the process's lifetime (unlike jobs, there was no "update an existing worker" event path at all). Fixed via a new core_upsert_worker() (mirrors the existing core_upsert_queue() add-or-update precedent): looks up the worker by name first, updates status+resvals in place if found, else builds+adds a new one as before. 2. src/scheduler/src/repl_info.c: repl_info_worker_one()/ repl_info_resource_one() checked `xce_val_t.which == XCE_VAL_NUMERIC` -- but XCE_VAL_NUMERIC (xce/types.h, value 1) is a resource-DEFINITION value-type constant (xce_resource_t.valtype's own enum), a completely different enum from xce_val_t.which's real domain, Value_u_which (controller.capnp.h, Value_u_fvalue=5). Every producer in this codebase sets xce_val_t.which to Value_u_fvalue, never XCE_VAL_NUMERIC, so the comparison was always false -- every genuinely-numeric resval displayed as 0.00 unconditionally, independent of fix (1). Corrected all 4 occurrences to Value_u_fvalue. Both fixed via TDD (RED test before each fix, GREEN after): test_core_events.c's test_workers_push_twice_updates_existing_ worker_in_place (two pushes, same name, different values -- both must apply) and test_repl_breakpoints_inspection.c's test_info_worker_reports_resource_values (a real numeric resval must display, not fall back to 0.00). Live-validated by reproducing the user's exact original scenario end-to-end (deploy a real cluster, record, `xce_scheduler -i -s <replay>`, `info worker <name>`) -- now shows slots=22.00, maxmem=33182089216.00 (30.9GB), mem=17151664128.00 (16GB), matching the user's own live xce_test resval/workerlist numbers exactly. Full scheduler suite green (41/41, excluding pre-existing unrelated bug-1560); controller suite green (1102/1104, the same pre-existing bug-1561 pair, confirmed unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
9f539583d3
commit
aabddf6025
SSH key fingerprint:
SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
9 changed files with 446 additions and 38 deletions
|
|
@ -515,6 +515,102 @@ static void test_non_picker_events_mutate_state_but_never_call_hooks(void)
|
|||
TEST_ASSERT_NOT_NULL(sched_get_worker(&core, "hostA"));
|
||||
}
|
||||
|
||||
/*
|
||||
* bug-1568 regression: a worker connecting triggers multiple WORKERS_PUSH
|
||||
* events in quick succession (live-confirmed against a real deploy -- an
|
||||
* early push before the controller finishes writing a worker's real
|
||||
* capacity values, carrying stale/zero data, followed shortly after by a
|
||||
* push carrying the real values). Before this fix, WORKERS_PUSH always
|
||||
* built a brand-new xce_sched_worker_t and called sched_add_worker() (a
|
||||
* plain key_add(), add-only) unconditionally -- so the SECOND push for an
|
||||
* already-registered worker name silently failed -EEXIST and its resource
|
||||
* values were discarded, leaving core->workers[name] stuck at whatever the
|
||||
* FIRST push happened to carry, forever. Two WORKERS_PUSH events for the
|
||||
* SAME worker name, with DIFFERENT resource values, must both be applied
|
||||
* -- the worker's resvals end up at the SECOND push's values, not the
|
||||
* first, and no duplicate/orphaned worker object is created.
|
||||
*/
|
||||
static void test_workers_push_twice_updates_existing_worker_in_place(void)
|
||||
{
|
||||
xce_sched_core_t core;
|
||||
xce_core_event_t event;
|
||||
xce_resource_t wire_resource;
|
||||
xce_worker_t wire_worker;
|
||||
xce_worker_t* worker_ptrs[1];
|
||||
xce_val_t val1;
|
||||
xce_name_val_t nv1;
|
||||
xce_name_val_t* nv1_ptrs[1];
|
||||
xce_val_t val2;
|
||||
xce_name_val_t nv2;
|
||||
xce_name_val_t* nv2_ptrs[1];
|
||||
xce_sched_worker_t* w;
|
||||
xce_resource_t* r = NULL;
|
||||
xce_val_t* applied;
|
||||
int rc;
|
||||
|
||||
fixture_init(&core, &g_full_plugin);
|
||||
|
||||
memset(&wire_resource, 0, sizeof(wire_resource));
|
||||
wire_resource.name = "mem";
|
||||
TEST_ASSERT_NOT_NULL(makeResource(core.resmgr, &wire_resource));
|
||||
|
||||
TEST_ASSERT_TRUE(key_find(core.resmgr, (void*)"mem", (void**)&r) >= 0);
|
||||
TEST_ASSERT_NOT_NULL(r);
|
||||
|
||||
/* First push: worker "hostA" joins with mem=100.0. */
|
||||
memset(&val1, 0, sizeof(val1));
|
||||
val1.which = Value_u_fvalue;
|
||||
val1.u.fvalue = 100.0;
|
||||
memset(&nv1, 0, sizeof(nv1));
|
||||
nv1.name = "mem";
|
||||
nv1.val = &val1;
|
||||
nv1_ptrs[0] = &nv1;
|
||||
|
||||
memset(&wire_worker, 0, sizeof(wire_worker));
|
||||
wire_worker.name = "hostA";
|
||||
wire_worker.status = 0;
|
||||
wire_worker.nresvals_to_sched = 1;
|
||||
wire_worker.resvals_to_sched = nv1_ptrs;
|
||||
worker_ptrs[0] = &wire_worker;
|
||||
|
||||
memset(&event, 0, sizeof(event));
|
||||
event.type = XCE_CORE_EVENT_WORKERS_PUSH;
|
||||
event.u.workers_push.nworkers = 1;
|
||||
event.u.workers_push.workers = worker_ptrs;
|
||||
|
||||
rc = xce_core_push_events(&core, &event, 1);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
w = sched_get_worker(&core, "hostA");
|
||||
TEST_ASSERT_NOT_NULL(w);
|
||||
applied = (xce_val_t*)zarray_get(&(w->resvals), r->idx);
|
||||
TEST_ASSERT_NOT_NULL(applied);
|
||||
TEST_ASSERT_EQUAL_FLOAT(100.0f, (float)applied->u.fvalue);
|
||||
|
||||
/* Second push: SAME worker name, mem now really 200.0 (the "arrived
|
||||
* late with the real value" push bug-1568 traced live). */
|
||||
memset(&val2, 0, sizeof(val2));
|
||||
val2.which = Value_u_fvalue;
|
||||
val2.u.fvalue = 200.0;
|
||||
memset(&nv2, 0, sizeof(nv2));
|
||||
nv2.name = "mem";
|
||||
nv2.val = &val2;
|
||||
nv2_ptrs[0] = &nv2;
|
||||
|
||||
wire_worker.resvals_to_sched = nv2_ptrs;
|
||||
|
||||
rc = xce_core_push_events(&core, &event, 1);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
/* Still exactly one "hostA" worker -- updated in place, not a second,
|
||||
* silently-discarded object leaving the first one stale. */
|
||||
w = sched_get_worker(&core, "hostA");
|
||||
TEST_ASSERT_NOT_NULL(w);
|
||||
applied = (xce_val_t*)zarray_get(&(w->resvals), r->idx);
|
||||
TEST_ASSERT_NOT_NULL(applied);
|
||||
TEST_ASSERT_EQUAL_FLOAT(200.0f, (float)applied->u.fvalue);
|
||||
}
|
||||
|
||||
/* JOB_RUSAGE's on_job_rusage hook is OPTIONAL (Req 5 AC 4) -- a picker
|
||||
* without it must still apply the state mutation and must not error or
|
||||
* crash. */
|
||||
|
|
@ -704,6 +800,7 @@ int main(void)
|
|||
RUN_TEST(test_push_events_fires_each_hook_once_per_event_with_picker_idle);
|
||||
RUN_TEST(test_jobs_push_fires_on_job_push_once_per_job);
|
||||
RUN_TEST(test_non_picker_events_mutate_state_but_never_call_hooks);
|
||||
RUN_TEST(test_workers_push_twice_updates_existing_worker_in_place);
|
||||
RUN_TEST(test_job_rusage_applies_state_when_hook_is_null);
|
||||
RUN_TEST(test_config_push_decomposes_queues_resources_and_resvals);
|
||||
RUN_TEST(test_push_events_rejects_with_ebusy_when_session_active_no_mutation);
|
||||
|
|
|
|||
|
|
@ -657,6 +657,85 @@ static void test_info_queue_reports_status_and_queued_jobs(void)
|
|||
printf("test_info_queue_reports_status_and_queued_jobs: PASS\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* bug-1568 (display-layer piece): repl_info_worker_one()/
|
||||
* repl_info_resource_one() (repl_info.c) checked
|
||||
* `avail_v->which == XCE_VAL_NUMERIC` to decide whether a worker's
|
||||
* xce_val_t resval held a real number -- but XCE_VAL_NUMERIC (xce/types.h,
|
||||
* value 1) is a RESOURCE-DEFINITION value-type constant (xce_resource_t.
|
||||
* valtype), a completely different enum from xce_val_t.which's own real
|
||||
* domain, Value_u_which (controller.capnp.h) -- every producer in this
|
||||
* codebase (update_val_res/core_apply_worker_resvals/snapshot_load.c/etc.)
|
||||
* sets xce_val_t.which to Value_u_fvalue (value 5), never XCE_VAL_NUMERIC.
|
||||
* The comparison was therefore ALWAYS false for a genuinely-numeric
|
||||
* resval, so `info worker`/`info resource` printed total=0.00/
|
||||
* available=0.00 for every worker resource, unconditionally -- live-
|
||||
* confirmed against a real deployed cluster: the value survived the full
|
||||
* controller-push -> scheduler-apply -> snapshot-save -> snapshot-load
|
||||
* round trip completely intact (traced at every hop), yet still displayed
|
||||
* as 0.00, isolating the bug to this display-layer type-tag mismatch
|
||||
* alone.
|
||||
*/
|
||||
static void test_info_worker_reports_resource_values(void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
xce_core_event_t events[2];
|
||||
xce_resource_t wire_resource;
|
||||
xce_worker_t wire_worker;
|
||||
xce_worker_t* worker_ptrs[1];
|
||||
xce_val_t val;
|
||||
xce_name_val_t nv;
|
||||
xce_name_val_t* nv_ptrs[1];
|
||||
char* captured;
|
||||
int rc;
|
||||
|
||||
memset(&sim, 0, sizeof(sim));
|
||||
rc = xce_sched_core_init(&(sim.core));
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
memset(&wire_resource, 0, sizeof(wire_resource));
|
||||
wire_resource.name = "mem";
|
||||
TEST_ASSERT_NOT_NULL(makeResource(sim.core.resmgr, &wire_resource));
|
||||
|
||||
memset(&val, 0, sizeof(val));
|
||||
val.which = Value_u_fvalue;
|
||||
val.u.fvalue = 42.5;
|
||||
memset(&nv, 0, sizeof(nv));
|
||||
nv.name = "mem";
|
||||
nv.val = &val;
|
||||
nv_ptrs[0] = &nv;
|
||||
|
||||
memset(&wire_worker, 0, sizeof(wire_worker));
|
||||
wire_worker.name = "hostA";
|
||||
wire_worker.status = 0;
|
||||
wire_worker.nresvals_to_sched = 1;
|
||||
wire_worker.resvals_to_sched = nv_ptrs;
|
||||
worker_ptrs[0] = &wire_worker;
|
||||
|
||||
memset(events, 0, sizeof(events));
|
||||
events[0].type = XCE_CORE_EVENT_WORKERS_PUSH;
|
||||
events[0].u.workers_push.nworkers = 1;
|
||||
events[0].u.workers_push.workers = worker_ptrs;
|
||||
|
||||
rc = xce_core_push_events(&(sim.core), events, 1);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
captured = capture_dispatch(&sim, "info worker hostA");
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "resource mem: total=42.50 available=42.50"),
|
||||
"a worker's real numeric resource value must be displayed, not "
|
||||
"the XCE_VAL_NUMERIC/Value_u_fvalue type-tag mismatch's 0.00 "
|
||||
"(bug-1568)");
|
||||
TEST_ASSERT_NULL_MESSAGE(strstr(captured, "total=0.00 available=0.00"),
|
||||
"must not fall back to the zero default for a "
|
||||
"genuinely-set numeric value");
|
||||
|
||||
free(captured);
|
||||
|
||||
printf("test_info_worker_reports_resource_values: PASS\n");
|
||||
}
|
||||
|
||||
static void test_info_summary_reports_counts(void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
|
|
@ -1055,6 +1134,7 @@ int main(void)
|
|||
RUN_TEST(test_info_job_reports_state_queue_priority);
|
||||
RUN_TEST(test_info_job_unknown_reports_not_found);
|
||||
RUN_TEST(test_info_queue_reports_status_and_queued_jobs);
|
||||
RUN_TEST(test_info_worker_reports_resource_values);
|
||||
RUN_TEST(test_info_summary_reports_counts);
|
||||
RUN_TEST(test_info_session_reports_position_after_synthetic_events);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue