diff --git a/src/configs/resources.yaml b/src/configs/resources.yaml index a92b6b2a..1b2b00a5 100644 --- a/src/configs/resources.yaml +++ b/src/configs/resources.yaml @@ -58,7 +58,6 @@ resources: - name: maxmem type: numeric - unit: bytes description: "max memory in bytes (built-in host attribute; CLI display converts to a human-readable unit)" - name: maxswap diff --git a/src/configs/schema/resources.schema.json b/src/configs/schema/resources.schema.json index eaa3e835..b11620a4 100644 --- a/src/configs/schema/resources.schema.json +++ b/src/configs/schema/resources.schema.json @@ -13,7 +13,6 @@ "consumable": { "type": "boolean" }, "release": { "type": "boolean" }, "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": { "type": "array", "items": { diff --git a/src/controller/opget.c b/src/controller/opget.c index fab329d8..4194bd51 100644 --- a/src/controller/opget.c +++ b/src/controller/opget.c @@ -62,20 +62,9 @@ static int do_reply_resvals(cluster_t* c, endpoint_t* ep, rc = get_resval(c, condition->condition.reskey->resname, condition->condition.reskey->hostname, &val); if (rc >= 0) { - xce_resource_t* r = NULL; - resval.val = &val; resval.name = condition->condition.reskey->resname; 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.vals = vals; } diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 0d11699f..a1b1052a 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -2036,11 +2036,40 @@ cleanup: } /* - * 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. + * Resource names whose canonical stored/wire value is raw bytes (not a + * generic schema-driven unit system -- deliberately kept as a small, + * explicit, client-side list rather than a wire field, since exactly one + * resource needs this today and a wire-level unit hint would have grown + * every Resource/ResourceVal message regardless of whether it's used). + * CLI display picks the best-fit human unit adaptively (format_bytes_human); + * the stored value is never converted. + */ +static int is_bytes_valued_resource(const char* name) +{ + static const char* const BYTES_VALUED_RESOURCES[] = { "maxmem" }; + const int nres = (int)(sizeof(BYTES_VALUED_RESOURCES) + / sizeof(BYTES_VALUED_RESOURCES[0])); + int i; + + if (name == NULL) { + return 0; + } + + for (i = 0; i < nres; i++) { + if (strcmp(name, BYTES_VALUED_RESOURCES[i]) == 0) { + return 1; + } + } + + return 0; +} + +/* + * Render a raw byte count as a human-readable string (e.g. "30.90 GB"), + * adaptively picking the smallest unit the value fits in. 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) { @@ -2098,7 +2127,7 @@ int cmd_resval(int argc, char* argv[]) printf("%s\n", val->val->u.svalue); break; case Value_u_fvalue: - if ((val->unit != NULL) && (strcmp(val->unit, "bytes") == 0)) { + if (is_bytes_valued_resource(val->name)) { char hbuf[64]; format_bytes_human(val->val->u.fvalue, hbuf, diff --git a/src/include/xce/types.h b/src/include/xce/types.h index 19b2e337..4c0ee34a 100644 --- a/src/include/xce/types.h +++ b/src/include/xce/types.h @@ -49,7 +49,6 @@ typedef struct { char* name; char* host; xce_val_t* val; - char* unit; } xce_resval_t; typedef struct { @@ -143,7 +142,6 @@ typedef struct { int ndistributions; res_distribution_t** distributions; xce_val_t* hbased_defined; - char* unit; } xce_resource_t; typedef struct { diff --git a/src/lib/resobj.c b/src/lib/resobj.c index 866289f9..aa0b4aa2 100644 --- a/src/lib/resobj.c +++ b/src/lib/resobj.c @@ -35,7 +35,6 @@ static int resobj_init(meta_obj_t* obj, va_list ap) dst->name = makeString(src->name); dst->desc = makeString(src->desc); - dst->unit = makeString(src->unit); #if 0 if (src->distributions != NULL) { @@ -65,7 +64,6 @@ static void resobj_destroy(meta_obj_t* obj) dereference(r->name); dereference(r->desc); - dereference(r->unit); memset(priv, 0x0, sizeof(*priv)); diff --git a/src/schema/controller.capnp b/src/schema/controller.capnp index 83e9786c..42df5db3 100644 --- a/src/schema/controller.capnp +++ b/src/schema/controller.capnp @@ -209,17 +209,6 @@ struct Resource $C.mapname("xce_resource_t"){ lastReport @7: UInt64 $C.mapname("last_report"); # distributions @8: List(ResourceDistribution) $C.maplistcount("ndistributions"); # 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"){ @@ -357,9 +346,6 @@ struct ResourceVal $C.mapname("xce_resval_t") { name @0: Text $C.mapname("name"); host @1: Text $C.mapname("host"); 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") { diff --git a/src/xceadmin/src/xcecommon/mapping.py b/src/xceadmin/src/xcecommon/mapping.py index f7d8cb89..70bfd2c2 100644 --- a/src/xceadmin/src/xcecommon/mapping.py +++ b/src/xceadmin/src/xcecommon/mapping.py @@ -71,11 +71,7 @@ reason -- never a silent drop. runtime (e.g. ``scheduler.c`` copies these three fields straight out of the parsed resource). See :data:`_RESOURCE_VALTYPE`, :data:`_RESOURCE_ORDTYPE`, and the flag constants below for the exact - values. ``unit`` is mapped straight through from YAML when present -- - a display-only hint (e.g. ``"bytes"``) the controller CLI (``resval``/ - ``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 + values. ``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 @@ -328,8 +324,6 @@ def _build_resource(item: dict, target: "capnp._DynamicStructBuilder") -> None: if item.get("release"): flags |= _RESOURCE_FLAG_RELEASE target.flags = flags - if "unit" in item: - target.unit = item["unit"] def _build_queue(item: dict, target: "capnp._DynamicStructBuilder") -> None: diff --git a/src/xceadmin/tests/fixtures/full/expected.bin b/src/xceadmin/tests/fixtures/full/expected.bin index 83feb286..3da1557f 100644 Binary files a/src/xceadmin/tests/fixtures/full/expected.bin and b/src/xceadmin/tests/fixtures/full/expected.bin differ diff --git a/src/xceadmin/tests/test_mapping.py b/src/xceadmin/tests/test_mapping.py index 8694874c..9c2f72c6 100644 --- a/src/xceadmin/tests/test_mapping.py +++ b/src/xceadmin/tests/test_mapping.py @@ -173,20 +173,6 @@ def test_map_list_section_noop_on_empty_or_missing_items(cluster_config): 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) # ---------------------------------------------------------------------------