repl_info_limit() (repl_info.c, not yet wired into repl_cmd_info() -- that's task 4) resolves "all" vs a specific name against limit_get_configured_limits(): a NULL/empty handle on the all path reports "no limits configured" (Req 1.7); a NULL handle or key_find() miss on the named path reports "no such limit '<name>'" (Req 1.3) and never collapses into the all-only message. New white-box test_repl_limit_table.c exercises both paths directly, including a dedicated regression guard for the NULL-scoping rule. Making repl_info.c genuinely call limit_get_configured_limits() broke the link for ~24 other white-box test files that #include repl_info.c without also linking plugins/limit.c -- fixed with the same link-satisfying-stub idiom this file family already uses elsewhere (all return NULL, verified behavior- preserving since none of these fixtures register the limit plugin). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
67 lines
9.2 KiB
Markdown
67 lines
9.2 KiB
Markdown
# Implementation Plan: scheduler-sim-diagnostics (Requirement Set 1 — Limit Inspection)
|
||
|
||
- [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`
|
||
- Never allocate, mutate, or trigger plugin hooks — a pure read of already-existing pointers
|
||
- New/extended unit tests in `test_limit_plugin.c` cover: plugin registered with configured limits (handle returned, iterable), plugin registered with zero limits, plugin not registered (`NULL`)
|
||
- Observable: a test that registers the limit plugin with a known limit and confirms the accessor's returned handle, when iterated via `iter_make()`/`iter_next()`, yields that same limit; a sibling test confirms `NULL` when the plugin was never registered
|
||
- _Requirements: 1.1, 1.2, 1.4, 2.1, 3.1_
|
||
- _Boundary: Limit Plugin Accessor_
|
||
|
||
- [ ] 2. Core: Limit lookup and configured-setting display
|
||
- [x] 2.1 Implement name/all resolution with correct absence messaging
|
||
- New white-box test file `test_repl_limit_table.c` created (sibling of `test_repl_resource_table.c`, own `#include` set covering `plugins/limit.c` and `resource.c` for real — not stubbed — per `design.md`'s build-wiring risk note citing bug-1762's precedent)
|
||
- `repl_info_limit(sim, arg)`: `arg == "all"` iterates `limit_get_configured_limits()`; `NULL`/empty prints "no limits configured" (Requirement 1.7); `arg != "all"` does a `key_find()` by name (not a scan) on the same handle, printing "no such limit '<name>'" (Requirement 1.3) for a `NULL` handle or a miss — a `NULL` handle on this path is never treated as "no limits configured," matching that message's own `all`-only scoping
|
||
- Wired as a direct call for now (`repl_dispatch()`/`repl_cmd_info()` wiring itself deferred to task 4); tests in this task call `repl_info_limit()` directly, not through `repl_dispatch()`
|
||
- Observable: REPL output shows "no such limit 'foo'" for an unconfigured name against a fixture with other limits configured, and "no limits configured" for `all` against a fixture with zero configured limits
|
||
- _Requirements: 1.3, 1.7, 4.1, 4.2_
|
||
- _Depends: 1_
|
||
|
||
- [ ] 2.2 Display configured setting including scope membership
|
||
- Matched limit's name, constrained resource, value/percent, and per-dimension scope print via the existing `fmt_limit()` library formatter into a stack buffer, line-prefixed with the existing `[sim] ` convention; all output via `fprintf(stdout, ...)`, never bare `printf()` (`test_no_bare_printf.c` regression-gates this)
|
||
- Inclusion and exclusion group membership both render (`fmt_group()` already handles this, adopted as-is)
|
||
- Observable: a limit configured with an exclusion group ("all except X") shows both the inclusion and the exclusion in its output; `info limit all` against a fixture with multiple configured limits displays every one's setting
|
||
- _Requirements: 1.1, 1.2, 1.5_
|
||
|
||
- [ ] 2.3 Display effective host-scope for implicitly host-scoped limits
|
||
- After `fmt_limit()`'s output, call `is_host_based_limit(core->resmgr, limit)`; if it returns `> 0` AND the limit's own per-host consumer is not configured, print an explanatory host-scope note
|
||
- Observable: two REPL fixtures — one percent-based limit on a per-host resource without explicit per-host scoping (note appears), one absolute-value or explicitly-global-scoped limit (note does not appear)
|
||
- _Requirements: 1.6_
|
||
|
||
- [ ] 3. Core: Live accounted-usage display
|
||
- [ ] 3.1 Display usage for cluster-wide (non-host-scoped) limit instances
|
||
- Iterate `limit_get_usage_instances()`, filter by `vres->limit == matched_limit`; for a GLOBAL/SHARED `vres`, read capacity from `core.sharedInst[idx]` and accounted usage from `core.sharedInUse[idx]`
|
||
- Existence-of-instance and zero-usage are distinct: zero matching `xce_vres_t` (no capacity entry at all) prints "no active usage instance yet" (Requirement 2.2); a matched `xce_vres_t` whose `sharedInUse[idx]` is `NULL`/absent instead displays a genuine `accounted=0` (it has been evaluated and has a real capacity, just nothing charged against it yet) — existence is gated on the capacity read, never on the in-use read
|
||
- Observable: a global limit that has matched at least one job shows an accounted/capacity pair reflecting current state; a configured-but-never-evaluated limit shows the explicit no-instance message instead; a matched-but-never-charged limit shows `accounted=0` rather than the no-instance message
|
||
- _Requirements: 1.4, 2.1, 2.2, 2.3, 3.3_
|
||
- _Depends: 1_
|
||
|
||
- [ ] 3.2 Display per-host usage for host-scoped limit instances
|
||
- For a host-scoped `vres` (explicit or implicit per 2.3), iterate `core.workers`; include a worker only if it has a populated capacity entry (`w->resvals[idx]` non-`NULL`) — read capacity from `w->resvals[idx]` and accounted usage from `w->resInUse[idx]` (same existence-vs-zero-usage distinction as 3.1, applied per worker); label each instance with its scope values (user/queue/project from the vresname, plus host)
|
||
- A limit varying by more than one scope dimension at once (e.g. per-user and per-host together) shows each distinct combination as its own labeled instance
|
||
- Observable: a limit matched against jobs on two distinct hosts shows two separate per-host usage lines each labeled with its own scope values, and a third configured-but-untouched host shows no line at all
|
||
- _Requirements: 1.4, 2.1, 2.2, 2.3, 3.1, 3.2, 3.4_
|
||
|
||
- [ ] 4. Register `info limit` as a recognized `info` subcommand
|
||
- New `else if (strcmp(tokens[1], "limit") == 0)` branch in `repl_cmd_info()` (`repl_info.c`) routes to `repl_info_limit()`; usage-string literal updated to list `limit` alongside the existing subcommands
|
||
- `test_repl_limit_table.c` gains a full dispatch-round-trip test (via `repl_dispatch()`, not a direct call) for both `info limit <name>` and `info limit all`
|
||
- Observable: `info limit <name>` and `info limit all` both work when typed as real REPL input (through `repl_dispatch()`), and `help`/usage output lists the new subcommand
|
||
- _Requirements: 4.1, 4.2_
|
||
- _Depends: 2.1, 3.1, 3.2_
|
||
|
||
- [ ] 5. Verify no mutation, replay determinism, and existing-subcommand regression
|
||
- Test: displaying limit information never alters configuration, accounting state, or scheduling decisions (assert config/accounting untouched before vs. after)
|
||
- Test: a replay run twice — once with `info limit` issued mid-session, once without — produces identical scheduling decisions and identical final accounted state
|
||
- Full existing `info` subcommand test suite (job/queue/worker/resource/summary) re-run to confirm zero output changes from this addition
|
||
- Observable: the two new determinism/no-mutation tests pass, and the full pre-existing info-command test suite shows zero diffs
|
||
- _Requirements: 5.1, 5.2, 5.3_
|
||
- _Depends: 4_
|
||
|
||
## 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.
|
||
|
||
## 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.
|
||
- **Task 2.1**: Making `repl_info.c` genuinely call `limit_get_configured_limits()` broke the link for ~24 pre-existing white-box test files that `#include "../src/repl_info.c"` without also linking `plugins/limit.c` — this codebase's white-box test convention means any new cross-file call from an included production file ripples to every file that includes the caller but not the callee. Fixed with an identical 5-line link-satisfying stub (`return NULL`) added to each, matching the established "duplicate, don't share" stub idiom already used throughout this file family; verified behavior-preserving since none of those 24 fixtures register the limit plugin (the real function would also return `NULL` there). **Task 3 (which calls `limit_get_usage_instances()`) will very likely hit the identical ripple across an overlapping or identical file set — expect it, and apply the same fix pattern rather than treating it as a surprise.**
|