feat(scheduler-sim-diagnostics): display effective host-scope note (Req 1.6)
repl_info_limit() now prints an explanatory note when a limit is implicitly host-scoped (is_host_based_limit() > 0 due to a percent-based value on a non-shared resource) but its own configuration doesn't show an explicit per-host scope. The real discriminator turned out to be consumers[CONSUMER_ HOST]->per_consumer, not pointer-nullity as design.md's gloss said -- build_consumer() unconditionally allocates a consumer for all four dimensions, so the struct field is never actually NULL in production. Fixed using the same field is_host_based_limit() itself already falls back to; verified via RED-phase reproduction that the literal design.md condition never fires. design.md corrected to document the real condition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
parent
117d01c77e
commit
5876716eee
4 changed files with 295 additions and 6 deletions
|
|
@ -213,7 +213,7 @@ extern var limit_get_usage_instances(xce_sched_core_t* core);
|
|||
- Single entry point `repl_info_limit(xce_sched_sim_driver_t* sim, const char* arg)`, called from `repl_cmd_info()` exactly like `repl_info_resource(sim, tokens[2])`.
|
||||
- `arg == "all"`: iterate `limit_get_configured_limits()`; if `NULL`/empty, print the Requirement 1.7 "no limits configured" message and return; otherwise print each limit via the per-limit path below.
|
||||
- `arg != "all"`: if `limit_get_configured_limits()` returns `NULL` (plugin not registered — a `NULL` handle is never passed to `key_find()`), or `key_find()` on the returned handle finds no match, print the Requirement 1.3 "no such limit `<name>`" message and return (no partial output). Requirement 1.7's "no limits configured" message is reserved for the `all` path specifically — its own acceptance criterion scopes it to "when an operator requests information for all limits" — so a `NULL`-handle result on the named-lookup path reports absence of *that name*, not absence of configuration in general.
|
||||
- Per-limit output: `fmt_limit()` into a stack buffer, printed line-by-line with the existing `[sim] ` prefix convention; then, only if `is_host_based_limit(core->resmgr, limit) > 0` AND the limit's own `consumers[CONSUMER_HOST] == NULL`, print the Requirement 1.6 effective-host-scope note.
|
||||
- Per-limit output: `fmt_limit()` into a stack buffer, printed line-by-line with the existing `[sim] ` prefix convention; then, only if `is_host_based_limit(core->resmgr, limit) > 0` AND the limit's own per-host consumer was not explicitly configured, print the Requirement 1.6 effective-host-scope note. **Implementation note (task 2.3, corrected from an earlier inaccurate gloss here):** `limit->consumers[CONSUMER_HOST]` is never literally `NULL` for a production-built limit — `build_consumer()` (`xcelimit.c`) unconditionally allocates a consumer for all four dimensions, defaulting to the built-in "all" group with `per_consumer = 0` when unconfigured. The real discriminator is `consumers[CONSUMER_HOST]->per_consumer` (the same field `is_host_based_limit()` itself falls back to, and the same one `fmt_consumer()` uses to choose `"HOST ="` vs `"PER_HOST ="`) — check `(host_consumer == NULL) || (host_consumer->per_consumer == 0)`, not pointer-nullity alone.
|
||||
- Live usage: iterate `limit_get_usage_instances()`, filter by `vres->limit == matched_limit` (pointer equality). `build_vres_name()` (existing, `xcelimit.c`) never encodes host in the vresname — so there is exactly one `xce_vres_t` per matched (user, queue, project) combination *regardless of host-scoping*; host breakout is NOT "one `xce_vres_t` per host," it is a per-worker fan-out over that single `xce_vres_t`'s shared resource index. Concretely, for each matching `xce_vres_t`:
|
||||
- If `vres->vres` is `XCE_RESF_GLOBAL`/`XCE_RESF_SHARED`: exactly one displayed instance, reading capacity from `core.sharedInst[vres->vres->idx]` and accounted usage from `core.sharedInUse[vres->vres->idx]` — the exact read `repl_info_resource_one()` already implements for a GLOBAL/SHARED resource (bug-1593).
|
||||
- Otherwise (host-scoped, explicit or implicit per Requirement 1.6): one displayed instance *per worker that has a populated entry at `vres->vres->idx`* — iterate `core.workers`, and for each worker `w`, include it only if `zarray_get(&(w->resvals), vres->vres->idx)` is non-NULL (a worker `put_resvals()` has never written to, e.g. an unevaluated host, must not produce a fabricated zero-usage line, matching Requirement 2.2's no-fabrication rule at per-host granularity). For an included worker, capacity comes from `w->resvals[vres->vres->idx]` and accounted usage from `w->resInUse[vres->vres->idx]` — the sibling capacity/in-use zarray pair `sched_check_host_satisfy_()` (`resource.c`) already reads the identical way for ordinary consumable resources.
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@
|
|||
- 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
|
||||
- [x] 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_
|
||||
|
|
|
|||
|
|
@ -1193,6 +1193,58 @@ static void repl_print_limit_dump(const char* buf)
|
|||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* repl_print_limit_host_scope_note_if_implicit - task 2.3, Requirement 1.6:
|
||||
* print an explanatory note when a limit is effectively host-based purely
|
||||
* because its configured value is a percentage of a resource that is not
|
||||
* shared/global cluster-wide (is_host_based_limit()'s own implicit rule,
|
||||
* src/lib/xcelimit.c), NOT because the limit's own configuration explicitly
|
||||
* turned on per-host scoping. When per-host scoping WAS explicitly
|
||||
* configured, fmt_limit()'s own dump (task 2.2, called just before this by
|
||||
* both of repl_info_limit()'s call sites) already shows a "PER_HOST = [...]"
|
||||
* line for that dimension (fmt_consumer()'s own per_consumer-driven title
|
||||
* choice), so printing this note too would be redundant -- this function is
|
||||
* the REPL-layer discriminator between the two cases design.md's REPL Limit
|
||||
* Info Command Responsibilities describes.
|
||||
*
|
||||
* design.md's own literal condition is "is_host_based_limit(...) > 0 AND
|
||||
* limit->consumers[CONSUMER_HOST] == NULL". consumers[CONSUMER_HOST] is
|
||||
* treated here as "not explicitly configured" both when it is NULL (that
|
||||
* literal condition, defensive for any xce_limit_t not built through
|
||||
* build_limit()/build_consumer()) AND when it is non-NULL with
|
||||
* per_consumer == 0 -- the shape every REAL, production-built xce_limit_t
|
||||
* actually has for a dimension nothing configured: build_consumer()
|
||||
* (xcelimit.c) unconditionally allocates a consumer for all four
|
||||
* dimensions on every build_limit() call, defaulting group to "all" and
|
||||
* per_consumer to 0, rather than leaving the slot NULL. per_consumer is
|
||||
* the one bit build_consumer() derives directly from config->per_host, and
|
||||
* is also exactly what fmt_consumer() itself keys its "HOST"-vs-"PER_HOST"
|
||||
* title on and what is_host_based_limit()'s own explicit-scope fallback
|
||||
* returns -- so it is the correct real-world discriminator for "was
|
||||
* per-host scoping explicitly configured", confirmed empirically: a bare
|
||||
* `== NULL` check never fires for any limit this file's own test fixtures
|
||||
* (or any real limit_update_config() caller) can produce.
|
||||
*/
|
||||
static void repl_print_limit_host_scope_note_if_implicit(
|
||||
xce_sched_core_t* core, xce_limit_t* limit)
|
||||
{
|
||||
xce_consumer_t* host_consumer = limit->consumers[CONSUMER_HOST];
|
||||
|
||||
if (is_host_based_limit(core->resmgr, limit) <= 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if ((host_consumer != NULL) && (host_consumer->per_consumer != 0)) {
|
||||
return;
|
||||
}
|
||||
|
||||
fprintf(stdout,
|
||||
"[sim] note: this limit's value is a percentage of a resource "
|
||||
"that is not shared across the cluster, so usage for it is "
|
||||
"tracked separately per host even though the configuration above "
|
||||
"does not show an explicit host scope\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* repl_info_limit - `info limit <name|all>` name/all resolution (task 2.1)
|
||||
* plus, once a genuine match is found, printing that limit's configured
|
||||
|
|
@ -1249,8 +1301,8 @@ static void repl_info_limit(xce_sched_sim_driver_t* sim, const char* arg)
|
|||
repl_print_limit_dump(limit_buf);
|
||||
}
|
||||
}
|
||||
/* task 2.3 adds the Req 1.6 effective-host-scope note here;
|
||||
* task 3 adds this limit's live usage. */
|
||||
repl_print_limit_host_scope_note_if_implicit(&(sim->core), limit);
|
||||
/* task 3 adds this limit's live usage. */
|
||||
n_shown++;
|
||||
}
|
||||
iter_destroy(&it);
|
||||
|
|
@ -1280,8 +1332,8 @@ static void repl_info_limit(xce_sched_sim_driver_t* sim, const char* arg)
|
|||
repl_print_limit_dump(limit_buf);
|
||||
}
|
||||
}
|
||||
/* task 2.3 adds the Req 1.6 effective-host-scope note here; task 3
|
||||
* adds this limit's live usage. */
|
||||
repl_print_limit_host_scope_note_if_implicit(&(sim->core), match);
|
||||
/* task 3 adds this limit's live usage. */
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -423,6 +423,96 @@ static void add_configured_limit_with_host_exclusion(
|
|||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
}
|
||||
|
||||
/*
|
||||
* register_per_host_resource - task 2.3's own fixture: registers a plain,
|
||||
* non-GLOBAL/non-SHARED resource named `resname` directly into `sim`'s real
|
||||
* core.resmgr, the same makeResource() pattern test_repl_resource_table.c's
|
||||
* own build_worker_with_one_numeric_resource() already establishes.
|
||||
* is_host_based_limit() (src/lib/xcelimit.c) key_find()s a limit's
|
||||
* val.resname directly against core->resmgr and inspects the resulting
|
||||
* xce_resource_t's flags -- a bare resource-name string with no backing
|
||||
* xce_resource_t makes that key_find() miss (-ENOENT), which is NOT the
|
||||
* same as "genuinely a per-host, not-shared-cluster-wide resource" for this
|
||||
* check's purposes, so this fixture must exist for is_host_based_limit()'s
|
||||
* implicit-host-scope rule to be exercised at all.
|
||||
*/
|
||||
static xce_resource_t* register_per_host_resource(xce_sched_sim_driver_t* sim,
|
||||
const char* resname)
|
||||
{
|
||||
xce_resource_t wire_resource;
|
||||
xce_resource_t* r;
|
||||
|
||||
memset(&wire_resource, 0, sizeof(wire_resource));
|
||||
wire_resource.name = (char*)resname;
|
||||
wire_resource.valtype = XCE_VAL_NUMERIC;
|
||||
wire_resource.flags = XCE_RESF_CONSUMABLE;
|
||||
r = makeResource(sim->core.resmgr, &wire_resource);
|
||||
TEST_ASSERT_NOT_NULL(r);
|
||||
return r;
|
||||
}
|
||||
|
||||
/*
|
||||
* add_configured_percent_limit - task 2.3's own Requirement 1.6 POSITIVE
|
||||
* fixture: a percent-based limit (val.percent=1) on `resname`, with NO
|
||||
* explicit per_host/hosts config (per_host left at its zero default, same
|
||||
* "no explicit scoping" shape make_limit_config()/add_configured_limit()
|
||||
* already establish for the other three dimensions) -- the exact shape
|
||||
* is_host_based_limit()'s own implicit-host-scope rule targets (xcelimit.c:
|
||||
* resource flags lack XCE_RESF_GLOBAL/XCE_RESF_SHARED AND
|
||||
* limit->val.percent != 0 -> host-based regardless of configured
|
||||
* per_host/hosts).
|
||||
*/
|
||||
static void add_configured_percent_limit(xce_sched_plugin_t* plugin,
|
||||
const char* name,
|
||||
const char* resname)
|
||||
{
|
||||
xce_limit_val_t val = { .resname = (char*)resname,
|
||||
.percent = 1,
|
||||
.val = 50.0 };
|
||||
xce_limit_config_t limit_cfg = { .name = (char*)name, .val = &val };
|
||||
xce_limit_config_t* limits[1] = { &limit_cfg };
|
||||
xce_conf4sched_t config;
|
||||
int rc;
|
||||
|
||||
memset(&config, 0, sizeof(config));
|
||||
config.nlimits = 1;
|
||||
config.limits = limits;
|
||||
|
||||
rc = limit_update_config(plugin, &config);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
}
|
||||
|
||||
/*
|
||||
* add_configured_percent_limit_explicit_per_host - task 2.3's own
|
||||
* self-review fixture (design.md: "if CONSUMER_HOST IS explicitly
|
||||
* configured, fmt_limit()'s own output already shows PER_HOST = [...], so
|
||||
* no extra note is needed there"): identical to
|
||||
* add_configured_percent_limit() above except `.per_host = 1` is set
|
||||
* explicitly, so build_consumer() (xcelimit.c) marks
|
||||
* consumers[CONSUMER_HOST]->per_consumer = 1 -- the genuinely-explicit case
|
||||
* the Requirement 1.6 note must NOT duplicate.
|
||||
*/
|
||||
static void add_configured_percent_limit_explicit_per_host(
|
||||
xce_sched_plugin_t* plugin, const char* name, const char* resname)
|
||||
{
|
||||
xce_limit_val_t val = { .resname = (char*)resname,
|
||||
.percent = 1,
|
||||
.val = 50.0 };
|
||||
xce_limit_config_t limit_cfg = { .name = (char*)name,
|
||||
.per_host = 1,
|
||||
.val = &val };
|
||||
xce_limit_config_t* limits[1] = { &limit_cfg };
|
||||
xce_conf4sched_t config;
|
||||
int rc;
|
||||
|
||||
memset(&config, 0, sizeof(config));
|
||||
config.nlimits = 1;
|
||||
config.limits = limits;
|
||||
|
||||
rc = limit_update_config(plugin, &config);
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* Tests */
|
||||
/* ---------------------------------------------------------------------- */
|
||||
|
|
@ -730,6 +820,147 @@ static void test_info_limit_all_displays_every_configured_limits_setting(void)
|
|||
"PASS\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* Task 2.3's own Observable #1 (Requirement 1.6, positive case): a
|
||||
* percent-based limit on a resource that is registered but not
|
||||
* GLOBAL/SHARED (an ordinary per-host resource, "mem"), with NO explicit
|
||||
* per-host scoping configured, must display the Requirement 1.6 effective-
|
||||
* host-scope note after fmt_limit()'s own dump -- is_host_based_limit()'s
|
||||
* implicit rule (xcelimit.c) treats this limit as host-based purely because
|
||||
* of its percentage value on a non-shared resource, regardless of the
|
||||
* limit's own consumers[CONSUMER_HOST] configuration, and Requirement 1.6
|
||||
* requires the REPL to surface that to the operator.
|
||||
*/
|
||||
static void
|
||||
test_info_limit_named_shows_host_scope_note_for_implicit_percent_based_limit(
|
||||
void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
xce_sched_plugin_t plugin;
|
||||
xce_scheduler_t group_sched;
|
||||
char* captured;
|
||||
int rc;
|
||||
|
||||
memset(&sim, 0, sizeof(sim));
|
||||
rc = xce_sched_core_init(&(sim.core));
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
register_per_host_resource(&sim, "mem");
|
||||
register_limit_plugin(&sim, &plugin, &group_sched);
|
||||
add_configured_percent_limit(&plugin, "pct_limit", "mem");
|
||||
|
||||
captured = capture_info_limit(&sim, "pct_limit");
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "limit: pct_limit"),
|
||||
"the matched limit's fmt_limit() dump must still print first "
|
||||
"(proves this note is appended after, not instead of, task 2.2's "
|
||||
"own output)");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "tracked separately per host"),
|
||||
"Requirement 1.6: a percent-based limit on a resource that is not "
|
||||
"shared cluster-wide, with no explicit host scoping configured, "
|
||||
"must display the effective host-scope note");
|
||||
|
||||
free(captured);
|
||||
|
||||
printf("test_info_limit_named_shows_host_scope_note_for_implicit_"
|
||||
"percent_based_limit: PASS\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* Task 2.3's own Observable #2 (Requirement 1.6, negative contrast): an
|
||||
* absolute-valued limit (percent=0) on the SAME per-host resource must NOT
|
||||
* display the note -- is_host_based_limit()'s implicit rule only fires for
|
||||
* a PERCENTAGE value, so an absolute-value limit is never implicitly
|
||||
* host-based and no host-scope note applies. Reuses add_configured_limit()'s
|
||||
* existing absolute-value ("mem", percent=0, val=10.0) fixture shape,
|
||||
* proving the contrast is driven purely by percent vs. absolute, not by
|
||||
* some other difference between fixtures.
|
||||
*/
|
||||
static void
|
||||
test_info_limit_named_does_not_show_host_scope_note_for_absolute_value_limit(
|
||||
void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
xce_sched_plugin_t plugin;
|
||||
xce_scheduler_t group_sched;
|
||||
char* captured;
|
||||
int rc;
|
||||
|
||||
memset(&sim, 0, sizeof(sim));
|
||||
rc = xce_sched_core_init(&(sim.core));
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
register_per_host_resource(&sim, "mem");
|
||||
register_limit_plugin(&sim, &plugin, &group_sched);
|
||||
add_configured_limit(&plugin, "abs_limit");
|
||||
|
||||
captured = capture_info_limit(&sim, "abs_limit");
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "limit: abs_limit"),
|
||||
"sanity: the matched limit's fmt_limit() dump must still print "
|
||||
"(proves this really matched a real, configured limit)");
|
||||
TEST_ASSERT_NULL_MESSAGE(
|
||||
strstr(captured, "tracked separately per host"),
|
||||
"Requirement 1.6: an absolute-valued limit is not implicitly "
|
||||
"host-based, so the effective host-scope note must NOT appear");
|
||||
|
||||
free(captured);
|
||||
|
||||
printf("test_info_limit_named_does_not_show_host_scope_note_for_"
|
||||
"absolute_value_limit: PASS\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* Task 2.3's own self-review guard (design.md: "if CONSUMER_HOST IS
|
||||
* explicitly configured, fmt_limit()'s own output already shows
|
||||
* PER_HOST = [...], so no extra note is needed there"): a percent-based
|
||||
* limit on the same non-shared resource, but with per_host EXPLICITLY
|
||||
* configured, must show fmt_limit()'s own "PER_HOST = [...]" line but must
|
||||
* NOT ALSO print the Requirement 1.6 note -- proves the condition does not
|
||||
* accidentally fire for the genuinely-explicit case too (it would be
|
||||
* redundant, since the operator already sees the host dimension called out
|
||||
* by task 2.2's own output).
|
||||
*/
|
||||
static void
|
||||
test_info_limit_named_does_not_duplicate_host_scope_note_when_per_host_explicitly_configured(
|
||||
void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
xce_sched_plugin_t plugin;
|
||||
xce_scheduler_t group_sched;
|
||||
char* captured;
|
||||
int rc;
|
||||
|
||||
memset(&sim, 0, sizeof(sim));
|
||||
rc = xce_sched_core_init(&(sim.core));
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
register_per_host_resource(&sim, "mem");
|
||||
register_limit_plugin(&sim, &plugin, &group_sched);
|
||||
add_configured_percent_limit_explicit_per_host(&plugin,
|
||||
"explicit_pct_limit",
|
||||
"mem");
|
||||
|
||||
captured = capture_info_limit(&sim, "explicit_pct_limit");
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "PER_HOST ="),
|
||||
"sanity: fmt_limit() must show a PER_HOST scope line for an "
|
||||
"explicitly per-host-scoped limit (task 2.2's own output)");
|
||||
TEST_ASSERT_NULL_MESSAGE(
|
||||
strstr(captured, "tracked separately per host"),
|
||||
"Requirement 1.6's note would be redundant once fmt_limit() "
|
||||
"already shows PER_HOST -- it must not duplicate");
|
||||
|
||||
free(captured);
|
||||
|
||||
printf("test_info_limit_named_does_not_duplicate_host_scope_note_when_"
|
||||
"per_host_explicitly_configured: PASS\n");
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
|
|
@ -748,5 +979,11 @@ int main(void)
|
|||
RUN_TEST(
|
||||
test_info_limit_named_shows_both_inclusion_and_exclusion_membership);
|
||||
RUN_TEST(test_info_limit_all_displays_every_configured_limits_setting);
|
||||
RUN_TEST(
|
||||
test_info_limit_named_shows_host_scope_note_for_implicit_percent_based_limit);
|
||||
RUN_TEST(
|
||||
test_info_limit_named_does_not_show_host_scope_note_for_absolute_value_limit);
|
||||
RUN_TEST(
|
||||
test_info_limit_named_does_not_duplicate_host_scope_note_when_per_host_explicitly_configured);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue