From 553da7c7605ad6de980cd4a8d2ffaa011b7be3a0 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 21 Jul 2026 07:58:29 +0000 Subject: [PATCH] =?UTF-8?q?feat(controller):=20clusterconfig-schema-split?= =?UTF-8?q?=20task=206.2=20=E2=80=94=20wire=20CLI=20tunables=20into=20init?= =?UTF-8?q?=5Fcontroller=5Ftunables=20(optional,=20Req=206)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires all 11 task-6.1 CLI-sourced myconf_t fields into init_controller_tunables() via the codebase's existing 0-sentinel fallback idiom ((myconf->x > 0) ? myconf->x : ); omitting all flags reproduces today's exact hardcoded values. Resolves both gaps flagged in task 6.1's review: resmgr_inst_delta bridges to the existing tunables.resmgr_inst field by meaning (confirmed via zarray_init's delta parameter), and xstrings_delta is deliberately left unwired with an operator-facing WARN rather than adding a dead field -- the underlying xstring_table_init/kv_params_t has no delta/growth-step concept at all, unlike the kvmgr-backed jobmgr/workermgr/qmgr/resmgr. This completes clusterconfig-schema-split -- all required tasks (1.1-5.3) and optional Requirement 6 tasks (6.1-6.2) are done. --- .../specs/clusterconfig-schema-split/tasks.md | 2 +- src/controller/cluster.c | 67 +++++++++++++++---- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/.kiro/specs/clusterconfig-schema-split/tasks.md b/.kiro/specs/clusterconfig-schema-split/tasks.md index d0222b1..a0e7194 100644 --- a/.kiro/specs/clusterconfig-schema-split/tasks.md +++ b/.kiro/specs/clusterconfig-schema-split/tasks.md @@ -100,7 +100,7 @@ - _Depends: 1.2_ - _Boundary: CLI Tunables Component (main.c, myconf.h)_ -- [ ] 6.2 Wire the tunables into `init_controller_tunables()` +- [x] 6.2 Wire the tunables into `init_controller_tunables()` - Read the new `myconf_t` fields in `init_controller_tunables()`, falling back to the existing hardcoded default for any field left at 0 (unset) - Observable: passing a tunable flag changes the corresponding controller tunable at startup; omitting all of them reproduces today's exact hardcoded values - _Requirements: 6.2_ diff --git a/src/controller/cluster.c b/src/controller/cluster.c index 1ea462b..dbcdb5f 100644 --- a/src/controller/cluster.c +++ b/src/controller/cluster.c @@ -1014,8 +1014,13 @@ static void config_obj_killer(void* arg) * Called early in cluster_init_phase1 so the values are available throughout * the startup sequence (including pre-election and phase2). * - * TODO: wire these to cluster_config fields once the config generation tool - * supports jobmgr/workermgr/qmgr/resmgr/xstrings (@13–@17). + * Req 6.2: each tunable is sourced from its CLI-set myconf_t field + * (--jobmgr-nhash etc., populated in main.c) when nonzero; a field left at + * 0 (unset) falls back to the hardcoded default below, so omitting all + * tunable flags reproduces today's exact values. The cluster_config + * @13–@17 fields this TODO used to reference were removed entirely by + * clusterconfig-schema-split Requirement 2 (they were already dead) and + * will never be wired — CLI flags are the sole configuration path now. */ /* * cluster_init_managers: allocate and initialize the manager subsystems. @@ -1071,18 +1076,52 @@ static int cluster_init_managers(cluster_t *c) static void init_controller_tunables(cluster_t *c) { /* Jobs can be numerous; workers/queues/resources are typically smaller. - * TODO: replace with config-driven values once the config generation tool - * supports cluster_config fields @13–@17 (jobmgr/workermgr/qmgr/resmgr/xstrings). */ - c->tunables.jobmgr_nhash = 512; - c->tunables.jobmgr_delta = 128; - c->tunables.workermgr_nhash = 128; - c->tunables.workermgr_delta = 32; - c->tunables.qmgr_nhash = 128; - c->tunables.qmgr_delta = 32; - c->tunables.resmgr_nhash = 128; - c->tunables.resmgr_delta = 32; - c->tunables.resmgr_inst = 64; - c->tunables.xstrings_nhash = 512; + * Req 6.2: same 0-sentinel idiom as myconf->vcpus (myapp.c) — a nonzero + * CLI-set myconf_t value overrides the hardcoded default; 0 (unset) + * reproduces today's exact value. */ + myconf_t *myconf = c->myconf; + + c->tunables.jobmgr_nhash = (myconf->jobmgr_nhash > 0) + ? myconf->jobmgr_nhash : 512; + c->tunables.jobmgr_delta = (myconf->jobmgr_delta > 0) + ? myconf->jobmgr_delta : 128; + c->tunables.workermgr_nhash = (myconf->workermgr_nhash > 0) + ? myconf->workermgr_nhash : 128; + c->tunables.workermgr_delta = (myconf->workermgr_delta > 0) + ? myconf->workermgr_delta : 32; + c->tunables.qmgr_nhash = (myconf->qmgr_nhash > 0) + ? myconf->qmgr_nhash : 128; + c->tunables.qmgr_delta = (myconf->qmgr_delta > 0) + ? myconf->qmgr_delta : 32; + c->tunables.resmgr_nhash = (myconf->resmgr_nhash > 0) + ? myconf->resmgr_nhash : 128; + c->tunables.resmgr_delta = (myconf->resmgr_delta > 0) + ? myconf->resmgr_delta : 32; + /* resmgr_inst_delta (myconf/CLI name) maps by MEANING, not by name, to + * tunables.resmgr_inst (runtime field name) — both are the same + * zarray_init() growth-delta for c->sharedinst (confirmed: zarray_init's + * 2nd parameter is literally named `delta` in zarray.c). Do not rename + * tunables.resmgr_inst itself — out of scope, other readers unverified. */ + c->tunables.resmgr_inst = (myconf->resmgr_inst_delta > 0) + ? myconf->resmgr_inst_delta : 64; + c->tunables.xstrings_nhash = (myconf->xstrings_nhash > 0) + ? myconf->xstrings_nhash : 512; + + /* myconf->xstrings_delta (CLI: --xstrings-delta, added in task 6.1) is + * intentionally NOT wired to any c->tunables field: xstring_table_init() + * (src/lib/xstring.c) takes only an nslots parameter, and its backing + * store (kvtab_type, src/lib/kv.c) has no delta/growth-step concept at + * all -- each bucket entry is calloc'd individually, unlike the + * xkvmgr_type-backed jobmgr/workermgr/qmgr/resmgr tables which do grow + * by a configurable delta. Adding a tunables.xstrings_delta field would + * be dead: nothing could ever read it. WARN so the flag isn't a silent + * no-op for an operator who sets it. */ + if (myconf->xstrings_delta > 0) { + WARN("--xstrings-delta=%u has no effect: the xstrings table has no " + "growth-delta dimension (xstring_table_init() takes only " + "nslots); use --xstrings-nhash instead", + myconf->xstrings_delta); + } } int cluster_init_phase1(cluster_t* c)