feat(scheduler-sim-diagnostics): promote byte-unit formatting to shared lib (task 7)
Adds Requirement 6 (Human-Readable Units for Byte-Valued Limit Amounts) to the spec: requirements, design, research, and tasks 7-10. Task 7 (Foundation): promotes the REPL's byte-unit scaling algorithm and byte-valued-resource classifier into src/lib/utils.c/utils.h (xce_format_bytes_human, xce_is_bytes_valued_resource), alongside a new display-mode-aware composer (xce_format_units_value, xce_units_mode_t) -- following the fmt_jobid()/fmt_jobid_ex() precedent. repl_fmt.c's scheduler-only functions become one-line delegating wrappers. repl_info.c's repl_format_numeric_resource_value() (shared by info worker/info resource, not just the upcoming info limit) is retyped to delegate to the new shared composer via a small translate_units_mode() bridge, so every consumer benefits from one implementation going forward. No existing symbol's signature changed; only new symbols were added. Confirmed behavior-preserving: full scheduler suite 85/85 unchanged, including info worker/info resource's own regression tests. One narrow, untested, out-of-requirement-scope corner case found and documented (swp's RAW/BOTH mode rendering under the new single-value composer interface) -- logged in tasks.md's Implementation Notes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
parent
8c363de75f
commit
eb23c158ae
10 changed files with 535 additions and 85 deletions
|
|
@ -1,4 +1,4 @@
|
|||
# Design Document: scheduler-sim-diagnostics (Requirement Set 1 — Limit Inspection)
|
||||
# Design Document: scheduler-sim-diagnostics (Requirement Sets 1 & 6 — Limit Inspection, Human-Readable Units)
|
||||
|
||||
## Overview
|
||||
|
||||
|
|
@ -13,35 +13,49 @@ This design covers the first capability of the open-ended `scheduler-sim-diagnos
|
|||
- Display live accounted usage per distinct scope instance (Requirements 2.1–2.3, 3.1–3.4).
|
||||
- Fit the existing `info <name|all>` command shape exactly (Requirements 4.1–4.2).
|
||||
- Guarantee zero effect on scheduling decisions, accounting state, or other `info` subcommands (Requirements 5.1–5.3).
|
||||
- Display a byte-valued limit's configured amount and live usage amounts in human-readable units, consistent with the REPL's existing units-mode convention for every other byte-valued resource display (Requirements 6.1–6.5, added 2026-09-19).
|
||||
|
||||
### Non-Goals
|
||||
- Decision-reasoning trace (seed capability 2 of this same open-ended spec) — separate, future requirement set.
|
||||
- Any live, controller-connected (non-simulation) inspection.
|
||||
- Any REPL command that mutates limit configuration or usage accounting.
|
||||
- Any change to `match_limit()`/`is_host_based_limit()`/allocation logic — this design reads existing state only.
|
||||
- Introducing a new units-mode toggle or a second, limit-specific display-mode concept — Requirement 6 reuses the REPL's one existing units-mode selection unchanged (Requirement 6 AC2).
|
||||
- Any change to a limit's stored/wire value, its matching arithmetic, or its accounting charge — this remains display-only, mirroring `repl_format_bytes_human()`'s own existing "display-only, never feeds back" discipline.
|
||||
|
||||
## Boundary Commitments
|
||||
|
||||
### This Spec Owns
|
||||
- The `info limit <name|all>` REPL command and its output format (`repl_info_limit()`, `repl_info.c`).
|
||||
- Two new read-only accessor functions exposing the limit plugin's private configured-limits and live-usage-instances tables to REPL code (`limit_get_configured_limits()`, `limit_get_usage_instances()`, `plugins/limit.c`).
|
||||
- **(Requirement 6, added 2026-09-19)** A new shared-library units-formatting primitive (`xce_units_mode_t`, `xce_format_bytes_human()`, `xce_is_bytes_valued_resource()`, `xce_format_units_value()`, `src/lib/utils.c`/`src/include/utils.h`), promoted from the scheduler-only equivalents `repl_fmt.c` already had — this spec owns the promotion and the new shared symbols, not `utils.c`'s pre-existing content.
|
||||
- **(Requirement 6)** A new sibling entry point, `fmt_limit_units()` (`src/lib/xcelimit.c`/`src/include/xcelimit.h`), and the correction of `repl_print_limit_usage()`/`repl_print_limit_usage_per_host()`'s resource-name argument bug (`repl_info.c`).
|
||||
|
||||
### Out of Boundary
|
||||
- `fmt_limit()`, `fmt_consumer()`, `fmt_group()` (`src/lib/xcelimit.c`, `src/lib/group.c`) — reused as-is, not modified.
|
||||
- `fmt_consumer()`, `fmt_group()` (`src/lib/xcelimit.c`, `src/lib/group.c`) — reused as-is, not modified.
|
||||
- `fmt_limit()`'s existing 3-argument signature and output contract — **explicitly preserved, not broken, by Requirement 6** (see the Reversed Decision note below); still out of boundary in the sense that no existing caller of `fmt_limit()` needs to change.
|
||||
- `is_host_based_limit()`, `match_limit()`, and the rest of the limit-matching/allocation path — read from, never modified.
|
||||
- `repl_info_resource()`/`repl_info_resource_one()`/`repl_info_worker_one()` — unmodified; this design follows their established GLOBAL/SHARED-vs-per-host read pattern but does not touch their code.
|
||||
- The pre-existing fact that a GLOBAL-scoped limit's synthetic capacity resource can already appear in `info resource all`/`info resource <name>` (flagged `LIMIT`) once instantiated — this is existing `scheduler-core` behavior, not introduced or altered here (Requirement 5.3 is satisfied by not touching that code path at all).
|
||||
- Decision-reasoning trace (seed capability 2) — future requirement set of this same spec.
|
||||
- Any change to `xce_repl_units_mode_t` (`repl_internal.h`) or its three existing call sites outside `repl_info.c`'s own two new/modified functions — Requirement 6 is satisfied with a small local translation at the seam, not by touching the REPL's existing units-mode plumbing.
|
||||
- `src/controller/testsuite/test.c`'s own `format_bytes_human()` — a separate, test-only, pre-existing duplicate of the same algorithm; not consolidated by this spec (no production consumer of it exists to justify touching controller test code here).
|
||||
|
||||
**Reversed decision note**: `research.md`'s original "Decision: Reuse `fmt_limit()`/`fmt_group()`" and "Decision: Effective-scope annotation is REPL-layer, not library-layer" (Requirement Set 1) both concluded `fmt_limit()` should stay untouched, reasoning that it is "a general-purpose library formatter used elsewhere" whose contract a single REPL caller should not reshape. Requirement 6 revisits this: confirmed by re-checking every caller (`repl_info.c`'s two call sites, `src/lib/testsuite/test_limit.c`'s five call sites — no other production or test consumer exists anywhere in the tree), `fmt_limit()` has exactly one production caller family (this spec's own REPL command) and one direct-unit-test file. Given that, and given the operator-facing need to select a display mode Requirement 6 AC2 requires, this design adds a new sibling entry point (`fmt_limit_units()`) that takes the additional units-mode parameter `fmt_limit()`'s own callers never needed, and makes `fmt_limit()` itself a one-line wrapper around it (`fmt_limit_units(buf, len, limit, XCE_UNITS_RAW)`) — provably byte-identical output to today's `fmt_limit()` for every input shape (see Requirement Traceability / Testing Strategy below), so none of `test_limit.c`'s five existing assertions change. This satisfies the *reformat-at-the-source* approach the user selected during discovery without breaking `fmt_limit()`'s existing contract for its one other consumer (the library's own unit tests).
|
||||
|
||||
### Allowed Dependencies
|
||||
- `core->resmgr`, `core->sharedInst`, `core->sharedInUse` — already read by `repl_info_resource()`/`repl_info_resource_one()` today. `core->workers[].resvals`/`core->workers[].resInUse` — the capacity side (`resvals`) is already read by `repl_info_worker_one()`; the in-use side (`resInUse`) is new to the REPL layer but is the exact pattern `sched_check_host_satisfy_()` (`resource.c`) already uses in production. `core->plugins` (name-keyed plugin registry) — read elsewhere in the scheduler (`plugin.c`'s hook fan-outs) but genuinely new to the REPL `info` layer; this design is the first `info` subcommand to cross into plugin-private state, which is why the Limit Plugin Accessor exists as its own boundary rather than being folded into `repl_info.c` directly.
|
||||
- The limit plugin's fixed registration name `"xce_sched_limit"` — owned entirely inside `limit.c`; no other file may hardcode it.
|
||||
- **(Requirement 6)** `xce_vres_t.limit` (existing back-pointer to the owning `xce_limit_t`, already read by `repl_print_limit_usage()`'s own `vres->limit != limit` filter) — the fix for the resource-name argument bug reads `vres->limit->val.resname` from this existing pointer; no new field is added.
|
||||
- **(Requirement 6)** `src/include/utils.h`/`src/lib/utils.c` — already linked into every component via `libxce` (`fmt_jobid()` lives here, the direct precedent for a small, generic, cross-component display-formatting helper with no domain coupling).
|
||||
|
||||
### Revalidation Triggers
|
||||
- A change to `xce_consumer_type_t` (adding/removing a scope dimension) — Requirement 1.1/3.1 enumerate the four dimensions explicitly.
|
||||
- A change to `is_host_based_limit()`'s percent/global-resource rule — Requirement 1.6/3.2 depend on its exact current semantics.
|
||||
- A change to how `add_vres()` flags synthetic capacity resources, or to `repl_info_resource_one()`'s GLOBAL/SHARED-vs-host read split — this design's live-usage read path mirrors that split and must be re-checked if it changes.
|
||||
- Renaming the limit plugin's registration string away from `"xce_sched_limit"`.
|
||||
- **(Requirement 6)** A change to `xce_repl_units_mode_t`'s three values (`repl_internal.h`) must keep `repl_info.c`'s new `translate_units_mode()` helper's mapping to `xce_units_mode_t` (`utils.h`) in sync — the two enums are intentionally independent types, not a shared definition, so nothing enforces this mapping mechanically.
|
||||
- A future second production caller of `fmt_limit()`/`fmt_limit_units()` (today there are none besides this spec's own REPL command and the library's own unit tests) should re-confirm the RAW-mode-is-byte-identical claim this design relies on still holds.
|
||||
|
||||
## Architecture
|
||||
|
||||
|
|
@ -56,10 +70,15 @@ graph TB
|
|||
CmdInfo --> InfoLimit[repl_info_limit new]
|
||||
InfoLimit --> Accessor1[limit_get_configured_limits new]
|
||||
InfoLimit --> Accessor2[limit_get_usage_instances new]
|
||||
InfoLimit --> FmtLimit[fmt_limit existing]
|
||||
InfoLimit --> FmtLimitUnits[fmt_limit_units new]
|
||||
FmtLimit[fmt_limit existing] --> FmtLimitUnits
|
||||
InfoLimit --> HostCheck[is_host_based_limit existing]
|
||||
FmtLimit --> FmtConsumer[fmt_consumer existing]
|
||||
FmtLimitUnits --> FmtConsumer[fmt_consumer existing]
|
||||
FmtConsumer --> FmtGroup[fmt_group existing]
|
||||
FmtLimitUnits --> SharedFmt[xce_format_units_value new]
|
||||
InfoLimit --> SharedFmt
|
||||
SharedFmt --> BytesHuman[xce_format_bytes_human new]
|
||||
InfoLimit --> IsBytes[xce_is_bytes_valued_resource new]
|
||||
Accessor1 --> PluginRegistry[core.plugins existing]
|
||||
Accessor2 --> PluginRegistry
|
||||
InfoLimit --> SharedState[core.sharedInst core.sharedInUse existing]
|
||||
|
|
@ -80,6 +99,7 @@ graph TB
|
|||
| Scheduler REPL (C) | Existing `src/scheduler` toolchain (Meson/GCC, ASAN debug profile per `build-deployment.md`) | New `info limit` command | No new dependency |
|
||||
| Scheduler plugin (C) | Existing `src/scheduler/src/plugins/limit.c` | New read-only accessors | No new dependency |
|
||||
| Shared lib (C) | Existing `src/lib/xcelimit.c`, `src/lib/group.c` | Reused formatters (`fmt_limit`, `fmt_consumer`, `fmt_group`) | No new dependency; already built into `libxce` |
|
||||
| Shared lib (C) | Existing `src/lib/utils.c`/`src/include/utils.h` | **(Requirement 6)** New units-formatting primitives, promoted from scheduler-only equivalents | No new dependency; already built into `libxce`, already linked by controller/worker/scheduler/apiserver(CGO) |
|
||||
|
||||
No new external dependency is introduced.
|
||||
|
||||
|
|
@ -88,11 +108,19 @@ No new external dependency is introduced.
|
|||
### Modified Files
|
||||
- `src/scheduler/include/plugin.h` — add two `extern` declarations: `limit_get_configured_limits()`, `limit_get_usage_instances()`, placed next to the existing `register_limit()` declaration.
|
||||
- `src/scheduler/src/plugins/limit.c` — define the two accessor functions. Each does `key_find(core->plugins, "xce_sched_limit", &plugin)`, returns `NULL` if not found or `plugin->data == NULL`, otherwise returns the relevant `limit_plugin_data_t` field (`limits` or `instances`).
|
||||
- `src/scheduler/src/repl_info.c` — add `repl_info_limit()` (the `all` case: iterate `limit_get_configured_limits()` via `iter_make()`/`iter_next()`, print each) and an inner per-limit printer (the `<name>` case: `key_find()` directly on `limit_get_configured_limits()`'s handle — the plugin's kvmgr is keyed by limit name, confirmed via bug-1685's duplicate-name `-EEXIST` handling, so this is a direct lookup, not a scan; report "no such limit" per Requirement 1.3 if not found, report "no limits configured" per Requirement 1.7 if the accessor returns `NULL` or an empty handle on the `all` path); add one new `else if (strcmp(tokens[1], "limit") == 0)` branch to `repl_cmd_info()`, and extend its usage-string literal to include `limit`.
|
||||
- `src/scheduler/src/repl_info.c` — add `repl_info_limit()` (the `all` case: iterate `limit_get_configured_limits()` via `iter_make()`/`iter_next()`, print each) and an inner per-limit printer (the `<name>` case: `key_find()` directly on `limit_get_configured_limits()`'s handle — the plugin's kvmgr is keyed by limit name, confirmed via bug-1685's duplicate-name `-EEXIST` handling, so this is a direct lookup, not a scan; report "no such limit" per Requirement 1.3 if not found, report "no limits configured" per Requirement 1.7 if the accessor returns `NULL` or an empty handle on the `all` path); add one new `else if (strcmp(tokens[1], "limit") == 0)` branch to `repl_cmd_info()`, and extend its usage-string literal to include `limit`. **(Requirement 6)** Also: add a small `static xce_units_mode_t translate_units_mode(xce_repl_units_mode_t)` helper; switch the two existing `fmt_limit()` call sites (inside the per-limit printer, one for the `all` path and one for the named-lookup path) to `fmt_limit_units(buf, len, limit, translate_units_mode(repl_current_units_mode(sim)))`; fix `repl_print_limit_usage()`/`repl_print_limit_usage_per_host()` to pass `limit->val.resname`/`vres->limit->val.resname` (not `vres->vres->name`) to `repl_format_consumable_value_and_used()`; retype `repl_format_numeric_resource_value()`'s byte-valued branch to delegate to the new `xce_format_units_value()` via `translate_units_mode()`.
|
||||
- `src/scheduler/src/repl_fmt.c` — **(Requirement 6)** `repl_format_bytes_human()`/`repl_is_bytes_valued_resource()` become one-line wrappers delegating to the new `xce_format_bytes_human()`/`xce_is_bytes_valued_resource()` (`utils.h`), removing the duplicated algorithm while leaving every existing scheduler call site and its function name completely unchanged.
|
||||
- `src/include/utils.h` — **(Requirement 6, new declarations in an existing file)** add `xce_units_mode_t` (`XCE_UNITS_RAW`/`XCE_UNITS_HUMAN`/`XCE_UNITS_BOTH`), `extern void xce_format_bytes_human(double, char*, size_t)`, `extern bool xce_is_bytes_valued_resource(const char*)`, `extern void xce_format_units_value(xce_units_mode_t, double, char*, size_t)` — placed near the existing `fmt_jobid()`/`fmt_jobid_ex()` declarations, the closest existing precedent for a small generic display-formatting helper.
|
||||
- `src/lib/utils.c` — **(Requirement 6)** implement the three functions above, bodies promoted verbatim from `repl_fmt.c`'s current `repl_format_bytes_human()`/`repl_is_bytes_valued_resource()`, plus a new `xce_format_units_value()` promoted from `repl_info.c`'s current `repl_format_numeric_resource_value()`'s byte-valued branch (the RAW/HUMAN/BOTH `%f`/`%s`/`%f (%s)` composition only — the non-byte-valued fallback formatting stays in `repl_info.c`, since `fmt_limit_units()` never needs it).
|
||||
- `src/include/xcelimit.h` — **(Requirement 6)** add `#include "utils.h"` (for `xce_units_mode_t`) and `extern int fmt_limit_units(char*, int, xce_limit_t*, xce_units_mode_t);`, placed next to the existing `fmt_limit()` declaration.
|
||||
- `src/lib/xcelimit.c` — **(Requirement 6)** implement `fmt_limit_units()` (identical consumer-dump loop to today's `fmt_limit()`; its final value line branches on `limit->val.percent` first — percent case keeps today's exact `"%f%%"` output regardless of mode — then, for the non-percent case, on `xce_is_bytes_valued_resource(limit->val.resname)` plus the same `"swp"` MB-special-case check the scheduler's own formatter already applies, calling `xce_format_units_value()` only in that branch); retype `fmt_limit()` itself to `return fmt_limit_units(buf, len, limit, XCE_UNITS_RAW);` — provably byte-identical to today's output for every input (RAW mode's `%f` composition matches today's bare `%f` exactly in both the percent and non-percent cases), so this is a behavior-preserving refactor of `fmt_limit()`, not a breaking change.
|
||||
- `src/scheduler/testsuite/test_limit_plugin.c` — add unit tests for the two new accessors (registered-plugin case, not-yet-initialized case).
|
||||
- `src/scheduler/testsuite/test_repl_limit_table.c` (new sibling of `test_repl_resource_table.c`, matching its dispatch-capture harness — kept separate rather than added to `test_repl_resource_table.c` itself, since it needs a different white-box `#include` set, see Implementation Notes below) — REPL-level tests for `info limit <name>`/`info limit all`.
|
||||
- `src/scheduler/testsuite/test_repl_limit_table.c` (new sibling of `test_repl_resource_table.c`, matching its dispatch-capture harness — kept separate rather than added to `test_repl_resource_table.c` itself, since it needs a different white-box `#include` set, see Implementation Notes below) — REPL-level tests for `info limit <name>`/`info limit all`. **(Requirement 6)** Its existing "mem"-resourced fixtures' `accounted=`/`capacity=` assertions (added under Requirement Set 1, e.g. `"accounted=4.00"`) were asserting on the resource-name-argument bug's buggy fallback output (plain `%.2f`, since `repl_is_bytes_valued_resource()` was never actually being asked about `"mem"` due to the bug) — these must be updated to expect the corrected human-formatted output once the bug is fixed, not left as a false "still passing" signal; see Testing Strategy below for the required RED→GREEN treatment.
|
||||
- `src/lib/testsuite/test_limit.c` — **(Requirement 6)** no changes expected — `fmt_limit()`'s output is unchanged by construction (see the Reversed Decision note above); its five existing `fmt_limit()` assertions are the regression proof for that claim, not a file this design needs to edit.
|
||||
- `src/lib/testsuite/test_utils.c` — **(Requirement 6)** add unit tests for `xce_format_bytes_human()`, `xce_is_bytes_valued_resource()`, `xce_format_units_value()` (all three modes), mirroring `test_limit.c`'s existing assertion style.
|
||||
- `src/scheduler/testsuite/test_repl_resource_formatting.c` — **(Requirement 6)** no changes expected — `repl_format_bytes_human()`/`repl_is_bytes_valued_resource()`'s observable behavior is unchanged (delegation only); this file's existing assertions are the regression proof for that claim.
|
||||
|
||||
No new files. No existing formatter, matcher, or allocation-path file changes.
|
||||
One new pair of shared-library symbols is added to an existing file (`utils.c`/`utils.h`); one new sibling function is added to an existing file (`xcelimit.c`/`xcelimit.h`). No new files besides the already-planned `test_repl_limit_table.c`. No change to `match_limit()`, `is_host_based_limit()`, `instantiate_limit()`, or any allocation-path file.
|
||||
|
||||
## System Flows
|
||||
|
||||
|
|
@ -103,7 +131,8 @@ sequenceDiagram
|
|||
participant Info as repl_cmd_info
|
||||
participant Limit as repl_info_limit
|
||||
participant Plugin as limit.c accessors
|
||||
participant Fmt as fmt_limit/fmt_group
|
||||
participant Fmt as fmt_limit_units/fmt_group
|
||||
participant Units as xce_format_units_value
|
||||
participant Core as core state
|
||||
|
||||
Op->>Dispatch: info limit ug1_slots
|
||||
|
|
@ -113,13 +142,18 @@ sequenceDiagram
|
|||
Plugin->>Core: key_find core.plugins xce_sched_limit
|
||||
Plugin-->>Limit: limits handle or NULL
|
||||
Limit->>Limit: key_find name lookup
|
||||
Limit->>Fmt: fmt_limit matched limit
|
||||
Limit->>Limit: translate_units_mode repl_current_units_mode
|
||||
Limit->>Fmt: fmt_limit_units matched limit units_mode
|
||||
Fmt->>Units: xce_format_units_value when byte valued and not percent
|
||||
Units-->>Fmt: raw human or both text
|
||||
Fmt-->>Limit: configured setting text
|
||||
Limit->>Limit: is_host_based_limit effective scope check
|
||||
Limit->>Plugin: limit_get_usage_instances core
|
||||
Plugin-->>Limit: instances handle
|
||||
Limit->>Limit: filter instances by pointer equality to matched limit
|
||||
Limit->>Core: read sharedInst/sharedInUse or per-worker resvals/resInUse
|
||||
Limit->>Units: xce_format_units_value via limit val resname when byte valued
|
||||
Units-->>Limit: accounted and capacity text
|
||||
Limit-->>Op: configured setting plus per-instance usage
|
||||
```
|
||||
|
||||
|
|
@ -148,13 +182,19 @@ Key decisions not obvious from the diagram: the "no such limit"/"no limits confi
|
|||
| 5.1 | No mutation of config/accounting/decisions | REPL Limit Info Command | read-only accessors, no write calls | — |
|
||||
| 5.2 | Identical replay outcome whether or not queried | REPL Limit Info Command | no state written, no hook installed | — |
|
||||
| 5.3 | Existing `info` subcommands unchanged | REPL dispatch | additive-only `repl_cmd_info()` branch | — |
|
||||
| 6.1 | Byte-valued amounts shown in human-readable scale | Shared Units Formatter, REPL Limit Info Command | `xce_format_units_value()`, `fmt_limit_units()` | Sequence diagram (updated) |
|
||||
| 6.2 | Respects the operator's existing units-mode selection | REPL Limit Info Command | `translate_units_mode()`, `repl_current_units_mode()` (existing) | — |
|
||||
| 6.3 | Non-byte-valued and percentage amounts excluded from scaling | Shared Units Formatter, REPL Limit Info Command | `xce_is_bytes_valued_resource()`, `fmt_limit_units()`'s percent-first branch | — |
|
||||
| 6.4 | Percentage-of-byte-resource resolved amount scaled the same way | REPL Limit Info Command | corrected `repl_format_consumable_value_and_used()` call (resname bug fix) | Sequence diagram (updated) |
|
||||
| 6.5 | Consistent units convention across config display and usage display | Shared Units Formatter | one shared `xce_format_units_value()`/`xce_is_bytes_valued_resource()` pair used by both surfaces | — |
|
||||
|
||||
## Components and Interfaces
|
||||
|
||||
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|
||||
|-----------|---------------|--------|---------------|---------------------------|-----------|
|
||||
| Limit Plugin Accessor | Scheduler Plugin (`limit.c`) | Expose the limit plugin's private configured-limits/usage-instances tables as raw iterable handles | 1.1, 1.2, 1.4, 2.1, 3.1 | `core->plugins` (P0) | Service |
|
||||
| REPL Limit Info Command | Scheduler REPL (`repl_info.c`) | Format and print `info limit <name|all>` output | 1.1–1.7, 2.1–2.3, 3.1–3.4, 4.1–4.2, 5.1–5.3 | Limit Plugin Accessor (P0), `fmt_limit`/`fmt_group` (P0), `is_host_based_limit` (P0), `core.sharedInst`/`core.workers` (P1) | Service |
|
||||
| REPL Limit Info Command | Scheduler REPL (`repl_info.c`) | Format and print `info limit <name|all>` output | 1.1–1.7, 2.1–2.3, 3.1–3.4, 4.1–4.2, 5.1–5.3, 6.2, 6.3, 6.4 | Limit Plugin Accessor (P0), `fmt_limit_units`/`fmt_group` (P0), `is_host_based_limit` (P0), `core.sharedInst`/`core.workers` (P1), Shared Units Formatter (P0) | Service |
|
||||
| Shared Units Formatter | Shared Library (`utils.c`, `xcelimit.c`) | Human-readable byte-unit scaling and mode-aware value composition, usable by any `libxce` consumer | 6.1, 6.3, 6.5 | none (leaf utility) | Service |
|
||||
|
||||
### Scheduler Plugin
|
||||
|
||||
|
|
@ -243,6 +283,54 @@ static void repl_info_limit(xce_sched_sim_driver_t* sim, const char* arg);
|
|||
- Validation: dispatch-capture tests per Requirement Traceability above, following `test_repl_resource_table.c`'s existing `run_dispatch_for_capture()` harness; at minimum one fixture with a percent-based limit on a per-host resource configured without `per_host` (exercises Requirement 1.6/3.2's implicit-scope case) and one fixture with an exclusion-based group (exercises Requirement 1.5).
|
||||
- Risks: **Build wiring** — `test_repl_resource_table.c` white-box `#include`s `repl_info.c` but deliberately does NOT include `plugins/limit.c` or `resource.c` today (confirmed by reading its include list; its own comment notes "resource.c, not linked here"). Adding calls from `repl_info_limit()` to the new `limit_get_configured_limits()`/`limit_get_usage_instances()` accessors without including `plugins/limit.c` (and, transitively, either `resource.c` or link-satisfying stubs for `get_resvals()`/`put_resvals()`, the same pattern `test_limit_plugin.c` already uses) produces undefined-reference link failures — the exact failure class bug-1762 hit and remediated for a different scheduler-sim-repl-verbosity command. This is the reason `test_repl_limit_table.c` is a new file rather than an addition to `test_repl_resource_table.c`: it needs its own white-box include set. Because this test needs to assert *real, non-trivial* accounted-usage values (not a stubbed-zero), it should white-box `resource.c` for real (not stub `get_resvals`/`put_resvals` as no-ops the way `test_limit_plugin.c` does for its narrower config-update-only scope) — left as a task-level decision, flagged here so it is made deliberately rather than discovered via a failed build.
|
||||
|
||||
### Shared Library
|
||||
|
||||
#### Shared Units Formatter
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Provide human-readable byte-unit scaling and mode-aware value composition as a leaf utility any `libxce` consumer can call, replacing the scheduler-only duplicate this design promotes from |
|
||||
| Requirements | 6.1, 6.3, 6.5 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Pure, stateless, display-only functions — never touch a stored/wire value, never allocate beyond the caller-supplied buffer.
|
||||
- `xce_is_bytes_valued_resource()` is a fixed 5-name list (`mem`, `tmp`, `maxmem`, `maxtmp`, `maxswap`) — identical to today's `repl_is_bytes_valued_resource()`; adding a sixth byte-valued resource name in the future means updating this one list, not two.
|
||||
- `xce_format_units_value()` assumes its caller has already decided the value is byte-valued (via `xce_is_bytes_valued_resource()` or the caller's own equivalent check, e.g. `fmt_limit_units()`'s `"swp"` special case) — it does not re-derive that decision itself, keeping it a pure three-mode composer.
|
||||
- Must not depend on any scheduler-only type (`xce_sched_core_t`, `xce_repl_units_mode_t`, etc.) — `xce_units_mode_t` is its own, independently-defined enum; crossing from the scheduler's `xce_repl_units_mode_t` happens at the scheduler-side call site (`repl_info.c`'s `translate_units_mode()`), never inside this component.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: REPL Limit Info Command (`fmt_limit_units()`, `repl_format_numeric_resource_value()`'s delegation) (P0)
|
||||
- Outbound: none — a leaf utility
|
||||
|
||||
**Contracts**: Service [x]
|
||||
|
||||
##### Service Interface
|
||||
```c
|
||||
/* src/include/utils.h */
|
||||
typedef enum {
|
||||
XCE_UNITS_RAW = 1,
|
||||
XCE_UNITS_HUMAN,
|
||||
XCE_UNITS_BOTH
|
||||
} xce_units_mode_t;
|
||||
|
||||
extern void xce_format_bytes_human(double bytes, char* buf, size_t bufsz);
|
||||
extern bool xce_is_bytes_valued_resource(const char* name);
|
||||
extern void xce_format_units_value(xce_units_mode_t mode, double bytes_value,
|
||||
char* buf, size_t bufsz);
|
||||
|
||||
/* src/include/xcelimit.h */
|
||||
extern int fmt_limit_units(char* buf, int len, xce_limit_t* limit,
|
||||
xce_units_mode_t units_mode);
|
||||
```
|
||||
- Preconditions: `buf`/`bufsz` describe a valid writable buffer; `name` may be `NULL` (returns `false`).
|
||||
- Postconditions: `xce_format_units_value(XCE_UNITS_RAW, v, ...)` always produces the bare `"%f"` rendering of `v` — the exact contract `fmt_limit_units(..., XCE_UNITS_RAW)` relies on to keep `fmt_limit()` byte-identical to today.
|
||||
- Invariants: none of these functions mutate any input; `fmt_limit_units()`'s consumer-dump loop is byte-for-byte identical to today's `fmt_limit()` — only the final value line's formatting branches on `units_mode`, and only when `limit->val.percent == 0` and the resource is byte-valued.
|
||||
|
||||
**Implementation Notes**
|
||||
- Integration: `repl_fmt.c`'s `repl_format_bytes_human()`/`repl_is_bytes_valued_resource()` and `repl_info.c`'s `repl_format_numeric_resource_value()` become thin callers of these new functions (see File Structure Plan) — zero change to any of their own existing call sites' function names or signatures.
|
||||
- Validation: `test_utils.c` gets direct unit tests for all three functions and all three modes; `test_limit.c`'s five existing `fmt_limit()` assertions serve as the regression proof that `fmt_limit()`'s output is unchanged.
|
||||
- Risks: none identified beyond the Revalidation Trigger already noted (a future second production caller of `fmt_limit()`/`fmt_limit_units()` should re-confirm the byte-identical claim).
|
||||
|
||||
## Data Models
|
||||
|
||||
### Domain Model
|
||||
|
|
@ -296,8 +384,20 @@ Not applicable — this is an interactive/replay-local REPL command with no pers
|
|||
### E2E/REPL Tests
|
||||
- Full dispatch-capture round trip for `info limit ug1_slots` and `info limit all` against a realistic multi-limit fixture (mirrors `limits.yaml`'s shape referenced in `research.md`), asserting every existing `info` subcommand's output is byte-identical before and after this change (Requirement 5.3).
|
||||
|
||||
### Requirement 6 Tests (added 2026-09-19)
|
||||
- `xce_format_bytes_human()`: adaptive B/KB/MB/GB/TB/PB scaling for representative magnitudes, including the existing algorithm's own boundary cases (exactly 1024, just under it) (`test_utils.c`).
|
||||
- `xce_is_bytes_valued_resource()`: true for all five listed names, false for `NULL`, an empty string, and a non-listed name like `"slots"` (`test_utils.c`).
|
||||
- `xce_format_units_value()`: all three modes produce the documented `%f` / `%s` / `%f (%s)` shapes for a representative byte value (`test_utils.c`).
|
||||
- `fmt_limit()` (unchanged contract): re-run `test_limit.c`'s five existing assertions unmodified — they must still pass byte-for-byte, proving the `fmt_limit_units(..., XCE_UNITS_RAW)` refactor is behavior-preserving.
|
||||
- `fmt_limit_units()`: a byte-valued, non-percent limit (e.g. `"mem"`) renders per mode (raw/human/both); a percent-based limit's `%` suffix and value are unaffected by mode; a non-byte-valued limit (e.g. `"slots"`) is unaffected by mode (`test_limit.c` or a new sibling — task-level decision).
|
||||
- `info limit <name>` end-to-end: a `"mem"`-resourced limit's configured-value line and its `accounted=`/`capacity=` usage line(s) both show human-formatted amounts under the default units mode, and both switch consistently when the operator changes units mode (Requirement 6.1, 6.2, 6.5) (`test_repl_limit_table.c`).
|
||||
- A percent-based limit on a byte-valued resource (e.g. `night_mem_pct` from the transcript that motivated this requirement) shows its resolved absolute amount human-formatted (Requirement 6.4) (`test_repl_limit_table.c`).
|
||||
- A slot-count limit's configured value and usage amounts are unaffected by units mode (Requirement 6.3) (`test_repl_limit_table.c`).
|
||||
- **Required RED→GREEN treatment for the resource-name bug fix**: before changing `repl_print_limit_usage()`'s argument, capture that `test_repl_limit_table.c`'s existing `"mem"`-resourced fixtures' `accounted=X.00`-style assertions currently pass only because the classifier never actually received `"mem"` (the bug); after the fix, update those specific assertions to expect the corrected human-formatted output, and confirm via a temporary revert that the OLD assertion now fails for the right reason (proving the fix, not a coincidental pass) before finalizing the new assertion.
|
||||
|
||||
## Constraints
|
||||
|
||||
- **Project Invariant Cross-Check** (`scheduler-driver-equivalence.md`): this design touches none of `resource.c`, `core_events.c`, `driver.c`, `sim_loop.c`, or `core_session.c` — the files that invariant's testing discipline gates — and is read-only by Requirement 5.1/5.2, so it cannot introduce normal-mode-vs-sim-mode accounting divergence. Checked, not applicable beyond the read-only guarantee already covered by the Integration Tests above.
|
||||
- **`ha-async-io.md`**: not applicable — this code runs entirely inside the standalone simulation REPL process, never on the controller's iomgr/delaymgr/vtask threads.
|
||||
- **`resource-accounting.md`** (reservation-vs-actual-usage reconciliation for real resources like `mem`): not applicable, checked by flag rather than by analogy — that document scopes itself explicitly to resources flagged both `XCE_RESF_DYNAMIC` and `XCE_RESF_CONSUMABLE`. A limit's synthetic capacity resource (`vres->vres`, `add_vres()` in `xcelimit.c`) is flagged `XCE_RESF_LIMIT | XCE_RESF_CONSUMABLE | XCE_RESF_RELEASE` plus `XCE_RESF_GLOBAL|XCE_RESF_SHARED` or `XCE_RESF_DEFAULT_HOST` — never `XCE_RESF_DYNAMIC`. Its capacity/in-use pair is an exact scheduler-computed accounting number (charged/released by the same dispatch/completion path as any consumable resource), not an estimate reconciled against independently-drifting OS-measured usage the way `mem` is — so this design's plain read of those two values has no reconciliation-window or staleness concern to account for.
|
||||
- **Requirement 6's shared-library change** (`fmt_limit()`/`utils.c`): checked for blast radius beyond this spec, since `libxce` is linked by controller, worker, scheduler, and (via CGO) the apiserver (`src/lib/CLAUDE.md`). Confirmed via full-tree grep that `fmt_limit()` has exactly one production caller (`repl_info.c`, this spec's own command) and one direct-unit-test file (`test_limit.c`); `utils.c`/`utils.h` gain new symbols only (no existing symbol's signature or behavior changes), so no other `libxce` consumer is affected. This is display-only, per this design's own Non-Goals — no wire format, matching arithmetic, or persisted value is touched, so `persistence.md`'s and `scheduler-driver-equivalence.md`'s invariants are not implicated (the same reasoning the original Requirement Set 1 design already applied above, extended to the new files).
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ Operators investigating scheduler behavior in the `--interactive` simulation REP
|
|||
## Boundary Context
|
||||
- **In scope**: REPL-visible display of a limit's configured setting (the resource it constrains, its configured value or percentage, and which scope dimensions it effectively varies by, including any host-scoping implied by a percentage value on a resource that is not shared cluster-wide) and its live accounted usage (per distinct scope instance, including explicit exclusion-based scope restrictions), in simulation mode only.
|
||||
- **Out of scope**: any command that mutates scheduler state; live, controller-connected (non-simulation) inspection; explaining *why* a specific job did or did not trigger a limit (decision-reasoning trace — expected to be added later as a separate requirement of this same open-ended spec, not covered here); any change to scheduling, allocation, or limit-matching behavior.
|
||||
- **Adjacent expectations**: this capability extends the REPL's existing `info` command family (shipped by `scheduler-core`, implementation-complete) without altering any existing subcommand's behavior or output; it must not perturb the guarantee that a simulation replay reproduces identical scheduling decisions and identical accounting state regardless of what inspection commands are issued during it.
|
||||
- **Adjacent expectations**: this capability extends the REPL's existing `info` command family (shipped by `scheduler-core`, implementation-complete) without altering any existing subcommand's behavior or output; it must not perturb the guarantee that a simulation replay reproduces identical scheduling decisions and identical accounting state regardless of what inspection commands are issued during it. Requirement 6 (added 2026-09-19, following operator feedback that a limit's raw byte amounts are hard to read) relies on the REPL's existing byte-unit display convention already used by other `info` subcommands for byte-valued resource amounts, including an operator's existing ability to select how those amounts are displayed — it extends that existing convention to limit displays rather than introducing a new one.
|
||||
|
||||
## Requirements
|
||||
|
||||
|
|
@ -53,3 +53,13 @@ Operators investigating scheduler behavior in the `--interactive` simulation REP
|
|||
1. The REPL shall not alter any limit's configuration, usage accounting, or any scheduling decision as a result of displaying limit information.
|
||||
2. The REPL shall produce identical scheduling decisions and identical final accounting state for a replay whether or not limit information was requested during that replay.
|
||||
3. The REPL shall preserve all existing `info` subcommands' behavior and output unchanged.
|
||||
|
||||
### Requirement 6: Human-Readable Units for Byte-Valued Limit Amounts
|
||||
**Objective:** As an operator inspecting a limit whose constrained resource is measured in bytes (for example memory), I want large byte amounts displayed in a human-readable scale rather than as a raw byte count, so that I can read and compare limit values and usage at a glance instead of counting digits.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When the REPL displays a limit's configured amount or a limit's live accounted or capacity amount for a resource whose canonical unit is bytes, the REPL shall present that amount using the same human-readable unit scaling (kilobytes, megabytes, gigabytes, or terabytes as appropriate) that it already uses when displaying other byte-valued resource amounts.
|
||||
2. Where an operator has selected how byte-valued resource amounts are displayed elsewhere in the REPL, the REPL shall present a limit's byte-valued amounts consistent with that same selection.
|
||||
3. The REPL shall not apply byte-unit scaling to a limit's amount when the limit's constrained resource is not byte-valued, including a limit expressed as a percentage of a resource whose own canonical unit is not bytes.
|
||||
4. Where a limit's configured value is a percentage of a resource whose canonical unit is bytes, the REPL shall display the resolved absolute amount described in Requirement 1's Acceptance Criterion 4 using the same human-readable unit scaling as a non-percentage byte-valued limit.
|
||||
5. The REPL shall apply this unit scaling consistently to both a limit's configured setting display and its live accounted usage display, so an operator sees the same units convention throughout a single request for limit information.
|
||||
|
|
|
|||
|
|
@ -85,6 +85,44 @@
|
|||
- `resource-accounting.md`'s reservation-vs-actual-usage reconciliation is explicitly scoped to resources flagged both `XCE_RESF_DYNAMIC` and `XCE_RESF_CONSUMABLE`. A limit's synthetic capacity resource (`add_vres()`) is flagged `XCE_RESF_LIMIT | XCE_RESF_CONSUMABLE | XCE_RESF_RELEASE` plus `GLOBAL|SHARED` or `DEFAULT_HOST` — never `XCE_RESF_DYNAMIC` — so that steering document does not apply here; the capacity/in-use pair this design reads is an exact scheduler-computed accounting number, not an OS-reconciled estimate.
|
||||
- `src/scheduler/testsuite/test_no_bare_printf.c` regression-tests every file under `src/scheduler/src/` for bare `printf()` calls (must use `fprintf(stdout, ...)`); bug-1599 documents a known scanner false-positive on *comment prose*, not an exemption for real code.
|
||||
|
||||
## Requirement 6 Research (added 2026-09-19 — Human-Readable Units for Byte-Valued Limit Amounts)
|
||||
|
||||
### Two independent root causes behind one operator-reported symptom
|
||||
- **Context**: A live REPL transcript showed a limit's configured amount (`mem = 5000000000.000000`) and its usage amount (`accounted=9011200.00 capacity=4635344384.00`) both as raw byte counts, despite the REPL already having a general byte-formatting convention used by `info worker`/`info resource`.
|
||||
- **Sources Consulted**: `src/scheduler/src/repl_info.c` (`repl_print_limit_usage()`, `repl_print_limit_usage_per_host()`, `repl_format_consumable_value_and_used()`, `repl_format_numeric_resource_value()`, `repl_current_units_mode()`), `src/scheduler/src/repl_fmt.c` (`repl_is_bytes_valued_resource()`, `repl_format_bytes_human()`), `src/lib/xcelimit.c` (`fmt_limit()`), `src/lib/testsuite/test_limit.c`.
|
||||
- **Findings**:
|
||||
- The usage lines (`accounted=`/`capacity=`) already call `repl_format_consumable_value_and_used()`, which already checks `repl_is_bytes_valued_resource()` — but the argument passed is `vres->vres->name`, a synthesized compound name (`build_vres_name()`, e.g. `limits2/u=rshen/q=xce/p=`), never the literal resource name (`"mem"`). The classifier silently always returns false for a limit's usage instance. The correct string (`limit->val.resname`, or `vres->limit->val.resname` in the per-host helper) is already reachable in scope — this is a genuine, isolated argument bug, not a missing feature.
|
||||
- The configured-value line is printed entirely by `fmt_limit()` (`src/lib/xcelimit.c`), a shared-library function with a fixed `"%s = %f%s\n"` format and its own direct unit tests (`test_limit.c`, 5 call sites) — it has no REPL units-mode concept at all, and `repl_info.c`'s own existing comment explicitly documents a prior decision NOT to reformat its output.
|
||||
- Full-tree grep confirms `fmt_limit()` has exactly one production caller (`repl_info.c`, this spec's own command) and one direct-test file — no other component (controller, worker, apiserver) calls it, despite `libxce` being linked by all of them.
|
||||
- **Implications**: the usage-line issue is a one-line-per-call-site bug fix. The config-line issue requires a real design decision (discovery phase surfaced 3 approaches; user selected "reformat at the source").
|
||||
|
||||
### Reformat-at-the-source approach: how to add a units-mode parameter without breaking `fmt_limit()`'s contract
|
||||
- **Context**: the user selected reformatting `fmt_limit()`'s output at its source over a REPL-layer annotation, but `fmt_limit()`'s existing signature has no way to receive a display-mode preference, and its 5 existing `test_limit.c` assertions pin its exact current output.
|
||||
- **Alternatives Considered**:
|
||||
1. Add a units-mode parameter directly to `fmt_limit()`'s existing signature — breaks its contract; all 5 `test_limit.c` call sites and both `repl_info.c` call sites need updating, and the meaning of "default" behavior for `fmt_limit()`'s own unit tests becomes ambiguous.
|
||||
2. Add a new sibling function `fmt_limit_units()` taking the extra parameter, and retype `fmt_limit()` itself into a one-line wrapper calling it with a fixed RAW mode.
|
||||
3. Duplicate `fmt_limit()`'s whole body into a new function rather than sharing it via delegation.
|
||||
- **Selected Approach**: (2).
|
||||
- **Rationale**: verified algebraically that `xce_format_units_value(XCE_UNITS_RAW, v, ...)` produces the bare `"%f"` rendering of `v`, identical to what `fmt_limit()` prints today in both its percent and non-percent branches — so `fmt_limit(buf, len, limit) == fmt_limit_units(buf, len, limit, XCE_UNITS_RAW)` for every input, provably byte-identical. This means `test_limit.c`'s 5 existing assertions require **zero changes**, contrary to this spec's own discovery-phase estimate that they would need updating. Delegation (not duplication) avoids the two functions' consumer-dump loops drifting apart later.
|
||||
- **Trade-offs**: two function names to maintain instead of one, but this matches the codebase's own precedent (`fmt_jobid()`/`fmt_jobid_ex()`, `limit_seed_config()`/`limit_seed_config_impl()`) for "add an extended sibling rather than break an existing signature."
|
||||
- **Follow-up**: none.
|
||||
|
||||
### Where the byte-formatter should live once it needs a non-scheduler caller
|
||||
- **Context**: `fmt_limit_units()` lives in `src/lib/xcelimit.c` (shared library) but needs the same byte-scaling algorithm `repl_format_bytes_human()` (`src/scheduler/src/repl_fmt.c`, scheduler-only) already implements.
|
||||
- **Alternatives Considered**:
|
||||
1. Duplicate the algorithm into `xcelimit.c` directly.
|
||||
2. Promote the algorithm (and its companion classifier/composer) into an existing shared-lib file, with the scheduler's existing functions becoming thin wrappers.
|
||||
3. Create a brand-new shared-lib file solely for this.
|
||||
- **Selected Approach**: (2), landing in `src/lib/utils.c`/`src/include/utils.h` — the existing home of `fmt_jobid()`/`fmt_jobid_ex()`, the closest existing precedent for a small, generic, cross-component display-formatting helper with no domain coupling (confirmed no similar helper already exists elsewhere in `src/lib`; `src/controller/testsuite/test.c`'s own `format_bytes_human()` is test-only and not a production dependency, so not consolidated by this spec).
|
||||
- **Rationale**: avoids duplicating the same divide-by-1024-while->=1024 algorithm a third time in the codebase (it already exists twice: `repl_format_bytes_human()` production, `test.c`'s `format_bytes_human()` test-only); `utils.c`/`utils.h` is already linked into every component that would ever need this, with zero new build-system surface.
|
||||
- **Trade-offs**: `xce_repl_units_mode_t` (scheduler-private, `repl_internal.h`) and the new `xce_units_mode_t` (shared, `utils.h`) are two independently-defined enums with the same three semantic values, rather than one shared type — deliberate, since promoting the scheduler's own enum into shared-lib scope would be a layering violation in the other direction (a `src/lib` type must not be defined in terms of a scheduler-internal concept, nor should the scheduler's already-tested, already-working `xce_repl_units_mode_t` plumbing need to change for this). A small `translate_units_mode()` switch at the one new call site (`repl_info.c`) bridges the two; a Revalidation Trigger in `design.md` flags this mapping for a future maintainer.
|
||||
- **Follow-up**: none.
|
||||
|
||||
### Existing test fixtures were asserting on the bug, not a locked contract
|
||||
- **Context**: `test_repl_limit_table.c` (built under Requirement Set 1) has multiple fixtures using `"mem"` as the limit's constrained resource, with assertions like `strstr(captured, "accounted=4.00")`.
|
||||
- **Finding**: these assertions currently pass BECAUSE of the resource-name argument bug (the classifier is never actually asked about `"mem"`), not because `"4.00"` is a deliberately-chosen display contract. Fixing the bug will legitimately change this output to a human-formatted form.
|
||||
- **Implication**: this is expected, not a regression to avoid — `design.md`'s Testing Strategy requires the implementer to prove this via a temporary revert (old assertion fails for the right reason once the bug is fixed) before finalizing the updated assertion, the same RED→GREEN discipline as any other bug fix in this codebase.
|
||||
|
||||
## References
|
||||
- `.wolf/buglog.json` bug-1784–bug-1790 (gitea#46 limit-matching chain, 2026-09-07) — internal, not a URL.
|
||||
- `.wolf/buglog.json` bug-1593 (`info resource` GLOBAL/SHARED-vs-host-based display fix, 2026-08-19) — internal, not a URL.
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
{
|
||||
"feature_name": "scheduler-sim-diagnostics",
|
||||
"created_at": "2026-08-16T11:02:06Z",
|
||||
"updated_at": "2026-09-17T02:30:00Z",
|
||||
"updated_at": "2026-09-20T03:30:00Z",
|
||||
"language": "en",
|
||||
"phase": "tasks-generated",
|
||||
"description": "Open-ended simulation-mode REPL diagnostics spec. First capability: limit inspection (info limit <name|all>) showing configured setting (scope, value/percent) and live accounted usage per instance. Future capability (seed capability 2, not yet in requirements.md): decision-reasoning trace for pend/dispatch outcomes.",
|
||||
"description": "Open-ended simulation-mode REPL diagnostics spec. First capability: limit inspection (info limit <name|all>) showing configured setting (scope, value/percent) and live accounted usage per instance. Requirement 6 (added 2026-09-19): human-readable unit scaling for byte-valued limit amounts, designed via a fmt_limit_units() sibling entry point plus a promoted shared-lib byte-formatter (utils.c), tasks 7-10. Future capability (seed capability 2, not yet in requirements.md): decision-reasoning trace for pend/dispatch outcomes.",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
# Implementation Plan: scheduler-sim-diagnostics (Requirement Set 1 — Limit Inspection)
|
||||
# Implementation Plan: scheduler-sim-diagnostics (Requirement Set 1 — Limit Inspection; Requirement 6 — Human-Readable Units)
|
||||
|
||||
- [x] 1. Add read-only accessors exposing the limit plugin's configured-limits and live-usage-instances tables
|
||||
- Two new functions (`limit_get_configured_limits()`, `limit_get_usage_instances()`) returning raw kvmgr handles (or `NULL` when the plugin is absent), declared in `plugin.h` alongside `register_limit()`, defined in `plugins/limit.c`
|
||||
|
|
@ -66,10 +66,60 @@
|
|||
- _Requirements: 5.1, 5.2, 5.3_
|
||||
- _Depends: 4_
|
||||
|
||||
- [x] 7. Foundation: Promote human-readable byte-unit formatting into the shared library
|
||||
- Move the existing byte-unit scaling algorithm and its byte-valued-resource classification out of the scheduler REPL and into the shared library, so any component linking the shared library can reach it, not only the scheduler's REPL
|
||||
- Add a display-mode-aware value composer to the shared library capturing the existing raw / human-readable / both-together rendering shapes the REPL already uses elsewhere for other byte-valued resources
|
||||
- The scheduler's own existing byte-formatting functions become thin callers of the newly shared versions — their names, signatures, and observable output stay exactly as they are today
|
||||
- Add the small scheduler-side bridge between the REPL's own existing display-mode selection and the shared library's newly-introduced mode concept, and retype the REPL's existing shared numeric-value composer (the one piece of formatting logic every byte-valued resource display in the REPL already funnels through, including — but not limited to — limit display) to delegate to the new shared composer through that bridge, so every consumer of that shared composer benefits from one single implementation from this point forward, not just the limit display this requirement set is adding
|
||||
- Observable: every existing scheduler test that exercises byte-unit formatting continues to pass completely unchanged — including the pre-existing, non-limit displays that already share this exact formatting logic — and new shared-library unit tests directly exercise the promoted scaling, classification, and mode-aware composition logic in isolation
|
||||
- _Requirements: 6.1, 6.3, 6.5, 5.3_
|
||||
- _Boundary: Shared Units Formatter, REPL Limit Info Command (the retyped composer and its thin-wrapper callers live in `repl_info.c`/`repl_fmt.c`)_
|
||||
|
||||
- [ ] 8. Core: Human-readable configured-setting display
|
||||
- [ ] 8.1 Add a display-mode-aware way to render a limit's configured setting
|
||||
- New entry point produces the same configured-setting output the existing formatter already produces, except a byte-valued, non-percentage resource's amount renders in human-readable form according to a supplied display mode
|
||||
- The existing formatter's own output is provably unchanged for every input it can be given — proven by its complete pre-existing test coverage continuing to pass with zero modification
|
||||
- Observable: the existing configured-setting formatter's test coverage passes unmodified, and a new test confirms the mode-aware variant renders a byte-valued resource's amount differently across each supported display mode while leaving a percentage or non-byte-valued resource's rendering untouched
|
||||
- _Requirements: 6.1, 6.3, 6.5_
|
||||
- _Depends: 7_
|
||||
- _Boundary: Shared Units Formatter_
|
||||
|
||||
- [ ] 8.2 Show configured limit amounts in the operator's selected display mode
|
||||
- The REPL's limit configuration display now renders a byte-valued limit's configured amount using the operator's already-selected display mode, reusing the scheduler-side bridge task 7 introduced, the same mode every other byte-valued resource display in the REPL already honors
|
||||
- Observable: `info limit <name>` against a byte-valued limit shows its configured amount in human-readable form under the REPL's default display mode, and the rendering changes consistently when the operator switches display modes
|
||||
- _Requirements: 6.1, 6.2, 6.5_
|
||||
- _Depends: 8.1_
|
||||
- _Boundary: REPL Limit Info Command_
|
||||
|
||||
- [ ] 9. Core: Correct live usage display's resource classification
|
||||
- The live usage display currently classifies a limit's amounts using an internal per-instance identifier rather than the limit's own constrained resource, so a byte-valued limit's usage amounts never actually get evaluated for human-readable rendering even though the surrounding display logic (already retyped onto the shared composer by task 7) already supports it
|
||||
- Classify a limit's live usage amounts by the limit's own constrained resource instead, for both the cluster-wide and the per-host usage display paths
|
||||
- Observable: a byte-valued limit's accounted and capacity amounts in its live usage display render in human-readable form, matching the same format its configured-setting display already uses; a limit expressed as a percentage of a byte-valued resource shows its resolved absolute amount in human-readable form too
|
||||
- _Requirements: 6.1, 6.3, 6.4, 6.5_
|
||||
- _Depends: 7_ — task 7 retypes the exact shared formatting call chain this task's fix flows through (the limit usage display's classification feeds into that same composer); sequencing after 7 avoids two tasks editing overlapping logic in an uncoordinated order, not a data dependency on any new symbol this task itself needs
|
||||
- _Boundary: REPL Limit Info Command_
|
||||
|
||||
- [ ] 10. Validation: Confirm correctness, consistency, and no regression
|
||||
- [ ] 10.1 Re-verify existing test coverage against the corrected usage display
|
||||
- Existing test coverage for the live usage display was passing only because the resource-classification defect prevented it from ever being evaluated correctly — identify every such assertion, and confirm each one would now fail against the pre-fix behavior before replacing it with an assertion on the corrected, human-readable output
|
||||
- Observable: each updated assertion demonstrably fails when checked against the prior (defective) behavior and passes against the corrected behavior — not merely a new assertion added alongside an untouched old one
|
||||
- _Requirements: 6.1, 6.3, 6.4_
|
||||
- _Depends: 9_
|
||||
|
||||
- [ ] 10.2 Confirm units consistency, exclusion rules, and zero regression end to end
|
||||
- A byte-valued limit's configured-setting display and its live usage display agree on units under every display mode, within one request for that limit's information
|
||||
- A limit whose constrained resource is not byte-valued, or whose own configured value is itself a percentage, is never subject to byte-unit scaling in either display
|
||||
- The complete pre-existing `info limit` test suite, and the complete pre-existing suite for every other `info` subcommand — including the two pre-existing, non-limit displays that share the same underlying formatting logic this requirement set retyped in task 7 — show zero output differences beyond what this requirement set intends
|
||||
- Observable: a single `info limit` invocation against a byte-valued limit shows the same units convention in its configured-setting line and its usage line(s) in every display mode, a slot-count limit's display is unaffected by display mode in either line, and the full pre-existing scheduler test suite — including the pre-existing non-limit byte-valued-resource displays task 7 touched — passes with no unintended output changes
|
||||
- _Requirements: 6.1, 6.2, 6.3, 6.4, 6.5, 5.1, 5.2, 5.3_
|
||||
- _Depends: 8.2, 9, 10.1_
|
||||
|
||||
## Parallel Execution Note
|
||||
|
||||
No `(P)` markers are used anywhere in this plan. Tasks 2.1–2.3 and 3.1–3.2 all incrementally edit the same function (`repl_info_limit()`) in the same file (`repl_info.c`) and largely share the same new test file (`test_repl_limit_table.c`) — the parallel-analysis rules' "no conflicting files or shared mutable resources" criterion rules out marking any of them concurrent. Task 1 is the sole Foundation task (rarely `(P)` by rule, and has no peer to run alongside regardless). Tasks 4 and 5 are Integration/Validation work that structurally depends on Core being complete.
|
||||
|
||||
Tasks 7–10 (Requirement 6) follow the same reasoning: task 9 has no genuine data dependency on task 8 (they touch different functions — the configured-setting formatter vs. the live-usage formatter), but both edit `repl_info.c` and both build on task 7's shared-library symbols and scheduler-side bridge, and task 10's validation work spans all of them — marking any of 8/9 `(P)` would only invite the same same-file coordination risk this plan has already declined once. Task 7 is the sole new Foundation task, and now also owns retyping the REPL's pre-existing shared numeric-value composer (used by `info worker`/`info resource` as well as `info limit`) onto the new shared formatter — a task-graph sanity review (2026-09-20) caught this as a design.md-specified change with no owning task in an earlier draft, corrected here rather than left implicit. Task 10 is Validation work that structurally depends on Core being complete.
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
- **Task 1**: `test_limit_plugin.c`'s pre-existing `fixture_init()` never populated `core->plugins` (`plugin_register()` is stubbed to a no-op in that white-box TU). Closed with two new test-local fixture helpers that hand-build a real `core->plugins` kvmgr and `key_add()` the plugin under its real `"xce_sched_limit"` name — necessary to exercise the accessors' real `key_find()` path, not a workaround. `test_repl_limit_table.c` (tasks 2.1+) is a *different* white-box TU with its own include set (per design.md's build-wiring risk note) — it should link real `plugin.c`/`core.c` machinery where `register_limit()`/`xce_sched_core_init()` actually populate `core->plugins`, so it likely does NOT need this same hand-built-kvmgr workaround; confirm this when starting 2.1 rather than assuming the same gap exists there.
|
||||
|
|
@ -78,3 +128,4 @@ No `(P)` markers are used anywhere in this plan. Tasks 2.1–2.3 and 3.1–3.2 a
|
|||
- **Task 3.2**: reviewer approved with one non-blocking suggestion — no test directly exercises Requirement 3.1's "per-user and per-host together" combined-dimension case end-to-end (structurally supported by the code, per independent review inspection, just not covered by a dedicated fixture). Worth adding if this area gets touched again; not required before shipping.
|
||||
- **Task 5 — significant follow-up needed (logged as `bug-1825`)**: all 5 tasks are correctly implemented, thoroughly tested, and genuinely read-only/deterministic — but `info limit` is currently **inert in real production usage**. `register_limit()` has exactly one call site in the whole codebase (`scheduler.c`'s normal-mode `sched_register_plugins()`); sim mode's own plugin bootstrap (`sim_bootstrap_picker()`, `sim_picker.c`) only registers the picker plugin, never the limit plugin. So a real operator running `xce_scheduler -i`/`-s <replay>` will always see "no limits configured"/"no such limit" from `info limit`, regardless of what the original live recording had configured — every positive-display test in this spec exercises the code via a test-only plugin-registration helper with no production equivalent in sim mode. This is a pre-existing sim-mode driver gap (same class as `scheduler-driver-equivalence.md`'s other documented normal-vs-sim parity gaps), not a defect in this spec's own work, and was out of task 5's test-only boundary to fix. Fixed by task 6.
|
||||
- **Task 6 — a real memory-safety lesson, caught by review before it shipped**: the first implementation attempt set `plugin->sched = (void*)sim` (a `xce_sched_sim_driver_t*`, disguised) before invoking the limit plugin's `config_hook`, relying on both `xce_scheduler_t` and `xce_sched_sim_driver_t` happening to have `xce_sched_core_t core` as their first struct member. An independent reviewer rejected this: `core.h`'s own `xce_core_cache_clear_cb_t` doc comment documents that this EXACT cross-cast pattern already corrupted memory once before in this codebase (`cache_clear()` reinterpreting `xce_sched_sim_driver_t`'s `alt_events_file` as a `kvtab_t*`), and there was no compiler/runtime guard against a future edit to the callee reading past `.core`. Fixed by adding `limit_seed_config(plugin, xce_sched_core_t* core, config)` — an honestly-typed entry point taking `core` as a real field access, no cast — with `limit_update_config()` (normal mode) and `sim_bootstrap_limit_plugin()` (sim mode) both delegating to a shared `_impl` function. **Lesson for future work in this codebase**: when a plugin hook or helper is invoked from BOTH a normal-mode (`xce_scheduler_t`) and a sim-mode (`xce_sched_sim_driver_t`) context, never derive one struct type from the other via a first-member cast, even when it "happens to work" today — take `xce_sched_core_t*` as an honest parameter instead. Two independent reviewers (one per round) verified this by reading the actual struct definitions and the full call chain, not by trusting the report.
|
||||
- **Task 7 — a genuine, narrow behavioral corner case surfaced by the promotion, deliberately left as-is**: the pre-existing `repl_format_numeric_resource_value()` had an asymmetry for `"swp"` (whose wire unit is megabytes, not bytes): RAW and BOTH modes printed the *un-converted* raw value, while only HUMAN mode used the MB→bytes-converted value for scaling. The new shared `xce_format_units_value(mode, bytes_value, ...)` interface — a single value argument, matching design.md's specified shape exactly — cannot preserve that asymmetry: `swp` under RAW/BOTH mode in `info worker`/`info resource` now renders the bytes-converted number instead of the original raw MB number. Confirmed by both the implementer and an independent reviewer: no test in the codebase exercises `swp` under RAW or BOTH mode (all existing `swp` tests use HUMAN only), and `swp` is not in Requirement 6's byte-valued-resource list (`mem`/`tmp`/`maxmem`/`maxtmp`/`maxswap`), so this is untested and out of this requirement's scope — not a regression caught by any existing coverage, but technically in tension with Requirement 5.3's literal "preserve all existing behavior unchanged." Left as-is rather than special-cased, since fixing it would mean giving the shared composer an `swp`-specific parameter it has no other reason to carry. If `swp`'s RAW/BOTH display is ever reported as user-visibly wrong, start here.
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
|
||||
#include <capnp_c.h>
|
||||
#include <math.h>
|
||||
#include <stdbool.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <tlog.h>
|
||||
|
|
@ -211,6 +212,51 @@ extern int fmt_jobid(char*, size_t, uint64_t);
|
|||
extern int parse_jobid(uint64_t*, char*);
|
||||
extern uint64_t array_jobid(uint64_t);
|
||||
extern uint64_t array_idx(uint64_t);
|
||||
|
||||
/*
|
||||
* xce_units_mode_t / xce_format_bytes_human() / xce_is_bytes_valued_resource()
|
||||
* / xce_format_units_value() -- scheduler-sim-diagnostics spec, task 7 (Req
|
||||
* 6.1, 6.3, 6.5). Shared-library promotion of the scheduler REPL's
|
||||
* byte-unit scaling algorithm (formerly repl_fmt.c's
|
||||
* repl_format_bytes_human()), its byte-valued-resource classification
|
||||
* (formerly repl_fmt.c's repl_is_bytes_valued_resource()), and a
|
||||
* display-mode-aware value composer (formerly repl_info.c's
|
||||
* repl_format_numeric_resource_value()'s byte-valued branch), so any
|
||||
* `libxce` consumer -- not only the scheduler's REPL -- can reach this
|
||||
* display-only formatting. `fmt_jobid()`/`fmt_jobid_ex()` immediately above
|
||||
* are the precedent this follows: a small, generic, cross-component
|
||||
* display-formatting helper with no domain coupling.
|
||||
*
|
||||
* All three functions are pure and display-only -- none of them ever
|
||||
* mutate a caller-supplied value or feed back into a stored/wire value.
|
||||
* `xce_is_bytes_valued_resource()` is a fixed 5-name list (`mem`, `tmp`,
|
||||
* `maxmem`, `maxtmp`, `maxswap`); a `NULL` name returns `false`.
|
||||
* `xce_format_bytes_human()` adaptively scales `bytes` to the smallest
|
||||
* B/KB/MB/GB/TB/PB unit it fits, writing e.g. "20.56 GB" into `buf`.
|
||||
* `xce_format_units_value()` assumes the caller has already decided
|
||||
* `bytes_value` is byte-valued (via `xce_is_bytes_valued_resource()` or the
|
||||
* caller's own equivalent check) -- it does not re-derive that decision:
|
||||
* RAW renders the bare "%f" of `bytes_value` alone, HUMAN renders
|
||||
* `xce_format_bytes_human()`'s scaled form alone, and BOTH (the default for
|
||||
* any other mode value) renders "<raw> (<human>)".
|
||||
*
|
||||
* `xce_repl_units_mode_t` (scheduler-private, `repl_internal.h`) is a
|
||||
* deliberately separate, independently-defined enum with the same three
|
||||
* semantic values -- a `src/lib` type must not be defined in terms of a
|
||||
* scheduler-internal concept. The scheduler bridges the two at its one call
|
||||
* site (`repl_info.c`'s `translate_units_mode()`).
|
||||
*/
|
||||
typedef enum {
|
||||
XCE_UNITS_RAW = 1,
|
||||
XCE_UNITS_HUMAN,
|
||||
XCE_UNITS_BOTH
|
||||
} xce_units_mode_t;
|
||||
|
||||
extern void xce_format_bytes_human(double bytes, char* buf, size_t bufsz);
|
||||
extern bool xce_is_bytes_valued_resource(const char* name);
|
||||
extern void xce_format_units_value(xce_units_mode_t mode, double bytes_value,
|
||||
char* buf, size_t bufsz);
|
||||
|
||||
extern void unblock_sig(int);
|
||||
extern void block_sig(int);
|
||||
|
||||
|
|
|
|||
|
|
@ -278,6 +278,154 @@ void test_compose_version_ex_matches_original_when_fingerprint_absent(void)
|
|||
TEST_ASSERT_EQUAL_STRING(buf_orig, buf_ex);
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------------
|
||||
* scheduler-sim-diagnostics spec, task 7 (Requirement 6.1, 6.3, 6.5):
|
||||
* xce_format_bytes_human() / xce_is_bytes_valued_resource() /
|
||||
* xce_format_units_value() -- shared-library promotion of the scheduler
|
||||
* REPL's byte-unit scaling algorithm, its byte-valued-resource
|
||||
* classification, and a display-mode-aware value composer (formerly
|
||||
* repl_fmt.c's repl_format_bytes_human()/repl_is_bytes_valued_resource()
|
||||
* and repl_info.c's repl_format_numeric_resource_value()'s byte-valued
|
||||
* branch). Mirrors test_repl_resource_formatting.c's own assertion style --
|
||||
* that file's tests are the scheduler-side regression proof that
|
||||
* delegating repl_fmt.c's two wrapper functions here left their observable
|
||||
* behavior unchanged; these are the direct unit tests for the promoted
|
||||
* primitives themselves.
|
||||
* ------------------------------------------------------------------------ */
|
||||
|
||||
static void test_xce_is_bytes_valued_resource_true_for_canonical_five(void)
|
||||
{
|
||||
TEST_ASSERT_TRUE_MESSAGE(xce_is_bytes_valued_resource("mem"),
|
||||
"mem's wire unit is bytes");
|
||||
TEST_ASSERT_TRUE_MESSAGE(xce_is_bytes_valued_resource("tmp"),
|
||||
"tmp's wire unit is bytes");
|
||||
TEST_ASSERT_TRUE_MESSAGE(xce_is_bytes_valued_resource("maxmem"),
|
||||
"maxmem's wire unit is bytes");
|
||||
TEST_ASSERT_TRUE_MESSAGE(xce_is_bytes_valued_resource("maxtmp"),
|
||||
"maxtmp's wire unit is bytes");
|
||||
TEST_ASSERT_TRUE_MESSAGE(xce_is_bytes_valued_resource("maxswap"),
|
||||
"maxswap's wire unit is bytes");
|
||||
}
|
||||
|
||||
static void
|
||||
test_xce_is_bytes_valued_resource_false_for_null_empty_and_unknown(void)
|
||||
{
|
||||
TEST_ASSERT_FALSE_MESSAGE(
|
||||
xce_is_bytes_valued_resource(NULL),
|
||||
"a NULL name must not be misclassified as bytes-valued");
|
||||
TEST_ASSERT_FALSE_MESSAGE(
|
||||
xce_is_bytes_valued_resource(""),
|
||||
"an empty name must not be misclassified as bytes-valued");
|
||||
TEST_ASSERT_FALSE_MESSAGE(
|
||||
xce_is_bytes_valued_resource("slots"),
|
||||
"an unknown/custom resource name must not be misclassified as "
|
||||
"bytes-valued -- this is a small, explicit, hardcoded list, not a "
|
||||
"heuristic");
|
||||
}
|
||||
|
||||
static void
|
||||
test_xce_format_bytes_human_scales_across_full_unit_range_at_boundaries(void)
|
||||
{
|
||||
char buf[64];
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(0.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("0.00 B", buf);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1023.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||
"1023.00 B", buf, "just below the KB boundary must stay in B");
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1024.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||
"1.00 KB", buf,
|
||||
"exactly 1024 bytes must render as '1.00 KB', not '1024.00 B'");
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1024.0 * 1024.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("1.00 MB", buf);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1024.0 * 1024.0 * 1024.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("1.00 GB", buf);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1024.0 * 1024.0 * 1024.0 * 1024.0, buf,
|
||||
sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("1.00 TB", buf);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(1024.0 * 1024.0 * 1024.0 * 1024.0 * 1024.0, buf,
|
||||
sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("1.00 PB", buf);
|
||||
|
||||
/* PB is the largest unit -- a value past it stays capped there instead
|
||||
* of inventing a further unit. */
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(
|
||||
1024.0 * 1024.0 * 1024.0 * 1024.0 * 1024.0 * 1024.0, buf,
|
||||
sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||
"1024.00 PB", buf,
|
||||
"a value past the PB range must stay capped at PB, not invent a "
|
||||
"further unit");
|
||||
}
|
||||
|
||||
static void
|
||||
test_xce_format_bytes_human_fractional_values_render_two_decimals(void)
|
||||
{
|
||||
char buf[64];
|
||||
|
||||
/* 5,000,000,000 bytes / 1024^3 = 4.656612873077393... GB, rounds via
|
||||
* "%.2f" to "4.66 GB" -- an arbitrary, non-power-of-2 value, the exact
|
||||
* transcript figure that motivated Requirement 6. */
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_bytes_human(5000000000.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING("4.66 GB", buf);
|
||||
}
|
||||
|
||||
static void test_xce_format_units_value_raw_mode_renders_bare_value(void)
|
||||
{
|
||||
char buf[64];
|
||||
char expected[64];
|
||||
|
||||
snprintf(expected, sizeof(expected), "%f", 5000000000.0);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_units_value(XCE_UNITS_RAW, 5000000000.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING(expected, buf);
|
||||
}
|
||||
|
||||
static void
|
||||
test_xce_format_units_value_human_mode_renders_scaled_string_alone(void)
|
||||
{
|
||||
char buf[64];
|
||||
char expected[64];
|
||||
|
||||
xce_format_bytes_human(5000000000.0, expected, sizeof(expected));
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_units_value(XCE_UNITS_HUMAN, 5000000000.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING(expected, buf);
|
||||
}
|
||||
|
||||
static void
|
||||
test_xce_format_units_value_both_mode_renders_raw_and_human(void)
|
||||
{
|
||||
char buf[128];
|
||||
char expected[128];
|
||||
char human[64];
|
||||
|
||||
xce_format_bytes_human(5000000000.0, human, sizeof(human));
|
||||
snprintf(expected, sizeof(expected), "%f (%s)", 5000000000.0, human);
|
||||
|
||||
memset(buf, 0, sizeof(buf));
|
||||
xce_format_units_value(XCE_UNITS_BOTH, 5000000000.0, buf, sizeof(buf));
|
||||
TEST_ASSERT_EQUAL_STRING(expected, buf);
|
||||
}
|
||||
|
||||
void test_utils(void)
|
||||
{
|
||||
printf("\n>> utils test\n\n");
|
||||
|
|
@ -298,4 +446,13 @@ void test_utils(void)
|
|||
RUN_TEST(test_compose_version_ex_empty_fingerprint_dirty_commit_unchanged);
|
||||
RUN_TEST(test_compose_version_ex_null_fingerprint_matches_empty);
|
||||
RUN_TEST(test_compose_version_ex_matches_original_when_fingerprint_absent);
|
||||
RUN_TEST(test_xce_is_bytes_valued_resource_true_for_canonical_five);
|
||||
RUN_TEST(
|
||||
test_xce_is_bytes_valued_resource_false_for_null_empty_and_unknown);
|
||||
RUN_TEST(
|
||||
test_xce_format_bytes_human_scales_across_full_unit_range_at_boundaries);
|
||||
RUN_TEST(test_xce_format_bytes_human_fractional_values_render_two_decimals);
|
||||
RUN_TEST(test_xce_format_units_value_raw_mode_renders_bare_value);
|
||||
RUN_TEST(test_xce_format_units_value_human_mode_renders_scaled_string_alone);
|
||||
RUN_TEST(test_xce_format_units_value_both_mode_renders_raw_and_human);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -703,6 +703,77 @@ uint64_t array_idx(uint64_t jobid)
|
|||
return EXTRACT_ARRAY_IDX(jobid);
|
||||
}
|
||||
|
||||
/*
|
||||
* xce_format_bytes_human / xce_is_bytes_valued_resource / xce_format_units_value
|
||||
* (scheduler-sim-diagnostics spec, task 7, Requirement 6.1/6.3/6.5) --
|
||||
* shared-library promotion of the scheduler REPL's byte-unit scaling
|
||||
* algorithm and byte-valued-resource classification (formerly repl_fmt.c's
|
||||
* repl_format_bytes_human()/repl_is_bytes_valued_resource(), now thin
|
||||
* wrappers around these), plus a display-mode-aware value composer
|
||||
* promoted from repl_info.c's repl_format_numeric_resource_value()'s
|
||||
* byte-valued branch (now delegated to via that file's own
|
||||
* translate_units_mode() bridge). See utils.h's doc comment on
|
||||
* xce_units_mode_t for the full contract; this is a pure, display-only,
|
||||
* leaf utility -- none of these functions ever mutate a caller-supplied
|
||||
* value.
|
||||
*/
|
||||
void xce_format_bytes_human(double bytes, char* buf, size_t bufsz)
|
||||
{
|
||||
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, bufsz, "%.2f %s", val, UNITS[i]);
|
||||
}
|
||||
|
||||
bool xce_is_bytes_valued_resource(const char* name)
|
||||
{
|
||||
static const char* const BYTES_VALUED_RESOURCES[]
|
||||
= { "mem", "tmp", "maxmem", "maxtmp", "maxswap" };
|
||||
const int nres = (int)(sizeof(BYTES_VALUED_RESOURCES)
|
||||
/ sizeof(BYTES_VALUED_RESOURCES[0]));
|
||||
int i;
|
||||
|
||||
if (name == NULL) {
|
||||
return false;
|
||||
}
|
||||
|
||||
for (i = 0; i < nres; i++) {
|
||||
if (strcmp(name, BYTES_VALUED_RESOURCES[i]) == 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void xce_format_units_value(xce_units_mode_t mode, double bytes_value,
|
||||
char* buf, size_t bufsz)
|
||||
{
|
||||
char human[64];
|
||||
|
||||
xce_format_bytes_human(bytes_value, human, sizeof(human));
|
||||
|
||||
switch (mode) {
|
||||
case XCE_UNITS_RAW:
|
||||
snprintf(buf, bufsz, "%f", bytes_value);
|
||||
break;
|
||||
case XCE_UNITS_HUMAN:
|
||||
snprintf(buf, bufsz, "%s", human);
|
||||
break;
|
||||
case XCE_UNITS_BOTH:
|
||||
default:
|
||||
snprintf(buf, bufsz, "%f (%s)", bytes_value, human);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
void unblock_sig(int signo)
|
||||
{
|
||||
sigset_t mask;
|
||||
|
|
|
|||
|
|
@ -72,69 +72,26 @@ void repl_format_ts_human(uint64_t ns, char* buf, size_t bufsz)
|
|||
}
|
||||
|
||||
/*
|
||||
* repl_is_bytes_valued_resource - scheduler-sim-repl-verbosity spec, task
|
||||
* 7.2, design.md's "Resource Value Formatter" component (Req 6.1). Mirrors
|
||||
* src/controller/testsuite/test.c's own is_bytes_valued_resource() exactly
|
||||
* -- same 5-name list, same "small, explicit, client-side list rather than
|
||||
* a wire field" rationale that file's own doc comment gives: mem/tmp's
|
||||
* canonical wire unit is bytes (node_information.c's node_info_get_mem()/
|
||||
* node_info_get_tmp() doc comments); maxmem/maxtmp/maxswap's canonical wire
|
||||
* unit is bytes too (xcecd.c's init_host_attrs() bug-1554 comment). `swp`
|
||||
* is deliberately NOT in this list -- its own wire unit is megabytes, not
|
||||
* bytes (node_info_get_swap() doc comment); see repl_format_bytes_human()'s
|
||||
* own doc comment below and repl_params.c's `set units` caller-side
|
||||
* MB->bytes conversion.
|
||||
* repl_is_bytes_valued_resource / repl_format_bytes_human -
|
||||
* scheduler-sim-repl-verbosity spec, task 7.2, design.md's "Resource Value
|
||||
* Formatter" component (Req 6.1/6.2). scheduler-sim-diagnostics spec, task
|
||||
* 7 (Req 6.1, 6.3, 6.5) promoted both functions' bodies verbatim into the
|
||||
* shared library (`xce_is_bytes_valued_resource()`/`xce_format_bytes_human()`,
|
||||
* `utils.h`/`utils.c`), so any `libxce` consumer -- not only the
|
||||
* scheduler's REPL -- can reach this display-only formatting. These two
|
||||
* remain in place as thin, behavior-preserving wrappers: every existing
|
||||
* call site in this file family keeps calling them by these exact names/
|
||||
* signatures, unchanged. See utils.h's doc comment on xce_units_mode_t for
|
||||
* the full algorithm/classification-list contract now owned there.
|
||||
*/
|
||||
bool repl_is_bytes_valued_resource(const char* name)
|
||||
{
|
||||
static const char* const BYTES_VALUED_RESOURCES[]
|
||||
= { "mem", "tmp", "maxmem", "maxtmp", "maxswap" };
|
||||
const int nres = (int)(sizeof(BYTES_VALUED_RESOURCES)
|
||||
/ sizeof(BYTES_VALUED_RESOURCES[0]));
|
||||
int i;
|
||||
|
||||
if (name == NULL) {
|
||||
return false;
|
||||
}
|
||||
|
||||
for (i = 0; i < nres; i++) {
|
||||
if (strcmp(name, BYTES_VALUED_RESOURCES[i]) == 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
return xce_is_bytes_valued_resource(name);
|
||||
}
|
||||
|
||||
/*
|
||||
* repl_format_bytes_human - scheduler-sim-repl-verbosity spec, task 7.2,
|
||||
* design.md's "Resource Value Formatter" component (Req 6.2). Adaptively
|
||||
* scales `bytes` to the smallest B/KB/MB/GB/TB/PB unit it fits (dividing by
|
||||
* 1024 while >= 1024.0, capping at PB once no larger unit remains), writing
|
||||
* e.g. "20.56 GB" into `buf` (bufsz-bounded) -- mirrors
|
||||
* src/controller/testsuite/test.c's own format_bytes_human() algorithm
|
||||
* exactly, same "%.2f %s" render format. Display-only -- never touches the
|
||||
* underlying stored/wire value, same discipline that file's own doc comment
|
||||
* states ("the stored/wire value stays raw bytes always, this never feeds
|
||||
* back into anything"). `swp`'s wire value is megabytes, not bytes
|
||||
* (node_info_get_swap() doc comment, repl_is_bytes_valued_resource() above)
|
||||
* -- callers convert MB->bytes (x 1024.0 x 1024.0) BEFORE calling this, so
|
||||
* it renders on the same adaptive scale a byte-native resource of
|
||||
* equivalent magnitude would (repl_params.c).
|
||||
*/
|
||||
void repl_format_bytes_human(double bytes, char* buf, size_t bufsz)
|
||||
{
|
||||
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, bufsz, "%.2f %s", val, UNITS[i]);
|
||||
xce_format_bytes_human(bytes, buf, bufsz);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
|||
|
|
@ -369,6 +369,32 @@ repl_classify_resource_def_section(const xce_resource_t* r)
|
|||
: XCE_REPL_RES_SECTION_LOAD;
|
||||
}
|
||||
|
||||
/*
|
||||
* translate_units_mode - scheduler-sim-diagnostics spec, task 7 (Req 6.1,
|
||||
* 6.3, 6.5): bridges the REPL's own units-mode selection
|
||||
* (xce_repl_units_mode_t, repl_internal.h) to the shared library's
|
||||
* independently-defined xce_units_mode_t (utils.h) at the one seam that
|
||||
* needs to cross between them. Deliberately NOT a shared enum -- a
|
||||
* src/lib type must not be defined in terms of a scheduler-internal
|
||||
* concept, nor should the scheduler's own already-tested
|
||||
* xce_repl_units_mode_t plumbing need to change for this (design.md's
|
||||
* "Where the byte-formatter should live" decision). If
|
||||
* xce_repl_units_mode_t's three values ever change, this mapping must be
|
||||
* re-checked by hand -- nothing enforces it mechanically.
|
||||
*/
|
||||
static xce_units_mode_t translate_units_mode(xce_repl_units_mode_t mode)
|
||||
{
|
||||
switch (mode) {
|
||||
case XCE_REPL_UNITS_RAW:
|
||||
return XCE_UNITS_RAW;
|
||||
case XCE_REPL_UNITS_HUMAN:
|
||||
return XCE_UNITS_HUMAN;
|
||||
case XCE_REPL_UNITS_BOTH:
|
||||
default:
|
||||
return XCE_UNITS_BOTH;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* repl_format_numeric_resource_value - Requirement 6.1/6.2/6.5: formats the
|
||||
* text that follows "value=" for a non-consumable ("Load Metrics") numeric
|
||||
|
|
@ -383,6 +409,13 @@ repl_classify_resource_def_section(const xce_resource_t* r)
|
|||
* by repl_info_worker_one()'s and repl_info_resource_one()'s identical Load
|
||||
* Metrics rendering so the two commands can never disagree about this
|
||||
* format.
|
||||
*
|
||||
* scheduler-sim-diagnostics spec, task 7 (Req 6.1, 6.3, 6.5): the
|
||||
* byte-valued branch now delegates its raw/human/both composition to the
|
||||
* shared library's xce_format_units_value() via translate_units_mode()
|
||||
* above, instead of switching on `mode` itself -- so this function, its
|
||||
* `info worker`/`info resource` callers, AND the `info limit` command
|
||||
* this spec is adding all funnel through one single implementation.
|
||||
*/
|
||||
static void repl_format_numeric_resource_value(xce_repl_units_mode_t mode,
|
||||
const char* rname,
|
||||
|
|
@ -393,22 +426,9 @@ static void repl_format_numeric_resource_value(xce_repl_units_mode_t mode,
|
|||
|
||||
if (repl_is_bytes_valued_resource(rname) || is_swp) {
|
||||
double bytes_value = is_swp ? (value * 1024.0 * 1024.0) : value;
|
||||
char human[64];
|
||||
|
||||
repl_format_bytes_human(bytes_value, human, sizeof(human));
|
||||
|
||||
switch (mode) {
|
||||
case XCE_REPL_UNITS_RAW:
|
||||
snprintf(buf, bufsz, "%f", value);
|
||||
break;
|
||||
case XCE_REPL_UNITS_HUMAN:
|
||||
snprintf(buf, bufsz, "%s", human);
|
||||
break;
|
||||
case XCE_REPL_UNITS_BOTH:
|
||||
default:
|
||||
snprintf(buf, bufsz, "%f (%s)", value, human);
|
||||
break;
|
||||
}
|
||||
xce_format_units_value(translate_units_mode(mode), bytes_value, buf,
|
||||
bufsz);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue