fix(bug-1735): job memory/CPU usage always read 0 in scheduler replay

Two independent gaps in misc.c/job.c, both silently zeroing every real
job's resource-usage data:

- update_host_runusage() matched worker rusage report resnames
  "cputime"/"mem", but the real worker's collector
  (collect_job_rusages(), xcecd_cgroups.c) sends "cpu_time_sec"/
  "memory_rss" -- the two never matched, so cpu_time/mem_usage stayed
  at their calloc()'d zero for every non-simulated job. Aligned both
  sides, including xce_test's own simulated-worker fixture.
- update_job_rusages() had a standing FIXME: xce_job_runusage_t's
  job-level max_mem/avg_mem/used_mem/cpu_time were never derived from
  the per-host host_runusage[] entries. Added aggregate_job_runusage()
  (sum/max/avg across hosts) and wired it in right after the per-host
  update.

Added test_host_runusage.c (standalone unit test, mirrors
resval_typeresolve_unit_test's isolation pattern) plus the
controller_unit_test stub aggregate_job_runusage() needs since that
target excludes the real misc.c.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA

src/controller/include/cluster.h

@@ -198,6 +198,7 @@ extern int       job_ctrl(cluster_t*, xce_jobctrl_t*);
 extern int       update_job_rusages(cluster_t*, endpoint_t*, xce_request_t*);
 extern int       update_host_runusage(xce_job_runusage_t**, char*, int,
                                       xce_rusage_t**);
+extern void      aggregate_job_runusage(xce_job_runusage_t*);
 extern int       update_pids(job_pidinfo_t**, int, int*);
 extern int       update_pgids(job_pidinfo_t**, int, int*);
 extern int       get_ep_role(endpoint_t*);

src/controller/job.c

@@ -3193,13 +3193,14 @@ int update_job_rusages(cluster_t* c, endpoint_t* ep, xce_request_t* request)
     update_host_runusage(&(j->job.run_usage), host, rusages->count,
                          rusages->usages);
 
+    /* bug-1735: derive job-level max_mem/avg_mem/used_mem/cpu_time from the
+     * per-host entries update_host_runusage() just updated -- replaces the
+     * standing "update other fields of job rusage later" FIXME. */
+    aggregate_job_runusage(j->job.run_usage);
+
     update_pids(&(j->job.pidinfo), rusages->npids, rusages->pids);
 
     update_pgids(&(j->job.pidinfo), rusages->npgids, rusages->pgids);
-
-    /* FIXME
-     * update other fields of job rusage later
-     */
     /* TEMP diagnostic (bug-1731 job-finish-notify-loss investigation).
      * DEBUG3-gated -- see bug-1731's own "re-add before next occurrence"
      * recommendation; the DEBUG3 log-retention bump (bug-1731 support,

src/controller/meson.build

@@ -355,3 +355,27 @@ test('resval typeresolve test', resval_typeresolve_unit_test,
      timeout: 60,
      env: ['ASAN_OPTIONS=detect_leaks=0'])
 
+# bug-1735: misc.c's update_host_runusage()/aggregate_job_runusage() are
+# small, dependency-light functions (calloc/realloc/strcmp only, no RTTI
+# locking, no networking) -- standalone binary linking the REAL misc.c
+# directly, mirroring resval_typeresolve_unit_test's isolation rationale
+# above. get_ep_worker()/get_ep_role() (same file, RTTI-backed) are unused
+# by these tests but still link fine via controller_unit_test_deps' existing
+# librtti_dep.
+host_runusage_unit_test_src = [
+  'testsuite/test_host_runusage.c',
+  'misc.c',
+]
+
+host_runusage_unit_test = executable('host_runusage_unit_test',
+                                     host_runusage_unit_test_src,
+                                     include_directories: [incdir],
+                                     dependencies: controller_unit_test_deps,
+                                     c_args: ['-DXCE_TESTING'],
+                                     install: false)
+
+test('host runusage test', host_runusage_unit_test,
+     suite: 'controller',
+     timeout: 60,
+     env: ['ASAN_OPTIONS=detect_leaks=0'])
+

src/controller/misc.c

@@ -126,12 +126,16 @@ int update_host_runusage(xce_job_runusage_t** runusage, char* host, int count,
     }
 
     for (i = 0; i < count; i++) {
-        if (strcmp(usages[i]->resname, "cputime") == 0) {
+        /* bug-1735: these must match the REAL worker's rusage collector
+         * (collect_job_rusages(), xcecd_cgroups.c res_names[]) -- "cputime"/
+         * "mem" never matched anything that worker actually sends, so
+         * cpu_time/mem_usage stayed at their calloc()'d zero forever. */
+        if (strcmp(usages[i]->resname, "cpu_time_sec") == 0) {
             rusages->host_runusage[j]->cpu_time = usages[i]->value;
             continue;
         }
 
-        if (strcmp(usages[i]->resname, "mem") == 0) {
+        if (strcmp(usages[i]->resname, "memory_rss") == 0) {
             rusages->host_runusage[j]->mem_usage = usages[i]->value;
             continue;
         }
@@ -140,6 +144,54 @@ int update_host_runusage(xce_job_runusage_t** runusage, char* host, int count,
     return 0;
 }
 
+/* bug-1735: derives xce_job_runusage_t's own job-level max_mem/avg_mem/
+ * used_mem/cpu_time from the per-host host_runusage[] entries
+ * update_host_runusage() populates -- replaces update_job_rusages()'s
+ * (job.c) standing "update other fields of job rusage later" FIXME.
+ * used_mem is the SUM across hosts (a multi-host job's total memory
+ * footprint); max_mem/avg_mem are the max/average across hosts; cpu_time is
+ * the SUM across hosts. peak_cpu_time/peak_cpu_duration/mem_efficiency/
+ * avg_cpu_efficiency/peak_cpu_efficiency are intentionally left untouched --
+ * bug-1735 was explicitly scope-narrowed to memory + CPU time only; those
+ * fields are bug-1736 (a design task, no formula exists yet). */
+void aggregate_job_runusage(xce_job_runusage_t* runusage)
+{
+    int    i, valid = 0;
+    double mem_sum = 0.0;
+    double mem_max = 0.0;
+    int    cpu_sum = 0;
+
+    if ((runusage == NULL) || (runusage->host_runusage == NULL)) {
+        return;
+    }
+
+    for (i = 0; i < runusage->num_hosts; i++) {
+        xce_host_runusage_t* h = runusage->host_runusage[i];
+
+        if (h == NULL) {
+            continue;
+        }
+
+        mem_sum += h->mem_usage;
+        cpu_sum += h->cpu_time;
+
+        if ((valid == 0) || (h->mem_usage > mem_max)) {
+            mem_max = h->mem_usage;
+        }
+
+        valid++;
+    }
+
+    if (valid == 0) {
+        return;
+    }
+
+    runusage->used_mem = mem_sum;
+    runusage->max_mem  = mem_max;
+    runusage->avg_mem  = mem_sum / valid;
+    runusage->cpu_time = cpu_sum;
+}
+
 int update_pids(job_pidinfo_t** pidinfo, int npids, int* pids)
 {
     job_pidinfo_t* info;

src/controller/testsuite/test.c

@@ -4284,9 +4284,12 @@ static void check_run_jobs()
             int          npids  = 1;
             int          npgids = 0;
             int          nusage = 2;
+            /* bug-1735: match the real worker's collect_job_rusages()
+             * resnames (xcecd_cgroups.c), not the old never-matching
+             * "cputime"/"mem" strings. */
             xce_rusage_t data[] = {
-                { "cputime", 100.0 },
+                { "cpu_time_sec", 100.0 },
-                { "mem",     200.0 }
+                { "memory_rss",   200.0 }
             };
             xce_rusage_t* usages[] = { &(data[0]), &(data[1]) };
 
@@ -4348,9 +4351,12 @@ static void check_run_tasks()
             int          npids  = 1;
             int          npgids = 0;
             int          nusage = 2;
+            /* bug-1735: match the real worker's collect_job_rusages()
+             * resnames (xcecd_cgroups.c), not the old never-matching
+             * "cputime"/"mem" strings. */
             xce_rusage_t data[] = {
-                { "cputime", 100.0 },
+                { "cpu_time_sec", 100.0 },
-                { "mem",     200.0 }
+                { "memory_rss",   200.0 }
             };
             xce_rusage_t* usages[] = { &(data[0]), &(data[1]) };
 

src/controller/testsuite/test_commit_barrier_reply.c

@@ -197,6 +197,7 @@ int update_pids(job_pidinfo_t **i, int n, int *p)
 { (void)i; (void)n; (void)p; return 0; }
 int update_pgids(job_pidinfo_t **i, int n, int *p)
 { (void)i; (void)n; (void)p; return 0; }
+void aggregate_job_runusage(xce_job_runusage_t *r) { (void)r; }
 int on_scheduler_join(cluster_t *c) { (void)c; return 0; }
 int add_apiserver(cluster_t *c, char *n, endpoint_t *e)
 { (void)c; (void)n; (void)e; return 0; }

src/controller/testsuite/test_host_runusage.c

@@ -0,0 +1,240 @@
+/* test_host_runusage.c -- bug-1735 regression suite
+ *
+ * bug-1735: job memory usage always reads 0 in the scheduler's simulation
+ * record file, despite the worker daemon genuinely measuring it. Two
+ * independent, confirmed root causes in misc.c/job.c:
+ *
+ *   1. update_host_runusage() (misc.c) only ever matched resnames
+ *      "cputime"/"mem" -- but the REAL worker's rusage collector
+ *      (collect_job_rusages(), xcecd_cgroups.c) sends "cpu_time_sec"/
+ *      "memory_rss" (see that file's res_names[] array). The two never
+ *      matched, so xce_host_runusage_t.cpu_time/mem_usage stayed at their
+ *      calloc()'d zero forever, for every real (non-simulated) job.
+ *
+ *   2. update_job_rusages() (job.c) had a standing FIXME ("update other
+ *      fields of job rusage later") -- xce_job_runusage_t's own job-level
+ *      max_mem/avg_mem/used_mem/cpu_time were never derived from the
+ *      per-host host_runusage[] entries update_host_runusage() populates,
+ *      so a multi-host job's aggregate usage stayed 0 even once (1) above
+ *      is fixed.
+ *
+ * This file unit-tests both fixes directly against the REAL misc.c (linked
+ * standalone, mirroring resval_typeresolve_unit_test's/sched_worker_change_
+ * unit_test's isolation rationale -- misc.c's update_host_runusage()/
+ * aggregate_job_runusage() are calloc/realloc/strcmp-only, no RTTI locking,
+ * no networking, so they link and run with zero fake/stub scaffolding).
+ * get_ep_worker()/get_ep_role() (same file) are untouched by this fix and
+ * unused by these tests, but still link fine via controller_unit_test_deps'
+ * existing librtti_dep.
+ *
+ * peak_cpu_time/peak_cpu_duration/mem_efficiency/avg_cpu_efficiency/
+ * peak_cpu_efficiency are intentionally NOT covered here -- bug-1735 was
+ * explicitly scope-narrowed to memory + CPU time only; those fields are
+ * bug-1736 (a design task, not a clean fix -- no formula exists yet).
+ */
+#include <math.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#include "cluster.h"
+
+/* ------------------------------------------------------------------ */
+/* Helpers                                                             */
+/* ------------------------------------------------------------------ */
+
+/* This project's shared unity_dep is built with double-precision support
+ * excluded (TEST_ASSERT_EQUAL_DOUBLE compiles to an unconditional "Unity
+ * Double Precision Disabled" failure) -- no other test file in this
+ * codebase uses it either. Plain epsilon comparison in ordinary C avoids
+ * Unity's own double formatting/compare path entirely. */
+#define ASSERT_DOUBLE_EQ_MSG(expected, actual, msg)                          \
+    TEST_ASSERT_TRUE_MESSAGE(fabs((expected) - (actual)) < 1e-6, msg)
+#define ASSERT_DOUBLE_EQ(expected, actual)                                   \
+    ASSERT_DOUBLE_EQ_MSG(expected, actual, #actual " != " #expected)
+
+static xce_rusage_t** make_usages(const char* name1, float val1,
+                                  const char* name2, float val2)
+{
+    xce_rusage_t** usages = (xce_rusage_t**)calloc(2, sizeof(xce_rusage_t*));
+
+    usages[0]          = (xce_rusage_t*)calloc(1, sizeof(xce_rusage_t));
+    usages[0]->resname = strdup(name1);
+    usages[0]->value   = val1;
+
+    usages[1]          = (xce_rusage_t*)calloc(1, sizeof(xce_rusage_t));
+    usages[1]->resname = strdup(name2);
+    usages[1]->value   = val2;
+
+    return usages;
+}
+
+static void free_usages(xce_rusage_t** usages)
+{
+    free(usages[0]->resname);
+    free(usages[0]);
+    free(usages[1]->resname);
+    free(usages[1]);
+    free(usages);
+}
+
+static void free_runusage(xce_job_runusage_t* r)
+{
+    int i;
+
+    if (r == NULL) {
+        return;
+    }
+
+    for (i = 0; i < r->num_hosts; i++) {
+        free(r->host_runusage[i]->host_name);
+        free(r->host_runusage[i]);
+    }
+    free(r->host_runusage);
+    free(r);
+}
+
+/* ------------------------------------------------------------------ */
+/* Tests -- update_host_runusage() resname alignment (root cause 1)   */
+/* ------------------------------------------------------------------ */
+
+static void
+test_update_host_runusage_matches_real_worker_resnames(void)
+{
+    xce_job_runusage_t* runusage = NULL;
+    xce_rusage_t**       usages
+        = make_usages("cpu_time_sec", 42.0f, "memory_rss", 128.5f);
+
+    int rc = update_host_runusage(&runusage, strdup("host-a"), 2, usages);
+
+    TEST_ASSERT_EQUAL_INT(0, rc);
+    TEST_ASSERT_NOT_NULL(runusage);
+    TEST_ASSERT_EQUAL_INT(1, runusage->num_hosts);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(
+        42, runusage->host_runusage[0]->cpu_time,
+        "cpu_time must be set from the worker's real 'cpu_time_sec' resname "
+        "(bug-1735 root cause 1) -- NOT the old, never-matching 'cputime'");
+    ASSERT_DOUBLE_EQ_MSG(
+        128.5, runusage->host_runusage[0]->mem_usage,
+        "mem_usage must be set from the worker's real 'memory_rss' resname "
+        "(bug-1735 root cause 1) -- NOT the old, never-matching 'mem'");
+
+    free_usages(usages);
+    free_runusage(runusage);
+}
+
+static void test_update_host_runusage_updates_existing_host_in_place(void)
+{
+    xce_job_runusage_t* runusage = NULL;
+    xce_rusage_t** first  = make_usages("cpu_time_sec", 10.0f, "memory_rss", 50.0f);
+    xce_rusage_t** second = make_usages("cpu_time_sec", 20.0f, "memory_rss", 75.0f);
+
+    update_host_runusage(&runusage, strdup("host-a"), 2, first);
+    update_host_runusage(&runusage, strdup("host-a"), 2, second);
+
+    TEST_ASSERT_EQUAL_INT_MESSAGE(
+        1, runusage->num_hosts,
+        "a second report for the SAME host must update in place, not "
+        "append a duplicate host entry");
+    TEST_ASSERT_EQUAL_INT(20, runusage->host_runusage[0]->cpu_time);
+    ASSERT_DOUBLE_EQ(75.0, runusage->host_runusage[0]->mem_usage);
+
+    free_usages(first);
+    free_usages(second);
+    free_runusage(runusage);
+}
+
+/* ------------------------------------------------------------------ */
+/* Tests -- aggregate_job_runusage() (root cause 2)                    */
+/* ------------------------------------------------------------------ */
+
+static xce_job_runusage_t* make_runusage_with_hosts(int n, const int* cpu,
+                                                     const double* mem)
+{
+    xce_job_runusage_t* r = (xce_job_runusage_t*)calloc(1, sizeof(*r));
+    int                  i;
+
+    r->num_hosts     = n;
+    r->host_runusage
+        = (xce_host_runusage_t**)calloc(n, sizeof(xce_host_runusage_t*));
+
+    for (i = 0; i < n; i++) {
+        r->host_runusage[i]
+            = (xce_host_runusage_t*)calloc(1, sizeof(xce_host_runusage_t));
+        r->host_runusage[i]->host_name = strdup("h");
+        r->host_runusage[i]->cpu_time  = cpu[i];
+        r->host_runusage[i]->mem_usage = mem[i];
+    }
+
+    return r;
+}
+
+static void test_aggregate_job_runusage_single_host(void)
+{
+    int    cpu[] = { 30 };
+    double mem[] = { 256.0 };
+    xce_job_runusage_t* r = make_runusage_with_hosts(1, cpu, mem);
+
+    aggregate_job_runusage(r);
+
+    ASSERT_DOUBLE_EQ(256.0, r->used_mem);
+    ASSERT_DOUBLE_EQ(256.0, r->max_mem);
+    ASSERT_DOUBLE_EQ(256.0, r->avg_mem);
+    TEST_ASSERT_EQUAL_INT(30, r->cpu_time);
+
+    free_runusage(r);
+}
+
+static void test_aggregate_job_runusage_multi_host_sum_max_avg(void)
+{
+    int    cpu[] = { 10, 20, 30 };
+    double mem[] = { 100.0, 300.0, 200.0 };
+    xce_job_runusage_t* r = make_runusage_with_hosts(3, cpu, mem);
+
+    aggregate_job_runusage(r);
+
+    ASSERT_DOUBLE_EQ_MSG(
+        600.0, r->used_mem,
+        "used_mem is the SUM of every host's mem_usage (total footprint of "
+        "a multi-host job), not any single host's value");
+    ASSERT_DOUBLE_EQ_MSG(300.0, r->max_mem, "max_mem is the MAX across hosts");
+    ASSERT_DOUBLE_EQ_MSG(200.0, r->avg_mem, "avg_mem is the AVERAGE across hosts");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(
+        60, r->cpu_time, "job-level cpu_time is the SUM across hosts");
+
+    free_runusage(r);
+}
+
+static void test_aggregate_job_runusage_null_and_empty_are_safe(void)
+{
+    xce_job_runusage_t empty = { 0 };
+
+    aggregate_job_runusage(NULL); /* must not crash */
+    aggregate_job_runusage(&empty); /* num_hosts == 0, must not crash */
+
+    ASSERT_DOUBLE_EQ(0.0, empty.used_mem);
+}
+
+/* ------------------------------------------------------------------ */
+/* Standalone Unity main (this binary is its own test executable -- see */
+/* meson.build's host_runusage_unit_test)                               */
+/* ------------------------------------------------------------------ */
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+int main(void)
+{
+    printf("\n>> bug-1735: host/job rusage resname + aggregation fix\n\n");
+
+    UNITY_BEGIN();
+
+    RUN_TEST(test_update_host_runusage_matches_real_worker_resnames);
+    RUN_TEST(test_update_host_runusage_updates_existing_host_in_place);
+    RUN_TEST(test_aggregate_job_runusage_single_host);
+    RUN_TEST(test_aggregate_job_runusage_multi_host_sum_max_avg);
+    RUN_TEST(test_aggregate_job_runusage_null_and_empty_are_safe);
+
+    return UNITY_END();
+}