admin-plane breakpoint 2: YAML resourcemap wiring + queue payload deep-copy
Breakpoint 2 of the admin-plane gaps (YAML flag/resourcemap wiring), plus
the queue payload ownership fix that the t1 controller segfault of
2026-10-23 12:41:13 traced to.
1. YAML resourcemap wiring (admin-plane-gaps 1.10):
- schema: Resource.distributions @8 / hostBased @9 uncommented --
resourcemap had no wire destination at all before this.
- resobj.c: resobj_init's #if 0 stub replaced with a real deep copy
(distributions + hbased_defined); resobj_destroy frees them. Deep
copy is mandatory: the source is myconf->cluster_config, which
keeps owning and later frees its tree.
- controller: reconcile_config_resources + seed_sharedinst_from_config
(cluster.c) seed c->sharedinst for SHARED|GLOBAL resources whose
distribution carries a value (the license-pool shape), from both
config_activate_committed paths, ahead of the conf4sched push.
- xceadmin: mapping.py maps resourcemap -> distributions (only the
location:[all]+value shape sets SHARED|GLOBAL), build.py renders
shared=true/distributions; golden expected.bin regenerated.
2. xcesched_push_configs resval double free (t1 12:39:52 abort): the
seeded push freed the resval entries AND the backing array via
free_name_val_list, then xlist_fini freed the same array again.
Detach xreslist before the FREEUP/xlist_fini block (same ownership
class bug-1506 fixed in sched_worker_change). Latent since inception:
every pre-1.10 push ran with nresvals==0 and never armed it.
3. queue payload deep-copy (t1 12:41:13 encode_Threshold_list segfault):
queue_init (create) MOVED the definition's users/hosts/thresholds/
window payloads out, and queue_refresh_def MOVED them out again --
but reconcile_config_queues runs MORE THAN ONCE over the same
cluster_config object (boot: pre-election activation AND phase2's
build_queues), so the second pass refreshed live queues from the
drained definition: thresholds became the poisoned (count>0,
array=NULL) pair, the generated encode_Threshold_list has no NULL
guard, and the next queue/worker push died on the NULL base.
Both paths now deep-copy (queue_init and queue_refresh_def); the
definition keeps its payloads, (NULL/0) genuinely means "no list in
this config", and reconcile is idempotent. New dup_group_config
helper in utils.c (the generated code has no GroupConfig dup).
Mirrors resobj_init's distributions deep-copy and worker.c
apply_join_state's dup pattern.
4. UT: test_reconcile_thresholds_survive_second_pass drives a triple
reconcile of a threshold-carrying queue and asserts definition
intactness plus a real encode_QueueConf -> wire -> decode round
trip; new test_config_resource_reconcile.c covers seed/deep-copy/
non-seed classes and idempotent re-seed.
Suites: controller_unit_test 1348/1348, sched_push 4, sched_worker_change 3,
apiserver_push 2, host_runusage 11, xcelib 759, xcealloc_oom 11,
99 scheduler binaries, all worker + harness suites, xceadmin pytest 375.
t1 E2E: boot double-reconcile clean, scheduler rejoin + full conf4sched
push applied, recovered crash-era PEND job reached DONE, 30-job saturation
flow (24 RUN + 6 PEND -> DONE), live gen-12 config activation with
re-push to the connected scheduler -- no crash, all four daemons healthy.
Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
parent
563f8e6890
commit
fc33a5da16
22 changed files with 1510 additions and 63 deletions
|
|
@ -90,6 +90,7 @@ extern void test_wal_job_fail_fact(void);
|
|||
extern void test_phase2_cold_boot(void);
|
||||
extern void test_persist_encryption(void);
|
||||
extern void test_config_queue_reconcile(void);
|
||||
extern void test_config_resource_reconcile(void);
|
||||
extern void test_wal_job_rusage_fact(void);
|
||||
extern void test_wal_job_ckpt_fact(void);
|
||||
extern void test_wal_job_preason_fact(void);
|
||||
|
|
@ -304,6 +305,7 @@ int main(int argc, char* argv[])
|
|||
TEST(test_phase2_cold_boot);
|
||||
TEST(test_persist_encryption);
|
||||
TEST(test_config_queue_reconcile);
|
||||
TEST(test_config_resource_reconcile);
|
||||
TEST(test_wal_job_rusage_fact);
|
||||
TEST(test_wal_job_ckpt_fact);
|
||||
TEST(test_wal_job_preason_fact);
|
||||
|
|
|
|||
|
|
@ -11,15 +11,18 @@
|
|||
|
||||
#include <stdarg.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#include "clusterconfig.h"
|
||||
#include "cluster.h"
|
||||
#include "controller.capnp.h"
|
||||
#include "hook.h"
|
||||
#include "myconf.h"
|
||||
#include "qobj.h"
|
||||
#include "utils.h"
|
||||
#include "xce/cluster_def.h"
|
||||
#include "xstring.h"
|
||||
|
||||
|
|
@ -180,9 +183,175 @@ static void test_reconcile_refreshes_existing_queue_in_place(void)
|
|||
dereference(c.qmgr);
|
||||
}
|
||||
|
||||
/* 2026-10-23 t1 controller segfault (encode_Threshold_list): the create path
|
||||
* (qobj.c queue_init) MOVED the config definition's payloads out (users/
|
||||
* hosts/thresholds/windows -- MOVE_PTR nulls the src pointers, and for
|
||||
* thresholds left src->nthresholds stale), and queue_refresh_def MOVED them
|
||||
* out again on refresh. reconcile_config_queues runs MORE THAN ONCE over
|
||||
* the SAME cluster_config object (boot: pre-election config_activate_committed
|
||||
* AND phase2's build_queues), so the second pass "refreshed" the live queue
|
||||
* from the drained definition: thresholds became the poisoned (count>0,
|
||||
* array=NULL) pair, the generated encode_Threshold_list has no NULL guard,
|
||||
* and the next queue/worker push dereferenced the NULL array base -- the
|
||||
* controller died. The fix makes BOTH paths deep-copy (the definition's
|
||||
* cluster_config keeps owning its payloads and frees them at swap; mirrors
|
||||
* resobj_init's distributions deep-copy and worker.c apply_join_state's dup
|
||||
* pattern), so a definition is never consumed, (NULL/0) in it genuinely
|
||||
* means "no list in this config", and reconcile is idempotent. This test
|
||||
* drives the poison scenario end to end: a queue carrying real thresholds
|
||||
* and membership must survive a triple reconcile with values intact, the
|
||||
* definition must stay untouched, and the encodable invariant
|
||||
* (count>0 <=> array!=NULL) must hold throughout. */
|
||||
static void test_reconcile_thresholds_survive_second_pass(void)
|
||||
{
|
||||
kvmgr_params_t str_params = { .key_type = KVMGR_KEY_STRING,
|
||||
.nslots = 16, .delta = 8 };
|
||||
cluster_t c;
|
||||
myconf_t conf;
|
||||
cluster_config_t cfg;
|
||||
xce_queue_t qa;
|
||||
xce_res_threshold_t thr;
|
||||
xce_res_threshold_t* thr_arr[1];
|
||||
xce_queue_t* queues[1];
|
||||
queue_t* q;
|
||||
|
||||
if (makeString((char*)"cqr-probe") == NULL)
|
||||
xstring_table_init(64);
|
||||
|
||||
memset(&c, 0, sizeof(c));
|
||||
memset(&conf, 0, sizeof(conf));
|
||||
memset(&cfg, 0, sizeof(cfg));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_hook_init(&c.hooksys, XCE_CONTROLLER_HOOK_MAX));
|
||||
c.qmgr = makeInstance(&xkvmgr_type, &str_params);
|
||||
TEST_ASSERT_NOT_NULL(c.qmgr);
|
||||
|
||||
/* A queue definition that actually carries a sched_stop threshold (the
|
||||
* "night" queue in src/configs/queues.yaml is the real-world shape).
|
||||
* The definition is created once and reconciled twice more, exactly
|
||||
* like a config object that survives boot's build_queues AND the
|
||||
* pre-election config_activate_committed. */
|
||||
memset(&qa, 0, sizeof(qa));
|
||||
qa.queue = (char*)"cqr-thr";
|
||||
qa.nthresholds = 1;
|
||||
memset(&thr, 0, sizeof(thr));
|
||||
thr.name = (char*)"tmp";
|
||||
thr.sched = 2000;
|
||||
thr.stop = 1000;
|
||||
thr_arr[0] = &thr;
|
||||
qa.thresholds = thr_arr;
|
||||
queues[0] = &qa;
|
||||
cfg.nqueues = 1;
|
||||
cfg.queues = queues;
|
||||
conf.cluster_config = &cfg;
|
||||
c.myconf = &conf;
|
||||
|
||||
/* First pass (boot's build_queues): queue created, thresholds
|
||||
* deep-copied in; the definition keeps ownership of its array. */
|
||||
TEST_ASSERT_EQUAL_INT(0, build_queues(&c));
|
||||
q = cqr_find(&c, "cqr-thr");
|
||||
TEST_ASSERT_NOT_NULL(q);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, q->queue.nthresholds,
|
||||
"first reconcile must adopt the thresholds count");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(q->queue.thresholds,
|
||||
"first reconcile must adopt the thresholds array");
|
||||
TEST_ASSERT_EQUAL_STRING("tmp", q->queue.thresholds[0]->name);
|
||||
/* (UNITY_INCLUDE_DOUBLE is off in this build -- compare as bits.) */
|
||||
TEST_ASSERT_EQUAL_MEMORY(&thr.sched, &q->queue.thresholds[0]->sched,
|
||||
sizeof(double));
|
||||
TEST_ASSERT_EQUAL_MEMORY(&thr.stop, &q->queue.thresholds[0]->stop,
|
||||
sizeof(double));
|
||||
|
||||
/* The create path must NOT have consumed the definition (the old
|
||||
* MOVE_PTR drained it to (0, dangling-array) -- the poison seed). */
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, qa.nthresholds,
|
||||
"create must deep-copy, not MOVE the definition's thresholds out");
|
||||
TEST_ASSERT_EQUAL_PTR_MESSAGE(thr_arr, qa.thresholds,
|
||||
"create must not steal the definition's thresholds array");
|
||||
|
||||
/* Second pass over the SAME config object (boot phase2): the refresh
|
||||
* re-copies from the intact definition -- the live queue must stay
|
||||
* (1, non-NULL). The old code left it (count>0, array=NULL), and the
|
||||
* next encode_Threshold_list segfaulted on the NULL base. */
|
||||
TEST_ASSERT_EQUAL_INT(0, reconcile_config_queues(&c, 1));
|
||||
TEST_ASSERT_EQUAL_PTR_MESSAGE(q, cqr_find(&c, "cqr-thr"),
|
||||
"refresh must stay in place");
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(1, q->queue.nthresholds,
|
||||
"second reconcile must keep the thresholds count");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(q->queue.thresholds,
|
||||
"2026-10-23 t1: the double reconcile used to leave (count>0, NULL), "
|
||||
"segfaulting the next encode_Threshold_list");
|
||||
TEST_ASSERT_EQUAL_STRING("tmp", q->queue.thresholds[0]->name);
|
||||
TEST_ASSERT_EQUAL_MEMORY(&thr.sched, &q->queue.thresholds[0]->sched,
|
||||
sizeof(double));
|
||||
|
||||
/* The definition must STILL be intact after the refresh pass too
|
||||
* (refresh deep-copies as well -- never MOVEs out of the config). */
|
||||
TEST_ASSERT_EQUAL_INT(1, qa.nthresholds);
|
||||
TEST_ASSERT_EQUAL_PTR(thr_arr, qa.thresholds);
|
||||
|
||||
/* Third pass still consistent (idempotent). */
|
||||
TEST_ASSERT_EQUAL_INT(0, reconcile_config_queues(&c, 1));
|
||||
TEST_ASSERT_EQUAL_INT(1, q->queue.nthresholds);
|
||||
TEST_ASSERT_NOT_NULL(q->queue.thresholds);
|
||||
TEST_ASSERT_EQUAL_STRING("tmp", q->queue.thresholds[0]->name);
|
||||
|
||||
/* Sanity: the encodable invariant the crash violated. */
|
||||
TEST_ASSERT_TRUE((q->queue.nthresholds > 0) ==
|
||||
(q->queue.thresholds != NULL));
|
||||
|
||||
/* End to end: encode the LIVE queue exactly like the conf4sched push
|
||||
* does (xcesched_push_configs -> encode_QueueConf -> encode_Threshold_
|
||||
* list). The generated list encoder has no NULL guard, so before the
|
||||
* fix the (count>0, NULL) shape the double reconcile produced crashed
|
||||
* right here. Round-trip the encoded queue back and check the
|
||||
* threshold survived the wire. */
|
||||
{
|
||||
struct capn cap;
|
||||
QueueConf_ptr qp;
|
||||
int64_t sz;
|
||||
uint8_t* buf;
|
||||
struct capn dcap;
|
||||
QueueConf_ptr dp;
|
||||
struct QueueConf wire;
|
||||
xce_queue_t back;
|
||||
|
||||
memset(&back, 0, sizeof(back));
|
||||
capn_init_malloc(&cap);
|
||||
encode_QueueConf_ptr(capn_root(&cap).seg, &qp, &q->queue);
|
||||
capn_setp(capn_root(&cap), 0, qp.p);
|
||||
sz = capn_size(&cap);
|
||||
TEST_ASSERT_TRUE(sz > 0);
|
||||
buf = (uint8_t*)malloc((size_t)sz);
|
||||
TEST_ASSERT_NOT_NULL(buf);
|
||||
/* capn_write_mem returns the bytes written (positive), not 0. */
|
||||
TEST_ASSERT_TRUE(capn_write_mem(&cap, buf, (size_t)sz, 0) > 0);
|
||||
capn_free(&cap);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, capn_init_mem(&dcap, buf, (size_t)sz, 0));
|
||||
dp.p = capn_getp(capn_root(&dcap), 0, 1);
|
||||
read_QueueConf(&wire, dp);
|
||||
decode_QueueConf(&back, &wire);
|
||||
TEST_ASSERT_EQUAL_STRING("cqr-thr", back.queue);
|
||||
TEST_ASSERT_EQUAL_INT(1, back.nthresholds);
|
||||
TEST_ASSERT_NOT_NULL(back.thresholds);
|
||||
TEST_ASSERT_EQUAL_STRING("tmp", back.thresholds[0]->name);
|
||||
TEST_ASSERT_EQUAL_MEMORY(&thr.sched, &back.thresholds[0]->sched,
|
||||
sizeof(double));
|
||||
TEST_ASSERT_EQUAL_MEMORY(&thr.stop, &back.thresholds[0]->stop,
|
||||
sizeof(double));
|
||||
free_QueueConf(&back);
|
||||
capn_free(&dcap);
|
||||
free(buf);
|
||||
}
|
||||
|
||||
xce_hook_fini(&c.hooksys);
|
||||
dereference(c.qmgr);
|
||||
}
|
||||
|
||||
void test_config_queue_reconcile(void)
|
||||
{
|
||||
UnitySetTestFile(__FILE__);
|
||||
RUN_TEST(test_reconcile_creates_queue_added_by_config);
|
||||
RUN_TEST(test_reconcile_refreshes_existing_queue_in_place);
|
||||
RUN_TEST(test_reconcile_thresholds_survive_second_pass);
|
||||
}
|
||||
|
|
|
|||
250
src/controller/testsuite/test_config_resource_reconcile.c
Normal file
250
src/controller/testsuite/test_config_resource_reconcile.c
Normal file
|
|
@ -0,0 +1,250 @@
|
|||
/* test_config_resource_reconcile.c -- admin-plane-gaps 1.10: a resource
|
||||
* definition added by a live config upgrade used to need a controller
|
||||
* restart (cluster_build_resources() ran only at boot), and a YAML-declared
|
||||
* STATIC shared pool (resourcemap `location: [all]` + `value:`, the
|
||||
* license-pool shape) never reached c->sharedinst at all -- the scheduler's
|
||||
* two-phase shared-pool machinery consumes exactly those resvals from every
|
||||
* conf4sched push. reconcile_config_resources() registers the config's
|
||||
* resource definitions (create-or-keep, EEXIST tolerated) and seeds
|
||||
* c->sharedinst for SHARED|GLOBAL resources whose distributions carry a
|
||||
* value; the whole pass is idempotent (a re-run swaps the seeded value via
|
||||
* zarray_put's killer, mirroring update_val_res's ownership contract). */
|
||||
#include <rtti.h>
|
||||
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#include "clusterconfig.h"
|
||||
#include "cluster.h"
|
||||
#include "hook.h"
|
||||
#include "myconf.h"
|
||||
#include "resobj.h"
|
||||
#include "xce/cluster_def.h"
|
||||
#include "xstring.h"
|
||||
|
||||
/* Same ownership contract as cluster.c's resval_obj_killer (static there --
|
||||
* test-local twin): c->sharedinst owns each xce_val_t and a string value's
|
||||
* private strdup. */
|
||||
static void crr_val_killer(void* arg)
|
||||
{
|
||||
xce_val_t* v = (xce_val_t*)arg;
|
||||
|
||||
if (v == NULL) {
|
||||
return;
|
||||
}
|
||||
if (v->which == Value_u_svalue) {
|
||||
FREEUP(v->u.svalue);
|
||||
}
|
||||
free(v);
|
||||
}
|
||||
|
||||
static xce_resource_t* crr_find(cluster_t* c, const char* name)
|
||||
{
|
||||
xce_resource_t* r = NULL;
|
||||
lock(c->resmgr, RTTI_LOCK_READ);
|
||||
key_find(c->resmgr, (void*)name, (void**)&r);
|
||||
unlock(c->resmgr);
|
||||
return r;
|
||||
}
|
||||
|
||||
static xce_val_t* crr_shared_val(cluster_t* c, int idx)
|
||||
{
|
||||
if (idx >= c->sharedinst.caps) {
|
||||
return NULL;
|
||||
}
|
||||
return (xce_val_t*)zarray_get(&c->sharedinst, idx);
|
||||
}
|
||||
|
||||
/* A config with:
|
||||
* res0 "slots" -- built-in hostbased (no resourcemap): registered, never seeded
|
||||
* res1 "lic1" -- SHARED|GLOBAL, distribution value 100: seeded
|
||||
* res2 "hostres"-- SHARED but NOT GLOBAL (per-host shape): NOT seeded
|
||||
* res3 "dyn1" -- SHARED|GLOBAL, distribution WITHOUT a value: NOT seeded */
|
||||
static void crr_build_config(cluster_config_t* cfg, xce_resource_t** resources,
|
||||
res_distribution_t** dists, xce_val_t** vals)
|
||||
{
|
||||
static xce_resource_t r0, r1, r2, r3;
|
||||
static res_distribution_t d1, d2, d3;
|
||||
static xce_val_t v1, v3;
|
||||
|
||||
memset(&r0, 0, sizeof(r0));
|
||||
r0.name = (char*)"slots";
|
||||
r0.flags = XCE_RESF_BUILTIN;
|
||||
|
||||
memset(&r1, 0, sizeof(r1));
|
||||
r1.name = (char*)"lic1";
|
||||
r1.flags = XCE_RESF_SHARED | XCE_RESF_GLOBAL;
|
||||
memset(&d1, 0, sizeof(d1));
|
||||
memset(&v1, 0, sizeof(v1));
|
||||
v1.which = Value_u_fvalue;
|
||||
v1.u.fvalue = 100.0;
|
||||
d1.value = &v1;
|
||||
r1.ndistributions = 1;
|
||||
r1.distributions = (res_distribution_t**)&dists[0];
|
||||
dists[0] = &d1;
|
||||
vals[1] = &v1;
|
||||
|
||||
memset(&r2, 0, sizeof(r2));
|
||||
r2.name = (char*)"hostres";
|
||||
r2.flags = XCE_RESF_SHARED;
|
||||
memset(&d2, 0, sizeof(d2));
|
||||
memset(&v3, 0, sizeof(v3));
|
||||
v3.which = Value_u_fvalue;
|
||||
v3.u.fvalue = 5.0;
|
||||
d2.value = &v3;
|
||||
r2.ndistributions = 1;
|
||||
r2.distributions = (res_distribution_t**)&dists[1];
|
||||
dists[1] = &d2;
|
||||
vals[2] = &v3;
|
||||
|
||||
memset(&r3, 0, sizeof(r3));
|
||||
r3.name = (char*)"dyn1";
|
||||
r3.flags = XCE_RESF_SHARED | XCE_RESF_GLOBAL;
|
||||
memset(&d3, 0, sizeof(d3)); /* value == NULL: worker-reported shape */
|
||||
r3.ndistributions = 1;
|
||||
r3.distributions = (res_distribution_t**)&dists[2];
|
||||
dists[2] = &d3;
|
||||
vals[3] = NULL;
|
||||
|
||||
resources[0] = &r0;
|
||||
resources[1] = &r1;
|
||||
resources[2] = &r2;
|
||||
resources[3] = &r3;
|
||||
cfg->nres = 4;
|
||||
cfg->resources = resources;
|
||||
}
|
||||
|
||||
static void test_reconcile_resources_registers_and_seeds_shared_pools(void)
|
||||
{
|
||||
kvmgr_params_t str_params = { .key_type = KVMGR_KEY_STRING,
|
||||
.nslots = 16, .delta = 8 };
|
||||
cluster_t c;
|
||||
myconf_t conf;
|
||||
cluster_config_t cfg;
|
||||
xce_resource_t* resources[4];
|
||||
res_distribution_t* dists[3];
|
||||
xce_val_t* vals[4];
|
||||
xce_resource_t* r;
|
||||
|
||||
if (makeString((char*)"crr-probe") == NULL)
|
||||
xstring_table_init(64);
|
||||
|
||||
memset(&c, 0, sizeof(c));
|
||||
memset(&conf, 0, sizeof(conf));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_hook_init(&c.hooksys, XCE_CONTROLLER_HOOK_MAX));
|
||||
c.resmgr = makeInstance(&xkvmgr_type, &str_params);
|
||||
TEST_ASSERT_NOT_NULL(c.resmgr);
|
||||
zarray_init(&c.sharedinst, 16,
|
||||
(zarray_item_killer_t)crr_val_killer);
|
||||
|
||||
crr_build_config(&cfg, resources, dists, vals);
|
||||
conf.cluster_config = &cfg;
|
||||
c.myconf = &conf;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, reconcile_config_resources(&c),
|
||||
"a full pass over a fresh config must succeed");
|
||||
|
||||
/* every definition registered with its own deep-copied runtime object */
|
||||
r = crr_find(&c, "lic1");
|
||||
TEST_ASSERT_NOT_NULL(r);
|
||||
TEST_ASSERT_EQUAL_INT(1, r->ndistributions);
|
||||
TEST_ASSERT_NOT_NULL(r->distributions[0]->value);
|
||||
/* deep-copy: the runtime object must NOT alias the config's value */
|
||||
TEST_ASSERT_NOT_EQUAL_MESSAGE(vals[1], r->distributions[0]->value,
|
||||
"the resmgr object owns a private copy of the config value");
|
||||
TEST_ASSERT_EQUAL_FLOAT(100.0, r->distributions[0]->value->u.fvalue);
|
||||
TEST_ASSERT_NOT_NULL(crr_find(&c, "slots"));
|
||||
TEST_ASSERT_NOT_NULL(crr_find(&c, "hostres"));
|
||||
TEST_ASSERT_NOT_NULL(crr_find(&c, "dyn1"));
|
||||
|
||||
/* SHARED|GLOBAL + valued distribution -> seeded into sharedinst */
|
||||
xce_val_t* v = crr_shared_val(&c, r->idx);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(v, "the static shared pool must be seeded "
|
||||
"from its config distribution value");
|
||||
TEST_ASSERT_EQUAL_FLOAT(100.0, v->u.fvalue);
|
||||
|
||||
/* SHARED-only (no GLOBAL) is NOT seeded -- host-subset pooling needs
|
||||
* the planned elastic-shared-resources scheduler work. */
|
||||
xce_resource_t* rh = crr_find(&c, "hostres");
|
||||
TEST_ASSERT_NULL_MESSAGE(crr_shared_val(&c, rh->idx),
|
||||
"a SHARED-but-not-GLOBAL resource must not seed the shared pool");
|
||||
|
||||
/* SHARED|GLOBAL but no distribution value (dynamic shape) is NOT
|
||||
* seeded -- workers report those values at runtime. */
|
||||
xce_resource_t* rd = crr_find(&c, "dyn1");
|
||||
TEST_ASSERT_NULL_MESSAGE(crr_shared_val(&c, rd->idx),
|
||||
"a value-less distribution must not seed the shared pool");
|
||||
|
||||
/* built-in, not shared at all: not seeded */
|
||||
xce_resource_t* rb = crr_find(&c, "slots");
|
||||
TEST_ASSERT_NULL(crr_shared_val(&c, rb->idx));
|
||||
|
||||
xce_hook_fini(&c.hooksys);
|
||||
dereference(c.resmgr);
|
||||
zarray_fini(&c.sharedinst);
|
||||
}
|
||||
|
||||
/* Idempotency + hot-apply: a second activation pass (1) keeps the existing
|
||||
* runtime objects (makeResource tolerates EEXIST), (2) swaps the seeded
|
||||
* value in place when the config changed it, and (3) leaves other slots
|
||||
* untouched. */
|
||||
static void test_reconcile_resources_is_idempotent_and_hot_applies(void)
|
||||
{
|
||||
kvmgr_params_t str_params = { .key_type = KVMGR_KEY_STRING,
|
||||
.nslots = 16, .delta = 8 };
|
||||
cluster_t c;
|
||||
myconf_t conf;
|
||||
cluster_config_t cfg;
|
||||
xce_resource_t* resources[4];
|
||||
res_distribution_t* dists[3];
|
||||
xce_val_t* vals[4];
|
||||
xce_resource_t* r;
|
||||
xce_val_t* v;
|
||||
|
||||
if (makeString((char*)"crr-probe") == NULL)
|
||||
xstring_table_init(64);
|
||||
|
||||
memset(&c, 0, sizeof(c));
|
||||
memset(&conf, 0, sizeof(conf));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_hook_init(&c.hooksys, XCE_CONTROLLER_HOOK_MAX));
|
||||
c.resmgr = makeInstance(&xkvmgr_type, &str_params);
|
||||
TEST_ASSERT_NOT_NULL(c.resmgr);
|
||||
zarray_init(&c.sharedinst, 16,
|
||||
(zarray_item_killer_t)crr_val_killer);
|
||||
|
||||
crr_build_config(&cfg, resources, dists, vals);
|
||||
conf.cluster_config = &cfg;
|
||||
c.myconf = &conf;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, reconcile_config_resources(&c));
|
||||
r = crr_find(&c, "lic1");
|
||||
TEST_ASSERT_NOT_NULL(r);
|
||||
v = crr_shared_val(&c, r->idx);
|
||||
TEST_ASSERT_NOT_NULL(v);
|
||||
TEST_ASSERT_EQUAL_FLOAT(100.0, v->u.fvalue);
|
||||
|
||||
/* The admin edits the shared pool's capacity in the activated config. */
|
||||
vals[1]->u.fvalue = 250.0;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, reconcile_config_resources(&c),
|
||||
"a re-run over the same config (idempotent activation) must succeed");
|
||||
TEST_ASSERT_EQUAL_PTR_MESSAGE(r, crr_find(&c, "lic1"),
|
||||
"an existing runtime resource must be kept, not re-created");
|
||||
v = crr_shared_val(&c, r->idx);
|
||||
TEST_ASSERT_NOT_NULL(v);
|
||||
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(250.0, v->u.fvalue,
|
||||
"the edited capacity must hot-apply into the shared pool");
|
||||
|
||||
xce_hook_fini(&c.hooksys);
|
||||
dereference(c.resmgr);
|
||||
zarray_fini(&c.sharedinst);
|
||||
}
|
||||
|
||||
void test_config_resource_reconcile(void)
|
||||
{
|
||||
UnitySetTestFile(__FILE__);
|
||||
RUN_TEST(test_reconcile_resources_registers_and_seeds_shared_pools);
|
||||
RUN_TEST(test_reconcile_resources_is_idempotent_and_hot_applies);
|
||||
}
|
||||
|
|
@ -45,11 +45,16 @@
|
|||
#include "cluster.h"
|
||||
#include "controller.capnp.h"
|
||||
#include "hook.h"
|
||||
#include "kvmgr.h"
|
||||
#include "myconf.h"
|
||||
#include "netio.h"
|
||||
#include "request.h"
|
||||
#include "resobj.h"
|
||||
#include "xce/cluster_def.h"
|
||||
#include "xce/types.h"
|
||||
#include "xlist.h"
|
||||
#include "xstring.h"
|
||||
#include "zarray.h"
|
||||
|
||||
extern int on_scheduler_join(cluster_t*);
|
||||
|
||||
|
|
@ -269,6 +274,318 @@ static void test_single_element_job_new_is_unaffected(void)
|
|||
printf("test_single_element_job_new_is_unaffected: PASS\n");
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* admin-plane-gaps 1.10/2: xcesched_push_configs() resval ownership */
|
||||
/* regression (the seeded sharedinst push) */
|
||||
/* */
|
||||
/* admin-plane-gaps breakpoint 2 (YAML resourcemap wiring) made the */
|
||||
/* controller SEED c->sharedinst for static SHARED|GLOBAL resources */
|
||||
/* (config_activate_committed -> reconcile_config_resources), so */
|
||||
/* xcesched_push_configs() started pushing with nresvals > 0 for the */
|
||||
/* first time ever. That immediately armed a LATENT double free: the */
|
||||
/* function freed the pushed resvals with free_name_val_list() -- which */
|
||||
/* frees the ENTRIES *and the backing array* (utils.c's FREEUP(resvals))*/
|
||||
/* -- then xlist_fini(&xreslist) freed the SAME array a second time */
|
||||
/* (the array xreslist.data aliases), aborting the controller with */
|
||||
/* glibc "double free or corruption (out)". Every pre-1.10 push ran */
|
||||
/* with nresvals == 0 and never armed it, because free_name_val_list() */
|
||||
/* early-returns WITHOUT freeing the array when count <= 0. */
|
||||
/* */
|
||||
/* The fix detaches xreslist (data/count/caps zeroed) after the */
|
||||
/* free_name_val_list() call and before the xlist_fini() block -- the */
|
||||
/* same ownership class bug-1506 fixed in sched_worker_change() (whose */
|
||||
/* own regression test lives in test_sched_worker_change.c, the other */
|
||||
/* standalone binary linking real sched.c). */
|
||||
/* */
|
||||
/* These tests need the REAL xcesched_push_configs(), which only this */
|
||||
/* standalone binary can link (controller_unit_test's xcesched_push_ */
|
||||
/* configs is test_config_repl.c's no-op capturing fake). Under ASAN a */
|
||||
/* regression of the double free aborts INSIDE xcesched_push_configs() */
|
||||
/* -- before any assertion below even runs. */
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
||||
/* Same killer cluster.c's resval_obj_killer uses for c->sharedinst:
|
||||
* the array owns each xce_val_t, and a string value owns a private
|
||||
* strdup()ed copy. */
|
||||
static void jsp_sharedinst_killer(void* p)
|
||||
{
|
||||
xce_val_t* v = (xce_val_t*)p;
|
||||
|
||||
if (v == NULL) {
|
||||
return;
|
||||
}
|
||||
if (v->which == Value_u_svalue) {
|
||||
FREEUP(v->u.svalue);
|
||||
}
|
||||
free(v);
|
||||
}
|
||||
|
||||
/* Stack fixtures for the one cluster_t this pair of tests drives. The
|
||||
* resources/queues/confvars collects run against a real resmgr (kvmgr)
|
||||
* but NULL configmgr/qmgr (both are RTTI-API NULL-safe; the real
|
||||
* controller has them non-NULL but this fixture has no configs/queues
|
||||
* to contribute). */
|
||||
static cluster_t g_jsp_cfg_c;
|
||||
static endpoint_t g_jsp_cfg_ep;
|
||||
static myconf_t g_jsp_myconf;
|
||||
static cluster_config_t g_jsp_ccfg;
|
||||
static var g_jsp_resmgr;
|
||||
static xce_resource_t* g_jsp_lic_res;
|
||||
static zarray_t g_jsp_sharedinst;
|
||||
static int g_jsp_cfg_seeded_state; /* 0 = fresh, 1 = seeded */
|
||||
|
||||
/* Registers one SHARED|GLOBAL resource ("staticres4", the E2E license-pool
|
||||
* name) in a fresh kvmgr resmgr -- mirroring cluster_build_resources()/
|
||||
* reconcile_config_resources()'s own makeResource() call -- and returns the
|
||||
* runtime object (idx assigned by key_add). rdef carries a distributions[0]
|
||||
* with value = fvalue 100.0 exactly like the seeded YAML resourcemap shape
|
||||
* (`location: [all]` + `value: 100`): resobj_init() DEEP-COPIES
|
||||
* distributions into the runtime object, so the fixture's own rdef/dvalue
|
||||
* can live on the caller's stack. */
|
||||
static xce_resource_t* jsp_cfg_setup(const char* resname)
|
||||
{
|
||||
kvmgr_params_t resmgr_params = {
|
||||
.key_type = KVMGR_KEY_STRING,
|
||||
.nslots = 64,
|
||||
.delta = 16,
|
||||
.datakiller = NULL
|
||||
};
|
||||
xce_val_t* dvalue;
|
||||
res_distribution_t* dist;
|
||||
xce_resource_t rdef;
|
||||
xce_resource_t* r;
|
||||
|
||||
if (!g_jsp_cfg_seeded_state) {
|
||||
memset(&g_jsp_cfg_c, 0, sizeof(g_jsp_cfg_c));
|
||||
memset(&g_jsp_myconf, 0, sizeof(g_jsp_myconf));
|
||||
memset(&g_jsp_ccfg, 0, sizeof(g_jsp_ccfg));
|
||||
TEST_ASSERT_EQUAL_INT(
|
||||
0, xce_hook_init(&g_jsp_cfg_c.hooksys, XCE_CONTROLLER_HOOK_MAX));
|
||||
g_jsp_cfg_seeded_state = 1;
|
||||
}
|
||||
|
||||
/* xcesched_push_configs()'s guard: both must be non-NULL or the push
|
||||
* is skipped entirely. */
|
||||
g_jsp_myconf.cluster_config = &g_jsp_ccfg;
|
||||
g_jsp_ccfg.clustername = (char*)"jsp-cluster";
|
||||
g_jsp_cfg_c.myconf = &g_jsp_myconf;
|
||||
|
||||
g_jsp_resmgr = makeInstance(&xkvmgr_type, &resmgr_params);
|
||||
TEST_ASSERT_NOT_NULL(g_jsp_resmgr);
|
||||
g_jsp_cfg_c.resmgr = g_jsp_resmgr;
|
||||
|
||||
/* The config-owned resource definition (what myconf->cluster_config->
|
||||
* resources would point at after an activation). SHARED|GLOBAL with a
|
||||
* static value -- the YAML resourcemap mapping's exact output shape. */
|
||||
dvalue = (xce_val_t*)calloc(1, sizeof(*dvalue));
|
||||
TEST_ASSERT_NOT_NULL(dvalue);
|
||||
dvalue->which = Value_u_fvalue;
|
||||
dvalue->u.fvalue = 100.0;
|
||||
|
||||
dist = (res_distribution_t*)calloc(1, sizeof(*dist));
|
||||
TEST_ASSERT_NOT_NULL(dist);
|
||||
dist->value = dvalue;
|
||||
dist->nhosts = 0; /* encode_ResourceDistribution strlen()s hosts[i];
|
||||
the [all] shape has no per-host legs */
|
||||
dist->hosts = NULL;
|
||||
|
||||
memset(&rdef, 0, sizeof(rdef));
|
||||
rdef.name = (char*)resname;
|
||||
rdef.valtype = XCE_VAL_NUMERIC;
|
||||
rdef.flags = XCE_RESF_SHARED | XCE_RESF_GLOBAL;
|
||||
rdef.idx = -1;
|
||||
rdef.ndistributions = 1;
|
||||
rdef.distributions = &dist;
|
||||
r = makeResource(g_jsp_resmgr, &rdef);
|
||||
TEST_ASSERT_NOT_NULL(r);
|
||||
g_jsp_lic_res = r;
|
||||
|
||||
/* c->sharedinst seeding (reconcile_config_resources ->
|
||||
* seed_sharedinst_from_config): one heap xce_val_t at the resource's
|
||||
* RUNTIME idx, owned by the array via the killer above. */
|
||||
TEST_ASSERT_EQUAL_INT(0, zarray_init(&g_jsp_sharedinst, 16,
|
||||
jsp_sharedinst_killer));
|
||||
g_jsp_cfg_c.sharedinst = g_jsp_sharedinst;
|
||||
|
||||
memset(&g_jsp_cfg_ep, 0, sizeof(g_jsp_cfg_ep));
|
||||
g_jsp_cfg_ep.fd = -1;
|
||||
|
||||
g_jsp_cfg_c.sched.ep = &g_jsp_cfg_ep;
|
||||
g_jsp_cfg_c.running = 1;
|
||||
g_jsp_cfg_c.myhost = &g_jsp_cfg_c.myhost_info;
|
||||
g_jsp_cfg_c.myhost_info.name = (char*)"ctrl-jsp-cfg";
|
||||
g_jsp_cfg_c.myhost_info.key = (char*)"";
|
||||
|
||||
return r;
|
||||
}
|
||||
|
||||
static void jsp_cfg_teardown(void)
|
||||
{
|
||||
jsp_drain_ep(&g_jsp_cfg_ep);
|
||||
g_jsp_cfg_c.sched.ep = NULL;
|
||||
|
||||
zarray_fini(&g_jsp_cfg_c.sharedinst);
|
||||
memset(&g_jsp_sharedinst, 0, sizeof(g_jsp_sharedinst));
|
||||
g_jsp_cfg_c.sharedinst = g_jsp_sharedinst;
|
||||
|
||||
g_jsp_cfg_c.resmgr = NULL;
|
||||
g_jsp_cfg_c.myconf = NULL;
|
||||
g_jsp_lic_res = NULL;
|
||||
if (g_jsp_resmgr != NULL) {
|
||||
dereference(g_jsp_resmgr);
|
||||
g_jsp_resmgr = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
/* Raw-capnp-decode the queued config push and return the deep-copied
|
||||
* Conf4Sched payload in *out (caller frees via free_ConfigData_ptr +
|
||||
* free(), then capn_free). Asserts the envelope shape. Returns 0, or
|
||||
* -1 if the queue did not hold exactly one configpushmsg packet. */
|
||||
static int jsp_decode_config_push(endpoint_t* ep, xce_configdata_t** out)
|
||||
{
|
||||
data_item_t* it;
|
||||
struct capn c;
|
||||
struct Request rq;
|
||||
Request_ptr rp;
|
||||
int found = 0;
|
||||
int rc = -1;
|
||||
|
||||
*out = NULL;
|
||||
while ((it = endpoint_deq(ep)) != NULL) {
|
||||
io_packet_t* pkt = ITPTR_TO_PACKET(it);
|
||||
|
||||
if (found == 0) {
|
||||
capn_init_mem(&c, (const uint8_t*)pkt->data, pkt->sz, 0);
|
||||
rp.p = capn_getp(capn_root(&c), 0, 1);
|
||||
read_Request(&rq, rp);
|
||||
if (rq.body_which == Request_body_configpushmsg) {
|
||||
decode_ConfigData_ptr(out, rq.body.configpushmsg);
|
||||
found = 1;
|
||||
}
|
||||
capn_free(&c);
|
||||
}
|
||||
free_packet(pkt);
|
||||
free(pkt);
|
||||
}
|
||||
|
||||
if (found && *out != NULL
|
||||
&& (*out)->data_which == ConfigData_data_schedconfig
|
||||
&& (*out)->data.conf4sched != NULL) {
|
||||
rc = 0;
|
||||
}
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
/* Test 3 (regression): a SEEDED sharedinst produces one configpushmsg with
|
||||
* the seeded resval (name + fvalue 100.0) -- and, under ASAN, completing at
|
||||
* all proves the post-push free sequence no longer double-frees the resval
|
||||
* backing array. Pre-fix this aborts inside xcesched_push_configs(). */
|
||||
static void test_seeded_sharedinst_push_carries_resvals_without_double_free(
|
||||
void)
|
||||
{
|
||||
xce_resource_t* r;
|
||||
xce_val_t* v;
|
||||
xce_configdata_t* cd = NULL;
|
||||
xce_conf4sched_t* cf;
|
||||
int rc;
|
||||
|
||||
r = jsp_cfg_setup("staticres4");
|
||||
TEST_ASSERT_TRUE(r->idx >= 0);
|
||||
|
||||
v = (xce_val_t*)calloc(1, sizeof(*v));
|
||||
TEST_ASSERT_NOT_NULL(v);
|
||||
v->which = Value_u_fvalue;
|
||||
v->u.fvalue = 100.0;
|
||||
TEST_ASSERT_TRUE(zarray_put(&g_jsp_cfg_c.sharedinst, r->idx, v) >= 0);
|
||||
|
||||
/* Caller contract: c->sched.lock is held (same as the config_repl.c
|
||||
* activation seam and on_scheduler_join's push branch). */
|
||||
mylock_lock(&(g_jsp_cfg_c.sched.lock));
|
||||
rc = xcesched_push_configs(&g_jsp_cfg_c);
|
||||
mylock_unlock(&(g_jsp_cfg_c.sched.lock));
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, rc, "the seeded config push must succeed");
|
||||
|
||||
rc = jsp_decode_config_push(&g_jsp_cfg_ep, &cd);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, rc,
|
||||
"exactly one configpushmsg carrying a non-NULL schedconfig must be "
|
||||
"queued");
|
||||
|
||||
cf = cd->data.conf4sched;
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
1, cf->nresvals,
|
||||
"the push must carry exactly the one seeded shared resval");
|
||||
TEST_ASSERT_NOT_NULL(cf->resvals);
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||
"staticres4", cf->resvals[0]->name,
|
||||
"the resval's name must be the shared resource's name");
|
||||
TEST_ASSERT_NOT_NULL(cf->resvals[0]->val);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
Value_u_fvalue, cf->resvals[0]->val->which,
|
||||
"the seeded value must survive the wire encode/decode as an fvalue");
|
||||
/* This Unity build excludes double-precision assertions (a
|
||||
* TEST_ASSERT_EQUAL_DOUBLE here fails with "Unity Double Precision
|
||||
* Disabled"), so compare against an exact-equality expectation the
|
||||
* literal way -- fvalue is copied by value through both encoders. */
|
||||
TEST_ASSERT_TRUE_MESSAGE(
|
||||
cf->resvals[0]->val->u.fvalue == 100.0,
|
||||
"the seeded static pool value (the YAML `value: 100` shape) must "
|
||||
"reach the scheduler-side payload intact");
|
||||
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||
"jsp-cluster", cf->clustername,
|
||||
"the push must still carry the clustername from the active config");
|
||||
|
||||
free_ConfigData_ptr(&cd);
|
||||
free(cd);
|
||||
|
||||
jsp_cfg_teardown();
|
||||
printf("test_seeded_sharedinst_push_carries_resvals_without_double_free:"
|
||||
" PASS\n");
|
||||
}
|
||||
|
||||
/* Test 4 (latency documentation): an UNSEEDED sharedinst still pushes, with
|
||||
* nresvals == 0. This is the shape EVERY push had before breakpoint 2's
|
||||
* seeding existed -- and why the double free stayed latent for years:
|
||||
* free_name_val_list() early-returns WITHOUT freeing the array when
|
||||
* count <= 0, so xreslist.data was never actually freed twice. */
|
||||
static void test_unseeded_push_still_sends_empty_resvals(void)
|
||||
{
|
||||
xce_resource_t* r;
|
||||
xce_configdata_t* cd = NULL;
|
||||
xce_conf4sched_t* cf;
|
||||
int rc;
|
||||
|
||||
r = jsp_cfg_setup("staticres4");
|
||||
TEST_ASSERT_TRUE(r->idx >= 0);
|
||||
(void)r;
|
||||
|
||||
mylock_lock(&(g_jsp_cfg_c.sched.lock));
|
||||
rc = xcesched_push_configs(&g_jsp_cfg_c);
|
||||
mylock_unlock(&(g_jsp_cfg_c.sched.lock));
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
|
||||
rc = jsp_decode_config_push(&g_jsp_cfg_ep, &cd);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, rc,
|
||||
"an unseeded cluster must still push configs to the scheduler");
|
||||
|
||||
cf = cd->data.conf4sched;
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, cf->nresvals,
|
||||
"no sharedinst seeds means no resvals on the push -- this is the "
|
||||
"shape every pre-admin-plane-gaps-1.10 push had, which is why the "
|
||||
"xreslist double free stayed latent (free_name_val_list() does not "
|
||||
"free the backing array when count <= 0)");
|
||||
|
||||
free_ConfigData_ptr(&cd);
|
||||
free(cd);
|
||||
|
||||
jsp_cfg_teardown();
|
||||
printf("test_unseeded_push_still_sends_empty_resvals: PASS\n");
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* Standalone Unity main (this binary is its own test executable — see */
|
||||
/* meson.build's sched_push_unit_test) */
|
||||
|
|
@ -282,10 +599,21 @@ int main(void)
|
|||
printf("\n>> job-array-single-wal-record task 5.1: compact scheduler "
|
||||
"push\n\n");
|
||||
|
||||
/* makeResource() (resobj.c) interns resource names via makeString(),
|
||||
* which needs the global xstring table initialized before first use --
|
||||
* same requirement test_sched_worker_change.c documents at its own
|
||||
* main(). This is a standalone binary (no other test file shares
|
||||
* process init with it), so it is done exactly once here. The two
|
||||
* compact-push tests above never needed it (no makeResource call);
|
||||
* the admin-plane-gaps resval-ownership tests below do. */
|
||||
xstring_table_init(139);
|
||||
|
||||
UNITY_BEGIN();
|
||||
|
||||
RUN_TEST(test_multi_element_job_new_produces_one_compact_jobarray_push);
|
||||
RUN_TEST(test_single_element_job_new_is_unaffected);
|
||||
RUN_TEST(test_seeded_sharedinst_push_carries_resvals_without_double_free);
|
||||
RUN_TEST(test_unseeded_push_still_sends_empty_resvals);
|
||||
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue