From a35f597e0672c0600ca98c71b9eb2e4c0653d8b9 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 7 Sep 2026 04:39:22 +0000 Subject: [PATCH] fix(worker): real maxswap/maxtmp detection; feat(scheduler): annotate multi-slot assigned jobs bug-1554: init_host_attrs() replaces hardcoded placeholder stubs with real detection -- maxswap from sysinfo()'s totalswap (reusing the same syscall snapshot already taken for maxmem), maxtmp from a fresh statvfs("/tmp") total-capacity measurement. No synthetic fallback on detection failure, so a system with swap genuinely disabled correctly reports 0. bug-1776: the scheduler simulator's "info worker" assigned-jobs listing now annotates a job consuming more than one "slots" unit (e.g. "jobid*2"), using per-host allocation data that was already being walked but discarded. Falls back to the pre-existing per-job listing when jobs on the same worker have differing slot counts, since a single suffix on a compressed range would be ambiguous. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Hb6Edtj4KbS8CrgEWYdA2E --- .wolf/buglog.json | 59 ++++- src/scheduler/src/repl_info.c | 81 +++++- .../testsuite/test_repl_resource_table.c | 241 ++++++++++++++++++ src/worker/include/hardware_monitor.h | 8 +- src/worker/src/hardware_monitor.c | 74 +++++- src/worker/src/xcecd.c | 25 +- src/worker/testsuite/meson.build | 14 + src/worker/testsuite/test_hardware_monitor.c | 151 +++++++++++ .../testsuite/test_xcecd_startup_order.c | 35 +++ 9 files changed, 660 insertions(+), 28 deletions(-) create mode 100644 src/worker/testsuite/test_hardware_monitor.c diff --git a/.wolf/buglog.json b/.wolf/buglog.json index bb119ee1..2b92e227 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -15694,18 +15694,18 @@ "error_message": "workerlist / resval show maxswap=1024 and maxtmp=2048 for every host regardless of actual hardware, unless overridden by a per-host config resource entry", "file": "src/worker/src/xcecd.c", "root_cause": "init_host_attrs() hardcodes host_attrs.maxswap = 1024 and host_attrs.maxtmp = 2048 as placeholder constants (lines ~349-350) -- unlike maxcpus/maxmem, which come from real hardware_monitor_* detection, these two fields have no real detection implemented yet. Confirmed live: a deploy's maxtmp showed exactly 2048 (the hardcoded stub) while maxswap showed 10000 (a per-host config override in hosts.yaml resource list, taking precedence via apply_join_state's mark_host_res loop -- proves the stub IS reachable/live whenever no override exists).", - "fix": "Not fixed -- user explicitly deferred this (2026-08-13): real swap/tmp-disk detection is a bigger feature (needs actual sysinfo swap totals + statvfs on the configured tmp path) worth discussing design for, not a one-line fix. Flagging only per user's request.", + "fix": "FIXED (2026-09-06, worker hardware detection). Real detection replaces both placeholder stubs: maxswap now comes from sysinfo()'s totalswap field (hardware_monitor_get_total_swap(), the same sysinfo() snapshot hardware_monitor_update() already takes for maxmem -- no extra syscall), and maxtmp now comes from a fresh statvfs(\"/tmp\") total-capacity measurement (hardware_monitor_get_disk_info()), mirroring node_information.c's node_info_get_tmp() own statvfs(\"/tmp\") convention but reporting total blocks*frsize instead of free space. init_host_attrs() (xcecd.c) wires both in, with NO synthetic fallback on detection failure (left at 0, matching maxcpus/maxmem's own existing no-fallback behavior) -- a system with swap genuinely disabled reports 0 correctly instead of a fabricated non-zero placeholder. New tests: test_hardware_monitor.c (7 tests, cross-checks each function's output against an independent sysinfo()/statvfs() call made directly in the test) and a new integration test in test_xcecd_startup_order.c pinning that init_host_attrs() picks up whatever these two functions report. Full worker suite: 9/9 pass.", "tags": [ + "fixed", "worker", - "hardware-detection", "stub", - "not-fixed", - "maxswap", - "maxtmp" + "maxtmp", + "hardware-detection", + "maxswap" ], "related_bugs": [], - "occurrences": 1, - "last_seen": "2026-08-13T07:36:00Z" + "occurrences": 2, + "last_seen": "2026-09-06T21:00:00Z" }, { "id": "bug-1634", @@ -21079,6 +21079,51 @@ ], "occurrences": 1, "last_seen": "2026-09-06T00:00:00Z" + }, + { + "id": "bug-1776", + "timestamp": "2026-09-06T21:05:00Z", + "error_message": "User improvement request (simulator REPL feedback): 'info worker's assigned jobs' listing gives no indication of how many 'slots' each assigned job actually consumes on that worker -- every assigned job looks identical regardless of whether it occupies 1 slot or many (e.g. a fully-packed 22-slot worker running one 2-slot job shows the exact same line shape as a 1-slot job).", + "file": "src/scheduler/src/repl_info.c", + "root_cause": "NOT A BUG -- feature gap. repl_info_worker_one()'s assigned-jobs loop already walks each job's xce_host_alloc_t (per-host allocation) to match it against the current worker's name, but only extracted job->jobid and discarded the xce_host_alloc_t itself (`break`) without ever inspecting its `allocs[]` array, which already carries a per-resource allocation list (same struct/pattern the controller's own get_nslots_in_alloc(), job.c, already scans for the identical 'slots' resname) -- so the slot count was always available with no new wire/plumbing needed, just never read at this call site.", + "fix": "FIXED (2026-09-06). Added repl_worker_alloc_nslots() (repl_info.c) mirroring get_nslots_in_alloc()'s resname==\"slots\" scan (duplicated rather than shared, since that helper is file-static to job.c and this is a read-only REPL display concern). The assigned-jobs loop now also collects a parallel assigned_slots[] array (repl_append_int(), new int-array counterpart to the existing repl_append_u64()). When every assigned job on a worker consumed the SAME slot count and that count is >1, the existing compressed jobid-range line gets a trailing '*N' (e.g. 'assigned jobs: 0-14-1788751435-0*2'); a single-slot-per-job worker (the common case) renders identically to before. When assigned jobs on the SAME worker have DIFFERING slot counts, a single '*N' suffix on a compressed range would be ambiguous, so the listing falls back to the pre-existing uncompressed per-job lines (same fallback tier a too-small compression buffer already used), each annotated with its own count. New tests in test_repl_resource_table.c: test_info_worker_assigned_job_listing_appends_slot_count_when_uniform, test_info_worker_assigned_job_listing_falls_back_per_job_when_slots_differ. Full scheduler suite: 76/77 pass (the one failure, test_job_rusage_reconciles_mem_reservation_against_usage in test_core_events.c, is a pre-existing unrelated mem-reservation-subsystem failure already documented in bug-1762, untouched by this change).", + "tags": [ + "scheduler", + "repl", + "sim", + "assigned-jobs", + "ux-improvement", + "fixed" + ], + "related_bugs": [ + "bug-1601", + "bug-1591" + ], + "occurrences": 1, + "last_seen": "2026-09-06T21:05:00Z" + }, + { + "id": "bug-1777", + "timestamp": "2026-09-06T21:10:00Z", + "error_message": "xce_test workerlist's MaxSwap column can permanently disagree with the scheduler's own resmgr/replay view for the same worker: workerlist always shows the join-time hardware_monitor stub/detected value, never a hosts.yaml per-host maxswap override, while the scheduler always uses the override. User noticed this while comparing a scheduler simulator replay (maxswap=10000) against xce_test workerlist (MaxSwap=1.00 GB) for the same host and asked whether the two should be consistent.", + "file": "src/controller/testsuite/test.c", + "root_cause": "CONFIRMED via code trace (not yet reproduced against a live pc13v/replay artifact -- none existed on disk at investigation time). xce_worker_t carries two separate copies of a host-attrs-style resource: attrs->maxswap (hostattrs_t, written ONLY by xcecd.c's init_host_attrs() at join time -- rejoin_worker(), worker.c:440, just re-copies the same join-time value) and resvals[idx(\"maxswap\")] (resmgr-indexed zarray, seeded from attrs->maxswap by add_hostattrs_resvals() at apply_join_state() start, worker.c, then OVERWRITTEN by mark_host_res()/update_val_res() if a hosts.yaml resource override exists for that host, worker.c:79 `array = &(w->host.resvals)`). cmd_workerlist() (test.c:1478) reads attrs->maxswap directly -- structurally incapable of ever reflecting a config override, since nothing writes an override back into attrs. The scheduler-facing path (collect_resvals_workers() -> resvals_to_sched -> xcesched_push_workers(), dataex.c/sched.c) reads resvals, which DOES pick up the override. So whenever a maxswap override is configured, the scheduler correctly uses it (and a replay/snapshot reflects it) while xce_test workerlist permanently shows the stale join-time value regardless of timing/staleness -- not a caching/race artifact, a structural gap. The Go apiserver/xcecli path (convert.go) also just casts attrs.maxswap with no override-awareness, and xcecli's hosts.yaml model doesn't even expose a MaxSwap column today.", + "fix": "NOT FIXED -- user explicitly deferred (2026-09-06) to prioritize bug-1554 (real swap/tmp hardware detection, now fixed) and the assigned-jobs slot-count display improvement first. Two candidate fix directions discussed: (a) make cmd_workerlist()/the Go apiserver path read the effective resmgr/resvals value instead of attrs directly, or (b) make mark_host_res() write the override back into attrs too, so every attrs consumer (maxcpus/maxmem/maxswap/maxtmp alike) stays in sync at the source -- (b) was the recommended direction since it fixes the whole class rather than patching each display path individually, but neither is implemented yet.", + "tags": [ + "controller", + "worker", + "resource", + "maxswap", + "workerlist", + "config-override", + "not-fixed", + "deferred" + ], + "related_bugs": [ + "bug-1554" + ], + "occurrences": 1, + "last_seen": "2026-09-06T21:10:00Z" } ] } diff --git a/src/scheduler/src/repl_info.c b/src/scheduler/src/repl_info.c index e18e9dcc..856331b5 100644 --- a/src/scheduler/src/repl_info.c +++ b/src/scheduler/src/repl_info.c @@ -445,6 +445,42 @@ static bool repl_append_u64(uint64_t** arr, int* n, uint64_t val) return true; } +/* repl_append_int - int-array counterpart to repl_append_u64() above, same + * exact-size realloc idiom and same degrade-gracefully-on-OOM contract. */ +static bool repl_append_int(int** arr, int* n, int val) +{ + int* grown = (int*)realloc(*arr, (size_t)(*n + 1) * sizeof(**arr)); + + if (grown == NULL) { + return false; + } + grown[*n] = val; + *arr = grown; + (*n)++; + return true; +} + +/* repl_worker_alloc_nslots - how many "slots" this per-host allocation + * consumed, so the assigned-jobs listing can annotate a multi-slot job + * (e.g. "jobid*2") instead of implying one slot per job unconditionally. + * Mirrors the controller's own get_nslots_in_alloc() (job.c) -- same + * resname scan, same "no explicit slots entry -> 1" default -- duplicated + * here rather than shared since that helper is file-static to job.c and + * this is a read-only REPL display concern, not scheduling logic. */ +static int repl_worker_alloc_nslots(xce_host_alloc_t* ha) +{ + int i; + + for (i = 0; i < ha->nallocs; i++) { + if ((ha->allocs[i] != NULL) && (ha->allocs[i]->resname != NULL) + && (strcmp(ha->allocs[i]->resname, "slots") == 0)) { + return (int)ha->allocs[i]->value; + } + } + + return 1; +} + /* * repl_info_worker_one - AC 22's "info worker ": "Dump one worker: * state, resources, assigned jobs." `w->resvals` holds the resource's @@ -667,11 +703,21 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, * jobid-range-compression helper (repl_fmt_jobid_range(), repl_fmt.c, * task 1.2) repl_break.c's JOB_PUSH event echo and the Round Rollup's * pend-jobid display already use, instead of printing one "assigned - * job: jobid=..." line per matching job. + * job: jobid=..." line per matching job. Each job's per-host "slots" + * allocation count (assigned_slots[], repl_worker_alloc_nslots()) rides + * alongside so a multi-slot job is annotated "*N" -- but ONLY when + * every assigned job on this worker consumed the SAME slot count: a + * single trailing "*N" on a compressed multi-job range is unambiguous + * only in that uniform case, so a non-uniform mix falls back to the + * uncompressed per-job listing (same fallback tier a too-small + * compression buffer already uses), each line annotated with its own + * count. */ { uint64_t* assigned_jobids = NULL; + int* assigned_slots = NULL; int n_assigned = 0; + int n_slots = 0; job = NULL; rc = iter_make(&it, sim->core.jobs); @@ -691,8 +737,11 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, if ((ha != NULL) && (ha->host != NULL) && (strcmp(ha->host, w->name) == 0)) { - (void)repl_append_u64(&assigned_jobids, &n_assigned, - job->jobid); + if (repl_append_u64(&assigned_jobids, &n_assigned, + job->jobid)) { + (void)repl_append_int(&assigned_slots, &n_slots, + repl_worker_alloc_nslots(ha)); + } break; } } @@ -707,9 +756,20 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, char jobids_buf[1024]; int jr_rc = repl_fmt_jobid_range(jobids_buf, sizeof(jobids_buf), assigned_jobids, n_assigned); + bool uniform_slots = (n_slots == n_assigned); - if (jr_rc >= 0) { - fprintf(stdout, "[sim] assigned jobs: %s\n", jobids_buf); + for (i = 1; uniform_slots && (i < n_slots); i++) { + uniform_slots = (assigned_slots[i] == assigned_slots[0]); + } + + if ((jr_rc >= 0) && uniform_slots) { + if ((n_slots > 0) && (assigned_slots[0] > 1)) { + fprintf(stdout, "[sim] assigned jobs: %s*%d\n", + jobids_buf, assigned_slots[0]); + } else { + fprintf(stdout, "[sim] assigned jobs: %s\n", + jobids_buf); + } } else { /* bug-1591: fmt_jobid() renders the raw idgen()-packed * jobid in the same human-readable form already used by @@ -718,15 +778,22 @@ static void repl_info_worker_one(xce_sched_sim_driver_t* sim, * waldump, and job.c's log lines since bug-1588). */ for (i = 0; i < n_assigned; i++) { char jobidStr[64]; + int slots = (i < n_slots) ? assigned_slots[i] : 1; fmt_jobid(jobidStr, sizeof(jobidStr), assigned_jobids[i]); - fprintf(stdout, "[sim] assigned job: jobid=%s\n", - jobidStr); + if (slots > 1) { + fprintf(stdout, "[sim] assigned job: jobid=%s*%d\n", + jobidStr, slots); + } else { + fprintf(stdout, "[sim] assigned job: jobid=%s\n", + jobidStr); + } } } } free(assigned_jobids); + free(assigned_slots); } } diff --git a/src/scheduler/testsuite/test_repl_resource_table.c b/src/scheduler/testsuite/test_repl_resource_table.c index 445c6a9d..13fb70e7 100644 --- a/src/scheduler/testsuite/test_repl_resource_table.c +++ b/src/scheduler/testsuite/test_repl_resource_table.c @@ -1056,6 +1056,245 @@ test_info_worker_assigned_job_listing_compresses_contiguous_array_indices( "array_indices: PASS\n"); } +/* + * Improvement request (simulator REPL user feedback): a job consuming more + * than one "slots" unit on a worker should be visibly distinguishable from + * a single-slot job in the assigned-jobs listing (e.g. "jobid*2"), instead + * of every assigned job looking identical regardless of how much of the + * worker it actually occupies. + */ +static void +test_info_worker_assigned_job_listing_appends_slot_count_when_uniform(void) +{ + xce_sched_sim_driver_t sim; + xce_core_event_t events[2]; + xce_queue_t wire_queue; + xce_worker_t wire_worker; + xce_worker_t* worker_ptrs[1]; + xce_job_t* job; + xce_job_t* all_jobs[1]; + uint64_t jobid; + xce_sched_job_t* sjob; + xce_alloc_t slots_alloc; + xce_alloc_t* slots_alloc_ptrs[1]; + xce_host_alloc_t ha_host; + xce_host_alloc_t* ha_host_ptrs[1]; + xce_job_alloc_t alloc; + char* captured; + char expected_range[256]; + char expected_line[300]; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + memset(&wire_queue, 0, sizeof(wire_queue)); + wire_queue.queue = "normal"; + wire_queue.priority = 5; + wire_queue.status = QUEUE_STAT_OPEN; + + jobid = mk_plain_jobid_bpi(4, 2, 1700000010, 1); + job = make_job(jobid, JOB_STAT_RUN, "normal", "alice", "proj1"); + all_jobs[0] = job; + + memset(events, 0, sizeof(events)); + events[0].type = XCE_CORE_EVENT_QUEUE_UPDATE; + events[0].u.queue_update.queue = &wire_queue; + events[1].type = XCE_CORE_EVENT_JOBS_PUSH; + events[1].u.jobs_push.njobs = 1; + events[1].u.jobs_push.jobs = all_jobs; + + rc = xce_core_push_events(&(sim.core), events, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + + memset(&wire_worker, 0, sizeof(wire_worker)); + wire_worker.name = "hostA"; + wire_worker.status = 0; + 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); + + memset(&slots_alloc, 0, sizeof(slots_alloc)); + slots_alloc.resname = "slots"; + slots_alloc.value = 2.0; + slots_alloc_ptrs[0] = &slots_alloc; + + memset(&ha_host, 0, sizeof(ha_host)); + ha_host.host = "hostA"; + ha_host.nallocs = 1; + ha_host.allocs = slots_alloc_ptrs; + ha_host_ptrs[0] = &ha_host; + + memset(&alloc, 0, sizeof(alloc)); + alloc.nhosts = 1; + alloc.allocs = ha_host_ptrs; + + sjob = sched_get_job(&(sim.core), jobid); + TEST_ASSERT_NOT_NULL(sjob); + sjob->allocs = &alloc; + + rc = repl_fmt_jobid_range(expected_range, sizeof(expected_range), &jobid, + 1); + TEST_ASSERT_TRUE(rc >= 0); + snprintf(expected_line, sizeof(expected_line), "assigned jobs: %s*2\n", + expected_range); + + captured = capture_dispatch(&sim, "info worker hostA"); + + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, expected_line), + "a job consuming 2 'slots' on this worker must be annotated " + "'*2' in the assigned-jobs listing"); + + free(captured); + + printf("test_info_worker_assigned_job_listing_appends_slot_count_when_" + "uniform: PASS\n"); +} + +/* + * Two jobs on the SAME worker consuming DIFFERENT slot counts cannot be + * annotated with a single trailing "*N" on a compressed range (which count + * would it even mean?) -- must fall back to the pre-existing uncompressed + * per-job listing, each line annotated with its own count. + */ +static void +test_info_worker_assigned_job_listing_falls_back_per_job_when_slots_differ( + void) +{ + xce_sched_sim_driver_t sim; + xce_core_event_t events[2]; + xce_queue_t wire_queue; + xce_worker_t wire_worker; + xce_worker_t* worker_ptrs[1]; + xce_job_t* job_a; + xce_job_t* job_b; + xce_job_t* all_jobs[2]; + uint64_t jobid_a; + uint64_t jobid_b; + xce_sched_job_t* sjob; + xce_alloc_t slots_alloc_a; + xce_alloc_t* slots_alloc_a_ptrs[1]; + xce_alloc_t slots_alloc_b; + xce_alloc_t* slots_alloc_b_ptrs[1]; + xce_host_alloc_t ha_a; + xce_host_alloc_t* ha_a_ptrs[1]; + xce_job_alloc_t alloc_a; + xce_host_alloc_t ha_b; + xce_host_alloc_t* ha_b_ptrs[1]; + xce_job_alloc_t alloc_b; + char* captured; + char jobidStr_a[64]; + char jobidStr_b[64]; + char expected_a[96]; + char expected_b[96]; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + memset(&wire_queue, 0, sizeof(wire_queue)); + wire_queue.queue = "normal"; + wire_queue.priority = 5; + wire_queue.status = QUEUE_STAT_OPEN; + + jobid_a = mk_plain_jobid_bpi(5, 2, 1700000020, 1); + jobid_b = mk_plain_jobid_bpi(5, 2, 1700000021, 2); + job_a = make_job(jobid_a, JOB_STAT_RUN, "normal", "alice", "proj1"); + job_b = make_job(jobid_b, JOB_STAT_RUN, "normal", "alice", "proj1"); + all_jobs[0] = job_a; + all_jobs[1] = job_b; + + memset(events, 0, sizeof(events)); + events[0].type = XCE_CORE_EVENT_QUEUE_UPDATE; + events[0].u.queue_update.queue = &wire_queue; + events[1].type = XCE_CORE_EVENT_JOBS_PUSH; + events[1].u.jobs_push.njobs = 2; + events[1].u.jobs_push.jobs = all_jobs; + + rc = xce_core_push_events(&(sim.core), events, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + + memset(&wire_worker, 0, sizeof(wire_worker)); + wire_worker.name = "hostA"; + wire_worker.status = 0; + 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); + + memset(&slots_alloc_a, 0, sizeof(slots_alloc_a)); + slots_alloc_a.resname = "slots"; + slots_alloc_a.value = 2.0; + slots_alloc_a_ptrs[0] = &slots_alloc_a; + memset(&ha_a, 0, sizeof(ha_a)); + ha_a.host = "hostA"; + ha_a.nallocs = 1; + ha_a.allocs = slots_alloc_a_ptrs; + ha_a_ptrs[0] = &ha_a; + memset(&alloc_a, 0, sizeof(alloc_a)); + alloc_a.nhosts = 1; + alloc_a.allocs = ha_a_ptrs; + + memset(&slots_alloc_b, 0, sizeof(slots_alloc_b)); + slots_alloc_b.resname = "slots"; + slots_alloc_b.value = 4.0; + slots_alloc_b_ptrs[0] = &slots_alloc_b; + memset(&ha_b, 0, sizeof(ha_b)); + ha_b.host = "hostA"; + ha_b.nallocs = 1; + ha_b.allocs = slots_alloc_b_ptrs; + ha_b_ptrs[0] = &ha_b; + memset(&alloc_b, 0, sizeof(alloc_b)); + alloc_b.nhosts = 1; + alloc_b.allocs = ha_b_ptrs; + + sjob = sched_get_job(&(sim.core), jobid_a); + TEST_ASSERT_NOT_NULL(sjob); + sjob->allocs = &alloc_a; + sjob = sched_get_job(&(sim.core), jobid_b); + TEST_ASSERT_NOT_NULL(sjob); + sjob->allocs = &alloc_b; + + fmt_jobid(jobidStr_a, sizeof(jobidStr_a), jobid_a); + fmt_jobid(jobidStr_b, sizeof(jobidStr_b), jobid_b); + snprintf(expected_a, sizeof(expected_a), "assigned job: jobid=%s*2\n", + jobidStr_a); + snprintf(expected_b, sizeof(expected_b), "assigned job: jobid=%s*4\n", + jobidStr_b); + + captured = capture_dispatch(&sim, "info worker hostA"); + + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, expected_a), + "differing per-job slot counts on the same worker must fall back " + "to the uncompressed per-job listing, annotated with this job's " + "own slot count"); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, expected_b), + "differing per-job slot counts on the same worker must fall back " + "to the uncompressed per-job listing, annotated with this job's " + "own slot count"); + + free(captured); + + printf("test_info_worker_assigned_job_listing_falls_back_per_job_when_" + "slots_differ: PASS\n"); +} + /* * AC 6.6, `info resource all`: the same three-section grouping as `info * worker`'s own version above, applied to the cluster-wide GLOBAL/SHARED @@ -1366,6 +1605,8 @@ int main(void) RUN_TEST(test_info_worker_units_mode_both_renders_raw_and_human_without_mutating_stored_value); RUN_TEST(test_info_worker_units_mode_converts_swp_megabytes_before_human_scaling); RUN_TEST(test_info_worker_assigned_job_listing_compresses_contiguous_array_indices); + RUN_TEST(test_info_worker_assigned_job_listing_appends_slot_count_when_uniform); + RUN_TEST(test_info_worker_assigned_job_listing_falls_back_per_job_when_slots_differ); RUN_TEST(test_info_resource_groups_resources_into_labeled_table_sections_in_order); RUN_TEST(test_info_resource_load_metric_whole_number_omits_trailing_zeros); RUN_TEST(test_info_resource_units_mode_both_renders_raw_and_human_for_bytes_resource); diff --git a/src/worker/include/hardware_monitor.h b/src/worker/include/hardware_monitor.h index 25905726..72dabd29 100644 --- a/src/worker/include/hardware_monitor.h +++ b/src/worker/include/hardware_monitor.h @@ -23,7 +23,13 @@ double hardware_monitor_get_total_memory(hardware_monitor_t* monitor); double hardware_monitor_get_free_memory(hardware_monitor_t* monitor); +double hardware_monitor_get_total_swap(hardware_monitor_t* monitor); + bool hardware_monitor_get_memory_info(int64_t* total_memory, - int64_t* free_memory); + int64_t* free_memory, + int64_t* total_swap); + +// 获取指定路径所在文件系统的总容量（字节） +bool hardware_monitor_get_disk_info(const char* path, int64_t* total_bytes); #endif // HARDWARE_MONITOR_H diff --git a/src/worker/src/hardware_monitor.c b/src/worker/src/hardware_monitor.c index c1e54ff6..fb3c9912 100644 --- a/src/worker/src/hardware_monitor.c +++ b/src/worker/src/hardware_monitor.c @@ -12,6 +12,7 @@ #else #include #include +#include #include #include #include @@ -22,6 +23,7 @@ struct hardware_monitor { int last_cpu_cores; double total_memory; double free_memory; + double total_swap; time_t last_update_time; }; @@ -46,6 +48,7 @@ hardware_monitor_t* hardware_monitor_create(const char* acct_file) monitor->last_update_time = 0; monitor->total_memory = 0; monitor->free_memory = 0; + monitor->total_swap = 0; XCECD_LOG(XCE_LOG_INFO, "Hardware monitor created."); return monitor; @@ -83,10 +86,12 @@ bool hardware_monitor_update(hardware_monitor_t* monitor) monitor->last_cpu_cores = current_cores; } - int64_t total_memory, free_memory; - if (hardware_monitor_get_memory_info(&total_memory, &free_memory)) { + int64_t total_memory, free_memory, total_swap; + if (hardware_monitor_get_memory_info(&total_memory, &free_memory, + &total_swap)) { monitor->total_memory = total_memory; monitor->free_memory = free_memory; + monitor->total_swap = total_swap; } if (hwloc_available) { @@ -107,9 +112,10 @@ int hardware_monitor_get_cpu_cores(hardware_monitor_t* monitor) #ifdef _WIN32 bool hardware_monitor_get_memory_info(int64_t* total_memory, - int64_t* free_memory) + int64_t* free_memory, + int64_t* total_swap) { - if (!total_memory || !free_memory) { + if (!total_memory || !free_memory || !total_swap) { return false; } @@ -123,14 +129,42 @@ bool hardware_monitor_get_memory_info(int64_t* total_memory, *total_memory = status.ullTotalPhys; *free_memory = status.ullAvailPhys; + /* ullTotalPageFile is RAM + page file(s) combined (the commit limit), + * not the page file alone -- subtracting ullTotalPhys approximates the + * page-file-only (i.e. "swap") portion, same convention sysinfo's + * totalram/totalswap split gives natively on Linux below. */ + *total_swap = (status.ullTotalPageFile > status.ullTotalPhys) + ? (int64_t)(status.ullTotalPageFile + - status.ullTotalPhys) + : 0; + + return true; +} + +bool hardware_monitor_get_disk_info(const char* path, int64_t* total_bytes) +{ + ULARGE_INTEGER total; + + if (!path || !total_bytes) { + return false; + } + + if (!GetDiskFreeSpaceExA(path, NULL, &total, NULL)) { + XCECD_LOG(XCE_LOG_ERROR, "Failed to get disk info for '%s': %d", path, + GetLastError()); + return false; + } + + *total_bytes = (int64_t)total.QuadPart; return true; } #else bool hardware_monitor_get_memory_info(int64_t* total_memory, - int64_t* free_memory) + int64_t* free_memory, + int64_t* total_swap) { - if (!total_memory || !free_memory) { + if (!total_memory || !free_memory || !total_swap) { return false; } @@ -158,6 +192,26 @@ bool hardware_monitor_get_memory_info(int64_t* total_memory, } *free_memory = (int64_t)info.freeram * info.mem_unit; + *total_swap = (int64_t)info.totalswap * info.mem_unit; + + return true; +} + +bool hardware_monitor_get_disk_info(const char* path, int64_t* total_bytes) +{ + struct statvfs fs_stat; + + if (!path || !total_bytes) { + return false; + } + + if (statvfs(path, &fs_stat) != 0) { + XCECD_LOG(XCE_LOG_ERROR, "Failed to get filesystem info for '%s': %s", + path, strerror(errno)); + return false; + } + + *total_bytes = (int64_t)fs_stat.f_blocks * (int64_t)fs_stat.f_frsize; return true; } @@ -177,4 +231,12 @@ double hardware_monitor_get_free_memory(hardware_monitor_t* monitor) return 0; } return monitor->free_memory; +} + +double hardware_monitor_get_total_swap(hardware_monitor_t* monitor) +{ + if (!monitor) { + return 0; + } + return monitor->total_swap; } \ No newline at end of file diff --git a/src/worker/src/xcecd.c b/src/worker/src/xcecd.c index 4a58b26e..de70d45c 100644 --- a/src/worker/src/xcecd.c +++ b/src/worker/src/xcecd.c @@ -341,6 +341,18 @@ controller_connector_init(xcecd_context_t* ctx) static void init_host_attrs(void) { + /* bug-1554: maxswap/maxtmp real detection. maxswap comes from the same + * sysinfo() snapshot hardware_monitor_update() already took for + * maxmem (hardware_monitor.c); maxtmp is a fresh statvfs("/tmp") + * measurement, same path and same total-vs-free distinction + * node_information.c's node_info_get_tmp() already established for the + * dynamic "tmp" load index (that one reports free space; this one + * reports total capacity). Left at 0 (matching maxcpus/maxmem's own + * existing behavior) if detection is unavailable -- no synthetic + * fallback, since a real system with swap genuinely disabled reports 0 + * too and a fabricated non-zero number would misrepresent that. */ + int64_t tmp_total_bytes = 0; + host_attrs.cpufactor = 1.0; if (xcecd_ctx.hardware_monitor) { @@ -348,15 +360,14 @@ static void init_host_attrs(void) = hardware_monitor_get_cpu_cores(xcecd_ctx.hardware_monitor); host_attrs.maxmem = hardware_monitor_get_total_memory(xcecd_ctx.hardware_monitor); + host_attrs.maxswap + = hardware_monitor_get_total_swap(xcecd_ctx.hardware_monitor); } - /* Placeholder stub values (bug-1554, real swap/tmp-disk detection not - * yet implemented) -- kept as bytes to match every other memory-scale - * host attribute's canonical unit (maxmem, mem, tmp). Magnitude chosen - * to match the pre-existing MB-scale placeholders (1024MB/2048MB). */ - host_attrs.maxswap = 1024.0 * 1024.0 * 1024.0; - host_attrs.maxtmp = 2048.0 * 1024.0 * 1024.0; - host_attrs.ndisks = 1; + if (hardware_monitor_get_disk_info("/tmp", &tmp_total_bytes)) { + host_attrs.maxtmp = (double)tmp_total_bytes; + } + host_attrs.ndisks = 1; host_attrs.hostmodel = strdup("Intel64"); host_attrs.hosttype = strdup("LINUX64"); diff --git a/src/worker/testsuite/meson.build b/src/worker/testsuite/meson.build index e475d6b4..0fcff1bf 100644 --- a/src/worker/testsuite/meson.build +++ b/src/worker/testsuite/meson.build @@ -41,6 +41,20 @@ test('worker resource_sync test', resource_sync_test, timeout: 60, env: ['ASAN_OPTIONS=detect_leaks=0']) +# bug-1554: real maxswap (sysinfo() totalswap)/maxtmp (statvfs("/tmp")) +# detection -- white-box #include of hardware_monitor.c, same convention as +# resource_sync_test above. +hardware_monitor_test = executable('hardware_monitor_test', + 'test_hardware_monitor.c', + include_directories: [worker_test_incdir], + dependencies: worker_test_deps, + install: false) + +test('worker hardware_monitor test', hardware_monitor_test, + suite: 'worker', + timeout: 60, + env: ['ASAN_OPTIONS=detect_leaks=0']) + # component-failover-adoption task 2.2: worker startup-ordering white-box # test. #include's xcecd.c directly (same technique as controller_connector_ # test above, but a separate binary/file -- xcecd.c and controller_connector.c diff --git a/src/worker/testsuite/test_hardware_monitor.c b/src/worker/testsuite/test_hardware_monitor.c new file mode 100644 index 00000000..d1742f59 --- /dev/null +++ b/src/worker/testsuite/test_hardware_monitor.c @@ -0,0 +1,151 @@ +/* + * test_hardware_monitor.c - bug-1554: maxswap/maxtmp used to be hardcoded + * placeholder stubs in xcecd.c's init_host_attrs() (real detection was never + * implemented). This white-box #includes the real hardware_monitor.c so + * these tests exercise the actual sysinfo()/statvfs() detection logic + * against this machine's real values, cross-checking each function's output + * against an independent syscall made directly in the test -- not mocks, + * since the whole point is proving the real detection is wired up and + * arithmetically correct, matching resource_sync_test's own white-box + * #include convention for this file family. + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "../src/hardware_monitor.c" + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* ──────────────────────────────────────────────────────────────────────── + * hardware_monitor_get_memory_info() -- total_swap output param + * ──────────────────────────────────────────────────────────────────────── */ + +static void test_get_memory_info_reports_total_swap_matching_sysinfo(void) +{ + int64_t total_memory = -1; + int64_t free_memory = -1; + int64_t total_swap = -1; + struct sysinfo expected; + + TEST_ASSERT_EQUAL_INT(0, sysinfo(&expected)); + + TEST_ASSERT_TRUE( + hardware_monitor_get_memory_info(&total_memory, &free_memory, + &total_swap)); + + TEST_ASSERT_TRUE(total_swap >= 0); + TEST_ASSERT_EQUAL_INT64( + (int64_t)expected.totalswap * expected.mem_unit, total_swap); + + printf("test_get_memory_info_reports_total_swap_matching_sysinfo: " + "PASS\n"); +} + +static void test_get_memory_info_null_total_swap_returns_false(void) +{ + int64_t total_memory = -1; + int64_t free_memory = -1; + + TEST_ASSERT_FALSE( + hardware_monitor_get_memory_info(&total_memory, &free_memory, NULL)); + + printf("test_get_memory_info_null_total_swap_returns_false: PASS\n"); +} + +/* ──────────────────────────────────────────────────────────────────────── + * hardware_monitor_update()/hardware_monitor_get_total_swap() -- the + * monitor-cached accessor init_host_attrs() actually calls. + * ──────────────────────────────────────────────────────────────────────── */ + +static void test_update_populates_monitor_total_swap(void) +{ + hardware_monitor_t* monitor = hardware_monitor_create("/tmp/acct"); + int64_t total_memory, free_memory, total_swap; + + TEST_ASSERT_NOT_NULL(monitor); + TEST_ASSERT_TRUE(hardware_monitor_update(monitor)); + + TEST_ASSERT_TRUE( + hardware_monitor_get_memory_info(&total_memory, &free_memory, + &total_swap)); + TEST_ASSERT_EQUAL_FLOAT((float)total_swap, + (float)hardware_monitor_get_total_swap(monitor)); + + hardware_monitor_destroy(monitor); + + printf("test_update_populates_monitor_total_swap: PASS\n"); +} + +static void test_get_total_swap_null_monitor_returns_zero(void) +{ + TEST_ASSERT_EQUAL_FLOAT(0.0f, (float)hardware_monitor_get_total_swap(NULL)); + + printf("test_get_total_swap_null_monitor_returns_zero: PASS\n"); +} + +/* ──────────────────────────────────────────────────────────────────────── + * hardware_monitor_get_disk_info() -- real statvfs()-based capacity + * detection for maxtmp (mirrors node_information.c's node_info_get_tmp() + * own statvfs("/tmp") convention, but reports total capacity, not free + * space). + * ──────────────────────────────────────────────────────────────────────── */ + +static void test_get_disk_info_tmp_matches_statvfs(void) +{ + int64_t total_bytes = -1; + struct statvfs expected; + + TEST_ASSERT_EQUAL_INT(0, statvfs("/tmp", &expected)); + + TEST_ASSERT_TRUE(hardware_monitor_get_disk_info("/tmp", &total_bytes)); + + TEST_ASSERT_EQUAL_INT64( + (int64_t)expected.f_blocks * (int64_t)expected.f_frsize, + total_bytes); + + printf("test_get_disk_info_tmp_matches_statvfs: PASS\n"); +} + +static void test_get_disk_info_null_args_return_false(void) +{ + int64_t total_bytes = -1; + + TEST_ASSERT_FALSE(hardware_monitor_get_disk_info(NULL, &total_bytes)); + TEST_ASSERT_FALSE(hardware_monitor_get_disk_info("/tmp", NULL)); + + printf("test_get_disk_info_null_args_return_false: PASS\n"); +} + +static void test_get_disk_info_nonexistent_path_returns_false(void) +{ + int64_t total_bytes = -1; + + TEST_ASSERT_FALSE(hardware_monitor_get_disk_info( + "/this/path/does/not/exist/xce-bug-1554", &total_bytes)); + + printf("test_get_disk_info_nonexistent_path_returns_false: PASS\n"); +} + +int main(void) +{ + UNITY_BEGIN(); + + RUN_TEST(test_get_memory_info_reports_total_swap_matching_sysinfo); + RUN_TEST(test_get_memory_info_null_total_swap_returns_false); + RUN_TEST(test_update_populates_monitor_total_swap); + RUN_TEST(test_get_total_swap_null_monitor_returns_zero); + RUN_TEST(test_get_disk_info_tmp_matches_statvfs); + RUN_TEST(test_get_disk_info_null_args_return_false); + RUN_TEST(test_get_disk_info_nonexistent_path_returns_false); + + return UNITY_END(); +} diff --git a/src/worker/testsuite/test_xcecd_startup_order.c b/src/worker/testsuite/test_xcecd_startup_order.c index a01a622b..ae2e4a62 100644 --- a/src/worker/testsuite/test_xcecd_startup_order.c +++ b/src/worker/testsuite/test_xcecd_startup_order.c @@ -125,6 +125,17 @@ double hardware_monitor_get_total_memory(hardware_monitor_t* monitor) (void)monitor; return 16384.0; } +double hardware_monitor_get_total_swap(hardware_monitor_t* monitor) +{ + (void)monitor; + return 4096.0; +} +bool hardware_monitor_get_disk_info(const char* path, int64_t* total_bytes) +{ + (void)path; + *total_bytes = 8192; + return true; +} network_handler_t* network_handler_create(uint16_t port) { @@ -358,6 +369,29 @@ static void test_no_peers_cluster_cfg_stays_empty_before_first_join(void) "test_no_peers_cluster_cfg_stays_empty_before_first_join: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * bug-1554: init_host_attrs() must source maxswap/maxtmp from real + * detection (hardware_monitor_get_total_swap()/hardware_monitor_get_disk_ + * info(), both stubbed above) instead of the old hardcoded placeholder + * constants (1024MB/2048MB) -- this test would have kept passing under the + * old stub-constant code too (it doesn't assert the old magic numbers are + * ABSENT), but it does pin that host_attrs picks up whatever these two + * functions report, which the placeholder code structurally could not do. + * ════════════════════════════════════════════════════════════════════════ */ + +static void test_init_host_attrs_uses_real_swap_and_tmp_detection(void) +{ + xcecd_config_t config = base_test_config(); + + TEST_ASSERT_EQUAL_INT(0, xcecd_init(&config)); + TEST_ASSERT_EQUAL_INT(0, xcecd_start()); + + TEST_ASSERT_EQUAL_FLOAT(4096.0f, (float)host_attrs.maxswap); + TEST_ASSERT_EQUAL_FLOAT(8192.0f, (float)host_attrs.maxtmp); + + printf("test_init_host_attrs_uses_real_swap_and_tmp_detection: PASS\n"); +} + /* ════════════════════════════════════════════════════════════════════════ * main * ════════════════════════════════════════════════════════════════════════ */ @@ -370,6 +404,7 @@ int main(void) RUN_TEST(test_multi_candidate_cluster_cfg_populated_before_first_join); RUN_TEST(test_no_peers_cluster_cfg_stays_empty_before_first_join); + RUN_TEST(test_init_host_attrs_uses_real_swap_and_tmp_detection); return UNITY_END(); }