lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(scheduler): replace limits on a config re-push (bug-1967)

Browse source 
limit_seed_config_impl() key_add()ed each pushed limit into the existing
table and returned at the first failure, so on any second push (live
config upgrade, or a simulator CONFIG_PUSH since bug-1964) the first
already-present name failed with -EEXIST: changed limits kept the old
definition and later new limits were never added.

A push now builds the whole new limit set into a fresh table and swaps it
in (all-or-nothing: on any build/register failure the previous set and its
instances stay). Usage instances are carried across: an instance of a kept
limit whose scoping and resource are unchanged is rebound to the new
definition; an instance of a removed limit, or of one whose per-consumer
scoping, resource or percent changed, is dropped and re-created on the next
match. Usage itself lives in the resource manager under the instance's
virtual resource, so counts survive either way. Not part of the snapshot
(a snapshot carries the config and reseeds from it).

The seed moves to plugins/limit_config.c (limit.c is near the 800-line
cap) with the private data type in plugins/limit_internal.h. Two REPL
limit-table fixtures relied on the additive behaviour (two single-limit
pushes) and now configure both limits in one push.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-10 11:37:36 +00:00
parent f45b85c9e5
commit 8faf59618e
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
7 changed files with 582 additions and 95 deletions
Download patch file Download diff file Expand all files Collapse all files

238
src/scheduler/testsuite/test_limit_plugin.c
View file

@ -99,6 +99,7 @@ int plugin_register(void* sched, void* plugin)
}
#include "../src/plugins/limit.c"
#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */
void setUp(void) { return; }
void tearDown(void) { return; }
@ -508,6 +509,237 @@ static void test_limit_scopes_resolve_against_own_group_kind(void)
free_limit_config(limits[1]);
}
/* ---------------------------------------------------------------------- */
/* bug-1967: a re-pushed config (live config upgrade, or a mid-replay
* CONFIG_PUSH in the simulator) REPLACES the configured limit set. Before
* the fix every name of the second push already existed, so the first
* key_add() failed with -EEXIST and the push changed nothing.
* ---------------------------------------------------------------------- */
/* pushes `n` limit configs through the config hook, as both drivers do */
static int push_limits(xce_sched_plugin_t* plugin, xce_limit_config_t** limits,
int n)
{
xce_conf4sched_t config;
memset(&config, 0, sizeof(config));
config.nlimits = n;
config.limits = limits;
return limit_update_config(plugin, &config);
}
static xce_limit_t* find_limit(xce_sched_plugin_t* plugin, const char* name)
{
limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data;
xce_limit_t* limit = NULL;
key_find(privdata->limits, (void*)name, (void**)&limit);
return limit;
}
static xce_vres_t* find_instance(xce_sched_plugin_t* plugin, const char* name)
{
limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data;
xce_vres_t* vres = NULL;
key_find(privdata->instances, (void*)name, (void**)&vres);
return vres;
}
static int count_entries(var table)
{
iterator_t it;
void* item = NULL;
int n = 0;
TEST_ASSERT_TRUE(iter_make(&it, table) >= 0);
while (iter_next(&it, &item) == RTTI_ITER_OK) {
n++;
}
iter_destroy(&it);
return n;
}
/* a usage instance as match_job_global_limit_() interns it: keyed by its
* virtual resource's name, pointing at the configured limit */
static void add_instance(xce_sched_plugin_t* plugin, xce_resource_t* res,
const char* limit_name)
{
limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data;
xce_vres_t* vres = (xce_vres_t*)calloc(1, sizeof(*vres));
TEST_ASSERT_NOT_NULL(vres);
vres->vres = res;
vres->limit = find_limit(plugin, limit_name);
TEST_ASSERT_NOT_NULL(vres->limit);
TEST_ASSERT_TRUE(key_add(privdata->instances, (void*)res->name, vres)
>= 0);
}
static void test_repush_changed_value_takes_effect(void)
{
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[1];
fixture_init(&sched, &plugin);
limits[0] = make_limit_config("lim_a");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
limits[0]->val->val = 20.0;
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a"));
TEST_ASSERT_EQUAL_FLOAT(20.0, find_limit(&plugin, "lim_a")->val.val);
free_limit_config(limits[0]);
}
static void test_repush_adds_new_limit_after_existing(void)
{
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[2];
fixture_init(&sched, &plugin);
limits[0] = make_limit_config("lim_a");
limits[1] = make_limit_config("lim_b");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2));
TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a"));
TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_b"));
free_limit_config(limits[0]);
free_limit_config(limits[1]);
}
static void test_repush_removes_dropped_limit(void)
{
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[2];
limit_plugin_data_t* privdata;
fixture_init(&sched, &plugin);
privdata = (limit_plugin_data_t*)plugin.data;
limits[0] = make_limit_config("lim_a");
limits[1] = make_limit_config("lim_b");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2));
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, &limits[1], 1));
TEST_ASSERT_NULL(find_limit(&plugin, "lim_a"));
TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_b"));
TEST_ASSERT_EQUAL_INT(1, count_entries(privdata->limits));
free_limit_config(limits[0]);
free_limit_config(limits[1]);
}
/* A kept limit's instance is rebound to the new definition (its usage lives
* on in the resource manager under the same virtual resource); a removed
* limit's instance is dropped, so none points into a freed definition. */
static void test_repush_rebinds_kept_and_drops_removed_instances(void)
{
static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 };
static xce_resource_t res_b = { .name = "limit=lim_b", .idx = 8 };
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[2];
limit_plugin_data_t* privdata;
xce_vres_t* vres;
fixture_init(&sched, &plugin);
privdata = (limit_plugin_data_t*)plugin.data;
limits[0] = make_limit_config("lim_a");
limits[1] = make_limit_config("lim_b");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2));
add_instance(&plugin, &res_a, "lim_a");
add_instance(&plugin, &res_b, "lim_b");
limits[0]->val->val = 20.0;
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
vres = find_instance(&plugin, "limit=lim_a");
TEST_ASSERT_NOT_NULL(vres);
TEST_ASSERT_EQUAL_PTR(find_limit(&plugin, "lim_a"), vres->limit);
TEST_ASSERT_EQUAL_FLOAT(20.0, vres->limit->val.val);
TEST_ASSERT_EQUAL_PTR(&res_a, vres->vres);
TEST_ASSERT_NULL(find_instance(&plugin, "limit=lim_b"));
TEST_ASSERT_EQUAL_INT(1, count_entries(privdata->instances));
free_limit_config(limits[0]);
free_limit_config(limits[1]);
}
/* A change that alters the instance's identity (here per_user: the vres
* name now carries the user, so the old shared instance no longer names
* the usage the limit counts) drops the instance; the next job match
* instantiates the right one. */
static void test_repush_scope_change_drops_instance(void)
{
static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 };
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[1];
fixture_init(&sched, &plugin);
limits[0] = make_limit_config("lim_a");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
add_instance(&plugin, &res_a, "lim_a");
limits[0]->per_user = 1;
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a"));
TEST_ASSERT_NULL(find_instance(&plugin, "limit=lim_a"));
free_limit_config(limits[0]);
}
/* A push that fails part-way (a duplicate name) leaves the previous limit
* set and its instances untouched instead of half-replacing them. */
static void test_repush_failure_keeps_previous_set(void)
{
static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 };
xce_scheduler_t sched;
xce_sched_plugin_t plugin;
xce_limit_config_t* limits[3];
xce_vres_t* vres;
fixture_init(&sched, &plugin);
limits[0] = make_limit_config("lim_a");
TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1));
add_instance(&plugin, &res_a, "lim_a");
limits[1] = make_limit_config("lim_c");
limits[2] = make_limit_config("lim_c");
limits[0]->val->val = 20.0;
TEST_ASSERT_EQUAL_INT(-EEXIST, push_limits(&plugin, limits, 3));
TEST_ASSERT_EQUAL_FLOAT(10.0, find_limit(&plugin, "lim_a")->val.val);
TEST_ASSERT_NULL(find_limit(&plugin, "lim_c"));
vres = find_instance(&plugin, "limit=lim_a");
TEST_ASSERT_NOT_NULL(vres);
TEST_ASSERT_EQUAL_PTR(find_limit(&plugin, "lim_a"), vres->limit);
free_limit_config(limits[0]);
free_limit_config(limits[1]);
free_limit_config(limits[2]);
}
int main(void)
{
int rc;
@ -522,6 +754,12 @@ int main(void)
RUN_TEST(test_get_configured_limits_handle_iterates_registered_limit);
RUN_TEST(test_get_configured_limits_registered_with_zero_limits);
RUN_TEST(test_limit_scopes_resolve_against_own_group_kind);
RUN_TEST(test_repush_changed_value_takes_effect);
RUN_TEST(test_repush_adds_new_limit_after_existing);
RUN_TEST(test_repush_removes_dropped_limit);
RUN_TEST(test_repush_rebinds_kept_and_drops_removed_instances);
RUN_TEST(test_repush_scope_change_drops_instance);
RUN_TEST(test_repush_failure_keeps_previous_set);
rc = UNITY_END();
xstring_table_fini();

30
src/scheduler/testsuite/test_repl_limit_table.c
View file

@ -81,6 +81,7 @@
#include "../src/plugin.c"
#include "../src/plugins/builtin_fcfs.c"
#include "../src/plugins/limit.c"
#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */
#include "../src/recording.c"
#include "../src/recording_session.c"
#include "../src/repl.c"
@ -586,6 +587,29 @@ static void add_configured_limit(xce_sched_plugin_t* plugin, const char* name)
free_limit_config(limits[0]);
}
/* two limits in ONE config push: a push replaces the configured set
* (bug-1967), so two add_configured_limit() calls leave only the second */
static void add_configured_limit_pair(xce_sched_plugin_t* plugin,
const char* name_a, const char* name_b)
{
xce_limit_config_t* limits[2];
xce_conf4sched_t config;
int rc;
limits[0] = make_limit_config(name_a);
limits[1] = make_limit_config(name_b);
memset(&config, 0, sizeof(config));
config.nlimits = 2;
config.limits = limits;
rc = limit_update_config(plugin, &config);
TEST_ASSERT_EQUAL_INT(0, rc);
free_limit_config(limits[0]);
free_limit_config(limits[1]);
}
/*
* add_configured_limit_with_host_exclusion - task 2.2's own Requirement 1.5
* fixture: an "all except X" exclusion group on the HOST dimension, the
@ -1216,8 +1240,7 @@ static void test_info_limit_all_displays_every_configured_limits_setting(void)
TEST_ASSERT_EQUAL_INT(0, rc);
register_limit_plugin(&sim, &plugin, &group_sched);
add_configured_limit(&plugin, "limitA");
add_configured_limit(&plugin, "limitB");
add_configured_limit_pair(&plugin, "limitA", "limitB");
captured = capture_info_limit(&sim, "all");
@ -1965,8 +1988,7 @@ test_info_limit_dispatch_all_reaches_repl_info_limit_via_real_dispatch(void)
TEST_ASSERT_EQUAL_INT(0, rc);
register_limit_plugin(&sim, &plugin, &group_sched);
add_configured_limit(&plugin, "limitA");
add_configured_limit(&plugin, "limitB");
add_configured_limit_pair(&plugin, "limitA", "limitB");
captured = capture_dispatch(&sim, "info limit all");

1
src/scheduler/testsuite/test_sim_reconcile_unaccounted.c
View file

@ -128,6 +128,7 @@
* limit_new_plugin()/limit_seed_config()), which are now real symbols
* defined by this include and would collide at link time if left in
* place. */
#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */
#include "../src/recording.c"
#include "../src/recording_session.c"
#include "../src/repl.c"