fix(controller,xceadmin): fix zero-slot scheduling block, misleading resval logs, byte-unit display

Three bugs found while diagnosing a live controller log, all in the
worker resource-value path:

1. Every host relying on the documented hosts.yaml auto-detect default
   (no max_slot, meaning "use CPU count") got 0 scheduling slots,
   permanently blocking the scheduler from placing jobs on it. mxj is
   UInt32 on the wire and can't carry the schema's -1 sentinel; mapping.py
   worked around this by leaving it unset, which defaults to 0 -- and 0 is
   never < 0, so apply_join_state()'s auto-detect fallback never fired.
   Fixed by encoding the sentinel as 0xFFFFFFFF, which the controller's
   `int mxj` field reinterprets back to -1 via its existing implicit
   uint32_t->int conversion. No C-side or wire schema change needed for
   this part; the fallback itself was already correct but had zero test
   coverage for a negative mxj, now added.

2. update_resvals()/update_resval_list() logged the failure reason via
   PERROR, which reads process-global errno -- but update_resval()'s
   failure paths return raw negative codes directly and never touch
   errno, so the controller log filled with misleading "update_resval:
   Success" / "update_resval: Resource temporarily unavailable" lines
   instead of the real reason. Fixed to decode the actual rc via
   xce_strerror_r and name the failing resource.

3. maxmem is canonically stored/wired as raw bytes but had no unit
   metadata, so resval/workerlist printed it as a bare number that read
   as garbage (e.g. workerlist's fixed-width columns visually smeared a
   multi-GB byte count across the MaxCPU/MaxMem headers). Added an
   optional `unit` field to the Resource/ResourceVal capnp structs (and
   xce_resource_t/xce_resval_t/resources.yaml) so resval and workerlist
   can render a human-readable value (e.g. "30.90 GB") while the stored
   value stays raw bytes -- display-only, every other resource unaffected.

Regenerated the xceadmin golden fixture (fixtures/full/expected.bin)
twice: once for the mxj sentinel encoding change, once for the new
Resource.unit field growing every resource's wire size by one pointer
slot. Both diffs verified byte-for-byte isolated to exactly the expected
change before regenerating.

Verified: 1095/1095 controller_unit_test (stable across repeated runs),
229/229 xceadmin pytest, clean rebuild of lib/controller/xceadmin/xce_test.
Two related findings logged to .wolf/buglog.json but deliberately not
fixed here (out of scope per discussion): a pre-existing flaky segfault
in the combined controller_unit_test binary, and maxswap/maxtmp being
hardcoded stub values on the worker side.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-13 08:12:09 +00:00
parent 9e15161228
commit e3b3ffaa97
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
12 changed files with 354 additions and 20 deletions

View file

@ -58,7 +58,8 @@ resources:
- name: maxmem - name: maxmem
type: numeric type: numeric
description: "max memory in MB (built-in host attribute)" unit: bytes
description: "max memory in bytes (built-in host attribute; CLI display converts to a human-readable unit)"
- name: maxswap - name: maxswap
type: numeric type: numeric

View file

@ -13,6 +13,7 @@
"consumable": { "type": "boolean" }, "consumable": { "type": "boolean" },
"release": { "type": "boolean" }, "release": { "type": "boolean" },
"description": { "type": "string" }, "description": { "type": "string" },
"unit": { "type": "string", "enum": ["bytes"], "description": "Display-unit hint for CLI/UI formatting; the stored value itself is always in this unit, never converted at rest. Omit for a plain unitless number (the default for every existing resource)." },
"resourcemap": { "resourcemap": {
"type": "array", "type": "array",
"items": { "items": {

View file

@ -62,9 +62,20 @@ static int do_reply_resvals(cluster_t* c, endpoint_t* ep,
rc = get_resval(c, condition->condition.reskey->resname, rc = get_resval(c, condition->condition.reskey->resname,
condition->condition.reskey->hostname, &val); condition->condition.reskey->hostname, &val);
if (rc >= 0) { if (rc >= 0) {
xce_resource_t* r = NULL;
resval.val = &val; resval.val = &val;
resval.name = condition->condition.reskey->resname; resval.name = condition->condition.reskey->resname;
resval.host = condition->condition.reskey->hostname; resval.host = condition->condition.reskey->hostname;
resval.unit = NULL;
lock(c->resmgr, RTTI_LOCK_READ);
key_find(c->resmgr, (void*)resval.name, (void**)&r);
unlock(c->resmgr);
if (r != NULL) {
resval.unit = r->unit;
}
resvaldata.n_total = 1; resvaldata.n_total = 1;
resvaldata.vals = vals; resvaldata.vals = vals;
} }

View file

@ -1050,6 +1050,9 @@ static char* worker_status_str(uint32_t status)
return "UNKOWN"; return "UNKOWN";
} }
/* forward decl: defined near cmd_resval, below, which also uses it */
static void format_bytes_human(double bytes, char* buf, size_t bufsize);
int cmd_workerlist(int argc, char* argv[]) int cmd_workerlist(int argc, char* argv[])
{ {
int rc, i; int rc, i;
@ -1068,11 +1071,19 @@ int cmd_workerlist(int argc, char* argv[])
"MaxSwap", "MaxTmp", "NDisks"); "MaxSwap", "MaxTmp", "NDisks");
for (i = 0; i < rc; i++) { for (i = 0; i < rc; i++) {
printf("%-20s%-8s%-25s%-25s%12f%12d%12f%12f%12f%12d\n", /* MaxMem is stored/wired as raw bytes (xce_resource_t.unit="bytes"
* for the maxmem resource definition) -- humanize for display only,
* every other column stays as-is. */
char maxmem_buf[64];
format_bytes_human(workers[i]->attrs->maxmem, maxmem_buf,
sizeof(maxmem_buf));
printf("%-20s%-8s%-25s%-25s%12f%12d%12s%12f%12f%12d\n",
workers[i]->name, worker_status_str(workers[i]->status), workers[i]->name, worker_status_str(workers[i]->status),
workers[i]->attrs->hosttype, workers[i]->attrs->hostmodel, workers[i]->attrs->hosttype, workers[i]->attrs->hostmodel,
workers[i]->attrs->cpufactor, workers[i]->attrs->maxcpus, workers[i]->attrs->cpufactor, workers[i]->attrs->maxcpus,
workers[i]->attrs->maxmem, workers[i]->attrs->maxswap, maxmem_buf, workers[i]->attrs->maxswap,
workers[i]->attrs->maxtmp, workers[i]->attrs->ndisks); workers[i]->attrs->maxtmp, workers[i]->attrs->ndisks);
} }
@ -2024,6 +2035,28 @@ cleanup:
return 0; return 0;
} }
/*
* Render a raw byte count as a human-readable string (e.g. "30.90 GB").
* Only used for display -- the stored/wire value stays raw bytes always,
* this never feeds back into anything that's compared or persisted. A
* pure function so it's directly unit-testable without a live client
* connection.
*/
static void format_bytes_human(double bytes, char* buf, size_t bufsize)
{
static const char* const UNITS[] = { "B", "KB", "MB", "GB", "TB", "PB" };
const int nunits = (int)(sizeof(UNITS) / sizeof(UNITS[0]));
double val = bytes;
int i = 0;
while (val >= 1024.0 && i < nunits - 1) {
val /= 1024.0;
i++;
}
snprintf(buf, bufsize, "%.2f %s", val, UNITS[i]);
}
int cmd_resval(int argc, char* argv[]) int cmd_resval(int argc, char* argv[])
{ {
char* host = NULL; char* host = NULL;
@ -2065,7 +2098,15 @@ int cmd_resval(int argc, char* argv[])
printf("%s\n", val->val->u.svalue); printf("%s\n", val->val->u.svalue);
break; break;
case Value_u_fvalue: case Value_u_fvalue:
printf("%f\n", val->val->u.fvalue); if ((val->unit != NULL) && (strcmp(val->unit, "bytes") == 0)) {
char hbuf[64];
format_bytes_human(val->val->u.fvalue, hbuf,
sizeof(hbuf));
printf("%f (%s)\n", val->val->u.fvalue, hbuf);
} else {
printf("%f\n", val->val->u.fvalue);
}
break; break;
case Value_u_bvalue: case Value_u_bvalue:
printf("%s\n", (val->val->u.bvalue == 0) ? "false" : "true"); printf("%s\n", (val->val->u.bvalue == 0) ? "false" : "true");

View file

@ -17,6 +17,7 @@
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <unistd.h>
#include <unity/unity.h> #include <unity/unity.h>
@ -152,6 +153,132 @@ static float get_worker_res_fval(worker_t* w, const char* name)
return (v != NULL) ? v->u.fvalue : -1.0f; return (v != NULL) ? v->u.fvalue : -1.0f;
} }
/*
* Capture whatever update_resvals()/update_resval_list() print during a
* failing update_resval() call, as raw runtime output rather than a
* source-text scan. This test binary never calls mylog_init()/tlog_init()
* (only the real daemon main()s do -- see test_apply_correctness_
* verification.c's own header comment, and the identical established
* precedent in scheduler/testsuite/test_scheduler_connect.c's
* capture_stdout_during_sched_init()), so tlog's internal root log handle
* stays NULL and every ERROR()/WARN() call falls through tlog's early-
* print path, which writes the fully formatted line directly to
* STDOUT_FILENO via a raw write() -- genuine runtime log capture, not a
* guess about which stream PERROR/ERROR happens to use.
*/
static char* capture_stdout_during(void (*fn)(void* ctx), void* ctx)
{
char tmp[] = "/tmp/xce_worker_join_logcap_XXXXXX";
int fd = mkstemp(tmp);
TEST_ASSERT_TRUE_MESSAGE(fd >= 0,
"fixture: mkstemp for stdout capture must succeed");
fflush(stdout);
int saved_stdout = dup(STDOUT_FILENO);
TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0,
"fixture: dup(STDOUT_FILENO) must succeed");
TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0,
"fixture: dup2 redirect of STDOUT_FILENO must "
"succeed");
fn(ctx);
fflush(stdout);
dup2(saved_stdout, STDOUT_FILENO);
close(saved_stdout);
off_t len = lseek(fd, 0, SEEK_CUR);
TEST_ASSERT_TRUE_MESSAGE(len >= 0,
"fixture: lseek on capture file must succeed");
char* buf = (char*)calloc(1, (size_t)len + 1);
TEST_ASSERT_NOT_NULL(buf);
if (len > 0) {
TEST_ASSERT_TRUE_MESSAGE(lseek(fd, 0, SEEK_SET) == 0,
"fixture: rewind of capture file must "
"succeed");
ssize_t nread = read(fd, buf, (size_t)len);
TEST_ASSERT_TRUE_MESSAGE(nread == len,
"fixture: reading back captured buffer "
"must succeed");
}
close(fd);
unlink(tmp);
return buf;
}
typedef struct {
cluster_t* c;
worker_t* w;
xce_resval_t* val;
} update_resvals_call_ctx_t;
static void call_update_resvals_1(void* ctx_)
{
update_resvals_call_ctx_t* ctx = (update_resvals_call_ctx_t*)ctx_;
update_resvals(ctx->c, ctx->w, 1, ctx->val);
}
/*
* update_resval() returns its failure reason as a raw negative code
* directly (key_find/zarray_put convention -- never via errno), so
* update_resvals()/update_resval_list() logging it via PERROR (which reads
* the process-global errno) reported whatever errno happened to be left
* over from an unrelated earlier call: "Success" if nothing had touched
* it, or a stray EAGAIN/etc from some other syscall in the same event-loop
* iteration -- never the real reason a resource update failed. This test
* reproduces that exact symptom (ambient errno=EAGAIN, unrelated to the
* genuine failure) and proves the fix decodes the actual rc instead.
*/
static void test_update_resvals_logs_actual_rc_not_stale_errno(void)
{
setup_test_cluster();
hostattrs_t attrs;
memset(&attrs, 0, sizeof(attrs));
attrs.hosttype = makeString("LINUX");
attrs.hostmodel = makeString("x86");
worker_t* w = make_test_worker("host-resval-log", &attrs);
TEST_ASSERT_NOT_NULL(w);
xce_val_t fval;
memset(&fval, 0, sizeof(fval));
fval.which = Value_u_fvalue;
fval.u.fvalue = 1.0f;
xce_resval_t val;
val.name = "does-not-exist-in-resmgr";
val.val = &fval;
/* Simulate the exact reported symptom: ambient errno unrelated to the
* real failure, left over from some other unrelated call. */
errno = EAGAIN;
update_resvals_call_ctx_t ctx = { .c = &g_tc.c, .w = w, .val = &val };
char* captured = capture_stdout_during(call_update_resvals_1, &ctx);
TEST_ASSERT_NOT_NULL(captured);
TEST_ASSERT_NULL_MESSAGE(
strstr(captured, "Resource temporarily unavailable"),
"must not log the ambient errno's string (EAGAIN) -- that is not "
"why update_resval failed");
TEST_ASSERT_NULL_MESSAGE(
strstr(captured, "Success"),
"must not log \"Success\" for a call that returned rc<0");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(captured, "does-not-exist-in-resmgr"),
"must name which resource failed");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(captured, "No such file or directory"),
"must log the real reason (-ENOENT, from key_find on an "
"unregistered resource name)");
free(captured);
teardown_test_cluster();
printf("test_update_resvals_logs_actual_rc_not_stale_errno: PASS\n");
}
/* ── Task 1.1: Tests for apply_join_state ──────────────────────────── */ /* ── Task 1.1: Tests for apply_join_state ──────────────────────────── */
/* /*
@ -295,6 +422,60 @@ static void test_apply_join_state_updates_resource_values(void)
printf("test_apply_join_state_updates_resource_values: PASS\n"); printf("test_apply_join_state_updates_resource_values: PASS\n");
} }
/*
* hconfig.mxj == -1 is the schema's documented "auto-detect" sentinel
* (hosts.schema.json: "max_slot": {"minimum": -1, "description": "-1 means
* auto-detect"}) -- apply_join_state must fall back to the worker's own
* reported maxcpus, NOT persist -1 (or, before the xceadmin mapping fix,
* silently ship 0 and permanently disable scheduling on that host: mxj is
* UInt32 on the wire, so an unset/absent max_slot used to decode as 0,
* which is not < 0 and so never reached this fallback -- see
* xcecommon.mapping.HOSTCONFIG_MXJ_AUTO_DETECT). This is the ONLY test in
* this file that exercises the nslots<0 branch; every sibling test above
* uses a positive hconfig.mxj and so cannot catch a regression here.
*/
static void test_apply_join_state_mxj_negative_one_falls_back_to_maxcpus(void)
{
setup_test_cluster();
hostattrs_t attrs;
memset(&attrs, 0, sizeof(attrs));
attrs.hosttype = makeString("LINUX");
attrs.hostmodel = makeString("x86");
attrs.cpufactor = 1.0f;
attrs.maxcpus = 12;
attrs.maxmem = 64;
attrs.maxswap = 128;
attrs.maxtmp = 256;
attrs.ndisks = 2;
worker_t* w = make_test_worker("host-ajs-autodetect", &attrs);
TEST_ASSERT_NOT_NULL(w);
xce_hostconfig_t hconfig;
memset(&hconfig, 0, sizeof(hconfig));
hconfig.hosttype = (char*)"!";
hconfig.hostmodel = (char*)"!";
hconfig.mxj = -1;
endpoint_t ep;
memset(&ep, 0, sizeof(ep));
lock(w, RTTI_LOCK_WRITE);
int rc = apply_join_state(&g_tc.c, w, &ep, &hconfig);
unlock(w);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
12.0f, get_worker_res_fval(w, "slots"),
"mxj=-1 must auto-detect slots from the worker's reported maxcpus, "
"not persist -1 or silently leave slots at 0");
teardown_test_cluster();
printf("test_apply_join_state_mxj_negative_one_falls_back_to_maxcpus: "
"PASS\n");
}
/* ── Task 1.2: Tests for init_worker ────────────────────────────────── */ /* ── Task 1.2: Tests for init_worker ────────────────────────────────── */
/* /*
@ -520,10 +701,13 @@ void test_worker_join(void)
/* xstring_table_init must be called once before any makeString usage */ /* xstring_table_init must be called once before any makeString usage */
xstring_table_init(139); xstring_table_init(139);
RUN_TEST(test_update_resvals_logs_actual_rc_not_stale_errno);
/* Task 1.1: apply_join_state (shared status/resource update) */ /* Task 1.1: apply_join_state (shared status/resource update) */
RUN_TEST(test_apply_join_state_clears_unavail); RUN_TEST(test_apply_join_state_clears_unavail);
RUN_TEST(test_apply_join_state_applies_hosttype_override); RUN_TEST(test_apply_join_state_applies_hosttype_override);
RUN_TEST(test_apply_join_state_updates_resource_values); RUN_TEST(test_apply_join_state_updates_resource_values);
RUN_TEST(test_apply_join_state_mxj_negative_one_falls_back_to_maxcpus);
/* Task 1.2: init_worker (first-time join) */ /* Task 1.2: init_worker (first-time join) */
RUN_TEST(test_init_worker_creates_new_entry); RUN_TEST(test_init_worker_creates_new_entry);

View file

@ -115,7 +115,7 @@ static int update_resval(cluster_t* c, worker_t* w, xce_resval_t* val)
} }
if (r == NULL) { if (r == NULL) {
ERROR("resource `%s' can not be found", val->name); ERROR("resource `%s' can not be found", val->name);
return -errno; return -ENOENT;
} }
if (r->idx < 0) { if (r->idx < 0) {
@ -130,6 +130,24 @@ static int update_resval(cluster_t* c, worker_t* w, xce_resval_t* val)
return rc; return rc;
} }
/*
* update_resval() returns its failure reason as a raw negative code
* directly (key_find/zarray_put convention throughout this codebase --
* never via errno), so logging it with PERROR (which reads the process-
* global errno) reported whatever errno happened to be left over from an
* unrelated earlier call -- "Success" if nothing had touched it, or a
* stray EAGAIN/etc from some other syscall in the same event-loop
* iteration, never the real reason. Decode the actual rc instead.
*/
static void log_update_resval_failure(const char* name, int rc)
{
char ebuf[128];
ebuf[0] = '\0';
xce_strerror_r(-rc, ebuf, sizeof(ebuf));
ERROR("update_resval `%s': %s", name, ebuf);
}
int update_resvals(cluster_t* c, worker_t* w, int nres, xce_resval_t* vals) int update_resvals(cluster_t* c, worker_t* w, int nres, xce_resval_t* vals)
{ {
int rc = 0; int rc = 0;
@ -138,7 +156,7 @@ int update_resvals(cluster_t* c, worker_t* w, int nres, xce_resval_t* vals)
for (i = 0; i < nres; i++) { for (i = 0; i < nres; i++) {
rc = update_resval(c, w, &(vals[i])); rc = update_resval(c, w, &(vals[i]));
if (rc < 0) { if (rc < 0) {
PERROR("update_resval"); log_update_resval_failure(vals[i].name, rc);
} }
} }
@ -153,7 +171,7 @@ int update_resval_list(cluster_t* c, worker_t* w, xce_resval_list_t* l)
for (i = 0; i < l->count; i++) { for (i = 0; i < l->count; i++) {
rc = update_resval(c, w, l->vals[i]); rc = update_resval(c, w, l->vals[i]);
if (rc < 0) { if (rc < 0) {
PERROR("update_resval"); log_update_resval_failure(l->vals[i]->name, rc);
} }
} }

View file

@ -49,6 +49,7 @@ typedef struct {
char* name; char* name;
char* host; char* host;
xce_val_t* val; xce_val_t* val;
char* unit;
} xce_resval_t; } xce_resval_t;
typedef struct { typedef struct {
@ -142,6 +143,7 @@ typedef struct {
int ndistributions; int ndistributions;
res_distribution_t** distributions; res_distribution_t** distributions;
xce_val_t* hbased_defined; xce_val_t* hbased_defined;
char* unit;
} xce_resource_t; } xce_resource_t;
typedef struct { typedef struct {

View file

@ -35,6 +35,7 @@ static int resobj_init(meta_obj_t* obj, va_list ap)
dst->name = makeString(src->name); dst->name = makeString(src->name);
dst->desc = makeString(src->desc); dst->desc = makeString(src->desc);
dst->unit = makeString(src->unit);
#if 0 #if 0
if (src->distributions != NULL) { if (src->distributions != NULL) {
@ -64,6 +65,7 @@ static void resobj_destroy(meta_obj_t* obj)
dereference(r->name); dereference(r->name);
dereference(r->desc); dereference(r->desc);
dereference(r->unit);
memset(priv, 0x0, sizeof(*priv)); memset(priv, 0x0, sizeof(*priv));

View file

@ -209,6 +209,17 @@ struct Resource $C.mapname("xce_resource_t"){
lastReport @7: UInt64 $C.mapname("last_report"); lastReport @7: UInt64 $C.mapname("last_report");
# distributions @8: List(ResourceDistribution) $C.maplistcount("ndistributions"); # distributions @8: List(ResourceDistribution) $C.maplistcount("ndistributions");
# hostBased @9: Value $C.mapname("hbased_defined"); # hostBased @9: Value $C.mapname("hbased_defined");
# capnp requires ordinals to be sequential with no holes, so @8/@9 above
# (commented out, not yet implemented) cannot be skipped -- unit takes @8
# here; whoever implements distributions/hostBased will need to renumber
# them to @9/@10 rather than reclaiming @8.
# Optional display-unit hint (e.g. "bytes") for resources whose canonical
# internal value isn't already human-scale -- empty/unset means "raw
# number, no conversion" (every pre-existing resource, unaffected). CLI
# display (resval/workerlist) converts to a human-readable form for
# known units instead of the raw value; the stored/wire value itself
# stays canonical (e.g. maxmem stays bytes, never converted at rest).
unit @8: Text;
} }
struct Threshold $C.mapname("xce_res_threshold_t"){ struct Threshold $C.mapname("xce_res_threshold_t"){
@ -346,6 +357,9 @@ struct ResourceVal $C.mapname("xce_resval_t") {
name @0: Text $C.mapname("name"); name @0: Text $C.mapname("name");
host @1: Text $C.mapname("host"); host @1: Text $C.mapname("host");
val @2: Value $C.mapname("val"); val @2: Value $C.mapname("val");
# Display-unit hint copied from the resource's own definition (Resource.unit)
# at reply-build time -- empty/unset means "raw number, no conversion".
unit @3: Text $C.mapname("unit");
} }
struct ResourceVals $C.mapname("xce_resval_data_t") { struct ResourceVals $C.mapname("xce_resval_data_t") {

View file

@ -71,9 +71,13 @@ reason -- never a silent drop.
runtime (e.g. ``scheduler.c`` copies these three fields straight out runtime (e.g. ``scheduler.c`` copies these three fields straight out
of the parsed resource). See :data:`_RESOURCE_VALTYPE`, of the parsed resource). See :data:`_RESOURCE_VALTYPE`,
:data:`_RESOURCE_ORDTYPE`, and the flag constants below for the exact :data:`_RESOURCE_ORDTYPE`, and the flag constants below for the exact
values. ``idx`` and ``lastReport`` remain unset: ``idx`` has no YAML values. ``unit`` is mapped straight through from YAML when present --
source at all (controller/runtime-assigned), and ``lastReport`` is a a display-only hint (e.g. ``"bytes"``) the controller CLI (``resval``/
runtime timestamp field (epoch seconds of last report) -- excluded by ``workerlist``) uses to render a human-readable value; the stored
value itself is never converted, only its display. ``idx`` and
``lastReport`` remain unset: ``idx`` has no YAML source at all
(controller/runtime-assigned), and ``lastReport`` is a runtime
timestamp field (epoch seconds of last report) -- excluded by
Requirement 9.2's "no timestamp" rule. ``resourcemap`` has no Requirement 9.2's "no timestamp" rule. ``resourcemap`` has no
destination: the capnp fields it would map to (``distributions``, destination: the capnp fields it would map to (``distributions``,
``hostBased``) are commented out of the ``Resource`` struct ``hostBased``) are commented out of the ``Resource`` struct
@ -95,13 +99,19 @@ reason -- never a silent drop.
no corresponding ``QueueConf`` field at all. no corresponding ``QueueConf`` field at all.
``HostConfig`` (``hosts.yaml`` hosts) ``HostConfig`` (``hosts.yaml`` hosts)
``description`` has no corresponding field. ``mxj`` is set only when ``description`` has no corresponding field. ``mxj`` is ``UInt32`` on the
YAML ``max_slot >= 0``; the schema's documented ``-1`` ("auto-detect") wire and cannot represent the schema's documented ``-1`` ("auto-detect
sentinel is left unmapped because ``mxj`` is ``UInt32`` and cannot from CPU count") sentinel directly (verified: assigning ``-1`` raises
represent a negative value (verified: assigning ``-1`` raises ``capnp.KjException`` rather than wrapping). When YAML ``max_slot`` is
``capnp.KjException`` rather than wrapping) -- shipping ``mxj`` unset absent or negative, ``mxj`` is instead set to ``HOSTCONFIG_MXJ_AUTO_DETECT``
(default ``0``) preserves "not sourced" instead of crashing the build (``0xFFFFFFFF``) -- the controller's ``int mxj`` field receives this via
or silently wrapping to ``4294967295``. ``capn_read32()``'s implicit ``uint32_t`` -> ``int`` conversion, which
reinterprets it back to ``-1`` on this project's (two's-complement)
target platforms, so ``apply_join_state()``'s ``if (nslots < 0)``
auto-detect fallback fires as intended. Previously this left ``mxj``
unset (default ``0``), which is indistinguishable from "explicitly
configure 0 slots" -- silently disabling scheduling on every host that
relies on auto-detect (bug, fixed).
``GroupConfig`` used as an embedded ``Membership`` (``QueueConf.users``/ ``GroupConfig`` used as an embedded ``Membership`` (``QueueConf.users``/
``.hosts``, ``LimitConfig.users``/``.queues``/``.projects``/``.hosts``) ``.hosts``, ``LimitConfig.users``/``.queues``/``.projects``/``.hosts``)
@ -318,6 +328,8 @@ def _build_resource(item: dict, target: "capnp._DynamicStructBuilder") -> None:
if item.get("release"): if item.get("release"):
flags |= _RESOURCE_FLAG_RELEASE flags |= _RESOURCE_FLAG_RELEASE
target.flags = flags target.flags = flags
if "unit" in item:
target.unit = item["unit"]
def _build_queue(item: dict, target: "capnp._DynamicStructBuilder") -> None: def _build_queue(item: dict, target: "capnp._DynamicStructBuilder") -> None:
@ -341,6 +353,20 @@ def _build_queue(item: dict, target: "capnp._DynamicStructBuilder") -> None:
_build_time_window(item.get("run_window"), target.runWindow) _build_time_window(item.get("run_window"), target.runWindow)
HOSTCONFIG_MXJ_AUTO_DETECT = 0xFFFFFFFF
"""Wire bit-pattern for the ``max_slot: -1`` ("auto-detect from CPU count")
sentinel. ``mxj`` is ``UInt32`` on the wire, so the negative sentinel itself
cannot be assigned directly (pycapnp raises ``capnp.KjException`` for a
negative value) -- but ``0xFFFFFFFF`` is a valid in-range ``UInt32`` value
that the controller's ``int mxj`` field (``xce/types.h``) receives via
``capn_read32()``'s implicit ``uint32_t`` -> ``int`` conversion, which on
this project's target platforms (two's-complement) reinterprets it back to
``-1`` -- reconstructing the sentinel losslessly end-to-end. See module
docstring, ``HostConfig``, for why this is unset-vs-sentinel and not a
schema change.
"""
def _build_host_config(item: dict, target: "capnp._DynamicStructBuilder") -> None: def _build_host_config(item: dict, target: "capnp._DynamicStructBuilder") -> None:
"""Populate one ``HostConfig`` list item from a ``hosts.yaml`` ``hosts`` """Populate one ``HostConfig`` list item from a ``hosts.yaml`` ``hosts``
entry. See module docstring, ``HostConfig``, for what is intentionally entry. See module docstring, ``HostConfig``, for what is intentionally
@ -354,6 +380,8 @@ def _build_host_config(item: dict, target: "capnp._DynamicStructBuilder") -> Non
max_slot = item.get("max_slot") max_slot = item.get("max_slot")
if max_slot is not None and max_slot >= 0: if max_slot is not None and max_slot >= 0:
target.mxj = max_slot target.mxj = max_slot
else:
target.mxj = HOSTCONFIG_MXJ_AUTO_DETECT
_build_config_var_list(item.get("resource"), target) _build_config_var_list(item.get("resource"), target)
_build_threshold_list(item.get("sched_stop"), target, "thresholds") _build_threshold_list(item.get("sched_stop"), target, "thresholds")
_build_time_window(item.get("dispatch_window"), target.dispatchwindows) _build_time_window(item.get("dispatch_window"), target.dispatchwindows)

Binary file not shown.

View file

@ -173,6 +173,20 @@ def test_map_list_section_noop_on_empty_or_missing_items(cluster_config):
assert len(cluster_config.resources) == 0 assert len(cluster_config.resources) == 0
def test_map_list_section_resources_maps_unit_when_present(cluster_config):
items = [
{"name": "maxmem", "type": "numeric", "unit": "bytes"},
{"name": "maxcpus", "type": "numeric"},
]
mapping.map_list_section(_spec_for("resources"), items, cluster_config)
r0, r1 = cluster_config.resources
assert r0.unit == "bytes"
# Every existing resource has no "unit" key -- must stay unset (empty
# Text default), not silently inherit or default to any value.
assert r1.unit == ""
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# map_list_section: users.yaml/hosts.yaml -> ugroups/hgroups (named groups) # map_list_section: users.yaml/hosts.yaml -> ugroups/hgroups (named groups)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@ -247,9 +261,11 @@ def test_map_list_section_host_config_full_field_mapping(cluster_config):
assert h.name == "default" assert h.name == "default"
assert h.type == "x86_64-linux-gnu" assert h.type == "x86_64-linux-gnu"
assert h.model == "x86_64-linux-gnu" assert h.model == "x86_64-linux-gnu"
# max_slot == -1 (auto-detect sentinel) cannot be represented by # max_slot == -1 (auto-detect sentinel) cannot be represented directly
# UInt32 mxj -- deliberately left at default (see module docstring). # by UInt32 mxj -- mapped to the 0xFFFFFFFF bit-pattern instead, which
assert h.mxj == 0 # the controller's `int mxj` field reinterprets back to -1 (see module
# docstring and HOSTCONFIG_MXJ_AUTO_DETECT).
assert h.mxj == mapping.HOSTCONFIG_MXJ_AUTO_DETECT
assert [r.name for r in h.resources] == ["mg", "maxswap"] assert [r.name for r in h.resources] == ["mg", "maxswap"]
assert h.resources[0].value == "" assert h.resources[0].value == ""
@ -275,6 +291,22 @@ def test_map_list_section_host_config_sets_mxj_when_max_slot_nonnegative(cluster
assert cluster_config.hostConfigs[0].mxj == 256 assert cluster_config.hostConfigs[0].mxj == 256
def test_map_list_section_host_config_sets_mxj_auto_detect_sentinel_when_max_slot_absent(
cluster_config,
):
"""The production case: a host entry with no `max_slot` key at all
(the normal way to request auto-detect, per hosts.schema.json) must
NOT silently ship mxj=0 -- that reads to the controller as "explicitly
configure 0 slots" and permanently blocks scheduling on that host
(worker.c apply_join_state: `if (nslots < 0) nslots = maxcpus;` never
fires for 0). It must ship the 0xFFFFFFFF sentinel instead."""
items = [{"hostname": "worker3"}]
mapping.map_list_section(_spec_for("hostConfigs"), items, cluster_config)
assert cluster_config.hostConfigs[0].mxj == mapping.HOSTCONFIG_MXJ_AUTO_DETECT
assert mapping.HOSTCONFIG_MXJ_AUTO_DETECT != 0
def test_time_window_day_prefixed_spec_parses_day_hour_minute(cluster_config): def test_time_window_day_prefixed_spec_parses_day_hour_minute(cluster_config):
items = [ items = [
{ {