feat(scheduler-allocation): add per-host resource-debit reversal

Adds sched_ctx_uncommit_host() (resource.c), the structural inverse
of sched_try_alloc_on_host_()'s existing XVEC_ADD_FLOAT commit call,
scoped to ctx->uncomitted only per requirements.md's Rollback scope
discussion. Test exercises the real sched_ctx_get_uncomit_host()
(schedobj.c) via white-box #include rather than a private
reimplementation, matching test_picker_config_fields.c's established
precedent.

Task 1.4 (scheduler-allocation spec). Requirement 4.2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
This commit is contained in:
Rongsong Shen 2026-09-04 16:06:59 +00:00
parent d8b5cf8718
commit d0024eb7e8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
4 changed files with 171 additions and 34 deletions

View file

@ -16,7 +16,7 @@
- Add an allocation-mode field to the per-attempt allocation context, defaulting to normal-resources-only
- Observable: the field exists, defaults correctly, and no existing allocation code path branches on any value other than the default
- _Requirements: 9.5_
- [ ] 1.4 Add the per-host resource-debit reversal primitive
- [x] 1.4 Add the per-host resource-debit reversal primitive
- Add a helper that reverses exactly the provisional resource debit an earlier successful host-claim made, using the existing subtraction primitive
- Write a unit test that debits a host's provisional usage, reverses it, and confirms the host's provisional usage returns to its pre-debit value
- Observable: a passing test shows debit-then-reverse leaves provisional usage unchanged from baseline
@ -106,5 +106,6 @@
## Implementation Notes
- (1.4) When a new scheduler test needs a real function from `schedobj.c`/`core.c` (e.g. `sched_ctx_get_uncomit_host()`), white-box `#include` the real source file rather than privately reimplementing its logic in the test — established precedent: `test_picker_config_fields.c`'s `#include "../src/core.c"` + `#include "../src/schedobj.c"` pairing. A private reimplementation may pass today but silently drifts from the real function with nothing to catch it later.
- (1.1) `xce_host_iter_t.next()` returns `xce_sched_worker_t*` (`sched/types.h`, carries `resvals`/`resInUse`/`host_eval_ctx`) — never `xce_worker_t*` (`xce/types.h`, the wire/config type `make_sched_worker()` converts FROM). requirements.md/design.md briefly had this wrong; fixed in commit `d1a3e158`. Task 2.1 (`host_iter.c`'s real implementation) and any later task touching worker candidates must use `xce_sched_worker_t*` from the start.
- (1.2) `resreq_fp`'s ingest-time population lives in `core_build_primary_schedobj()` (`src/scheduler/src/core_events.c`), right after `primary->resreq = resreq;` — confirmed wired (min_tasks/max_tasks/select_node_id/rusage_hash all set from the parsed `xce_primary_resreq_t`) as of task 1.2 round 2, regression-covered by `test_core_build_primary_schedobj_populates_resreq_fp_from_parsed_resreq` in `src/scheduler/testsuite/test_core_events.c`. Round 1 added the type/field/hash machinery but never wired this call site, leaving `resreq_fp` permanently zeroed — do not rediscover this gap in a later task.

View file

@ -198,6 +198,7 @@ extern void sched_ctx_fini(xce_sched_ctx_t*);
extern void sched_ctx_set_primary(xce_sched_ctx_t*, xce_primary_resreq_t*);
extern int sched_ctx_get_uncomit_shared(xvec_t**, xce_sched_ctx_t*);
extern int sched_ctx_get_uncomit_host(xvec_t**, xce_sched_ctx_t*, int);
extern int sched_ctx_uncommit_host(xce_sched_ctx_t*, int, xvec_idval_t*);
extern void reset_sched_obj(xce_sched_obj_t*);
#endif

View file

@ -557,6 +557,44 @@ int sched_try_alloc_on_hosts(xce_worker_group_t* g, va_list ap)
return sched_try_alloc_on_host_(w, ap);
}
/*
* sched_ctx_uncommit_host - reverses exactly one provisional per-host
* resource debit a prior, successful sched_try_alloc_on_host_() call made
* for host `idx`, undoing the XVEC_ADD_FLOAT commit
* (`xvec_idval(uncomit, &(ctx->alloc_req.host), XVEC_ADD_FLOAT)`, above)
* with the exact inverse operation against the SAME per-host uncommitted
* vector (`ctx->uncomitted.hosts[idx]`, via sched_ctx_get_uncomit_host() --
* the same get-or-create accessor the commit path itself uses, so a
* reversal always lands on the identical xvec_t instance the debit did).
*
* `committed` must be the exact xvec_idval_t snapshot the earlier commit
* added (e.g. the `claimed[]` entry's saved `alloc_req.host`, per
* requirements.md's "Rollback scope" -- ctx->alloc_req is reset and
* repopulated for every subsequent host tried, so callers must retain
* their own copy rather than reading ctx->alloc_req.host after the fact).
*
* Scoped entirely to ctx->uncomitted (per-job-allocation-attempt scratch
* state) -- never touches the real, session-wide resInUse/sharedInUse
* books (see resource-accounting.md / requirements.md's "Rollback scope").
*
* Returns the same contract as xvec_idval(): >=0 (count of entries
* applied) on success, negative errno on failure (e.g. -EINVAL if ctx/idx
* are invalid, propagated from sched_ctx_get_uncomit_host()).
*/
int sched_ctx_uncommit_host(xce_sched_ctx_t* ctx, int idx,
xvec_idval_t* committed)
{
xvec_t* uncomit = NULL;
int rc;
rc = sched_ctx_get_uncomit_host(&uncomit, ctx, idx);
if (rc < 0) {
return rc;
}
return xvec_idval(uncomit, committed, XVEC_SUB_FLOAT);
}
static int sched_update_account(xce_sched_core_t* core,
xce_job_account_t* account, int charge)
{

View file

@ -45,19 +45,31 @@
*
* White-box #include of ../src/core.c (for the real
* xce_sched_core_init()/sched_add_worker()/sched_get_worker() the fixture
* needs) and ../src/resource.c (the functions under test). Neither file
* needs core_clock.c/core_events.c/core_session.c/decision.c/plugin.c --
* this test never calls xce_core_push_events()/xce_core_begin_session()/
* anything decision- or plugin-registration-related, only the plain
* add-worker/charge-account entry points core.c/resource.c themselves
* define. Stubs below (kill_sched_* family, plugin_unregister) are the
* standard link-satisfying no-ops xce_sched_core_init()'s kvmgr datakillers
* reference (never actually invoked -- this fixture is never torn down,
* same convention as test_driver_session_ordering.c/
* test_snapshot_save_load.c's identical stub set).
* sched_worker_release_host_ctx() is the one real (non-stub) symbol this
* pulls from resource.c itself, not from core_session.c -- unused here
* (this test never calls it), listed only because resource.c defines it.
* needs), ../src/resource.c (the functions under test), and
* ../src/schedobj.c (for the real sched_ctx_get_uncomit_host() that the
* new sched_ctx_uncommit_host() under test itself calls -- white-box
* #included here rather than reimplemented, the same convention
* test_picker_config_fields.c's own #include "../src/core.c" +
* #include "../src/schedobj.c" pairing already established in this
* directory "so it exercises the real functions, not a re-implementation
* of any of them"). None of the three needs core_clock.c/core_events.c/
* core_session.c/decision.c/plugin.c -- this test never calls
* xce_core_push_events()/xce_core_begin_session()/anything decision- or
* plugin-registration-related, only the plain add-worker/charge-account/
* uncommit-host entry points core.c/resource.c/schedobj.c themselves
* define. The kill_sched_* family (the kvmgr datakillers
* xce_sched_core_init() references) is now the REAL implementation
* pulled in from schedobj.c, not a stub -- only plugin_unregister remains
* a link-satisfying no-op (plugin.c is still not white-box #included).
* sched_worker_release_host_ctx() is a real (non-stub) symbol this pulls
* from resource.c itself, not from core_session.c -- unused here (this
* test never calls it), listed only because resource.c defines it.
* plugin_job_update()/plugin_queue_update()/plugin_job_rusage_update()
* below are new link-satisfying stubs for schedobj.c's OWN unrelated
* functions (sched_update_existing_queue()/sched_update_existing_job()/
* sched_apply_job_rusage()) that reference them (plugin.c, still not
* included) -- never actually invoked, since this file never calls any
* of those three schedobj.c functions.
*/
#include <errno.h>
#include <stdlib.h>
@ -70,28 +82,26 @@
#include "sim.h"
/* Pull in the real xce_sched_core_init()/sched_add_worker()/
* sched_get_worker() (core.c) and sched_charge_account()/
* sched_uncharge_account()/sched_commit_job_alloc()/get_resvals() (resource.c,
* the functions under test) into this TU (white-box #include, same
* convention as every other file in this directory). */
* sched_get_worker() (core.c), sched_charge_account()/
* sched_uncharge_account()/sched_commit_job_alloc()/get_resvals()/
* sched_ctx_uncommit_host() (resource.c, the functions under test), and
* sched_ctx_get_uncomit_host() (schedobj.c) into this TU (white-box
* #include, same convention as every other file in this directory --
* see this file's own header comment for why schedobj.c specifically). */
#include "../src/core.c"
#include "../src/resource.c"
#include "../src/schedobj.c"
void kill_sched_job(void* arg) { (void)arg; }
void kill_sched_queue(void* arg) { (void)arg; }
void kill_sched_worker(void* arg) { (void)arg; }
void kill_sched_user(void* arg) { (void)arg; }
void kill_sched_project(void* arg) { (void)arg; }
void kill_sched_resval(xce_val_t* val) { (void)val; }
void plugin_unregister(void* arg) { (void)arg; }
/* Link-satisfying stubs for resource.c's sched_try_alloc_on_host_() (its
* OWN callees, plugin_prepare_allocation()/sched_ctx_get_uncomit_host(),
* live in plugin.c/schedobj.c -- neither white-box #included here). Never
* actually invoked: this file never calls sched_try_alloc_on_hosts()/
/* Link-satisfying stub for resource.c's sched_try_alloc_on_host_() (its
* OWN callee plugin_prepare_allocation() lives in plugin.c, not white-box
* #included here; its other callee, sched_ctx_get_uncomit_host(), is now
* the real function pulled in from schedobj.c above). Never actually
* invoked: this file never calls sched_try_alloc_on_hosts()/
* sched_try_alloc_on_host_(), only sched_charge_account()/
* sched_uncharge_account()/sched_commit_job_alloc(). Same two stubs
* test.c's own #include "../src/resource.c" already establishes needing. */
* sched_uncharge_account()/sched_commit_job_alloc(). Same stub test.c's
* own #include "../src/resource.c" already establishes needing. */
int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c)
{
(void)s;
@ -99,11 +109,19 @@ int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c)
return 0;
}
int sched_ctx_get_uncomit_host(xvec_t** v, xce_sched_ctx_t* c, int i)
/* Link-satisfying stubs for schedobj.c's OWN unrelated functions
* (sched_update_existing_queue()/sched_update_existing_job()/
* sched_apply_job_rusage()) that reference plugin_job_update()/
* plugin_queue_update()/plugin_job_rusage_update() (plugin.c, not
* white-box #included here). Never actually invoked: this file never
* calls any of those three schedobj.c functions. */
int plugin_job_update(void* s, xce_sched_job_t* j) { (void)s; (void)j; return 0; }
int plugin_queue_update(void* s, xce_sched_queue_t* q) { (void)s; (void)q; return 0; }
int plugin_job_rusage_update(void* s, xce_sched_job_t* j, xce_job_rusages_t* r)
{
(void)v;
(void)c;
(void)i;
(void)s;
(void)j;
(void)r;
return 0;
}
@ -720,6 +738,84 @@ static void test_reconcile_job_resource_usage_null_args_return_einval(void)
sched_reconcile_job_resource_usage(NULL, NULL, NULL));
}
/*
* test_ctx_uncommit_host_reverses_provisional_debit - Requirement 4.2
* (scheduler-allocation spec, task 1.4): sched_ctx_uncommit_host()
* (resource.c, new) reverses exactly the provisional per-host resource
* debit an earlier successful host-claim made.
*
* Debits ctx->uncomitted.hosts[idx] directly via xvec_idval(...,
* XVEC_ADD_FLOAT) -- the exact call sched_try_alloc_on_host_() makes on a
* successful host claim (resource.c line ~544) -- rather than driving the
* full plugin_prepare_allocation()/sched_check_host_satisfy() chain, which
* needs a real worker/select-expression/resmgr fixture unrelated to what
* this rollback primitive itself does (this file's own convention: see
* make_test_account()'s residx-only shortcut above for the same rationale).
* Then calls the new sched_ctx_uncommit_host() and asserts the per-host
* uncommitted vector is back to its pre-debit (zero) baseline -- proving
* debit-then-reverse leaves provisional usage unchanged, this task's
* Observable.
*
* sched_ctx_get_uncomit_host() (the baseline/re-read get-or-create
* accessor called below) and host_uncommit_killer() (the zarray item
* killer passed to zarray_init() below) are both the REAL functions
* white-box #included from schedobj.c above, not reimplementations --
* host_uncommit_killer() is `static` in schedobj.c, but the #include
* pastes it into this same translation unit, so it resolves normally.
* This is the same "exercise the real functions, not a re-implementation
* of any of them" precedent test_picker_config_fields.c already
* established for combining core.c + schedobj.c in one white-box test.
*/
#define UNCOMMIT_HOST_IDX 3
#define UNCOMMIT_RESIDX 0
#define UNCOMMIT_DEBIT 2.5f
static void test_ctx_uncommit_host_reverses_provisional_debit(void)
{
xce_sched_ctx_t ctx;
xvec_t* uncomit = NULL;
xvec_idval_t committed;
float* data;
int rc;
memset(&ctx, 0, sizeof(ctx));
rc = zarray_init(&(ctx.uncomitted.hosts), 8,
(zarray_item_killer_t)host_uncommit_killer);
TEST_ASSERT_EQUAL_INT(0, rc);
rc = xvec_idval_init(&committed, 4);
TEST_ASSERT_EQUAL_INT(0, rc);
rc = xvec_idval_store(&committed, UNCOMMIT_RESIDX, UNCOMMIT_DEBIT);
TEST_ASSERT_TRUE(rc >= 0);
/* Baseline: a freshly created per-host uncommitted vector reads 0. */
rc = sched_ctx_get_uncomit_host(&uncomit, &ctx, UNCOMMIT_HOST_IDX);
TEST_ASSERT_EQUAL_INT(1, rc);
TEST_ASSERT_NOT_NULL(uncomit);
data = (float*)uncomit->data;
TEST_ASSERT_EQUAL_FLOAT(0.0f, data[UNCOMMIT_RESIDX]);
/* Debit: the exact commit call sched_try_alloc_on_host_() makes on a
* successful host claim. */
rc = xvec_idval(uncomit, &committed, XVEC_ADD_FLOAT);
TEST_ASSERT_EQUAL_INT(committed.count, rc);
TEST_ASSERT_EQUAL_FLOAT(UNCOMMIT_DEBIT, data[UNCOMMIT_RESIDX]);
/* Reverse: the new primitive under test. */
rc = sched_ctx_uncommit_host(&ctx, UNCOMMIT_HOST_IDX, &committed);
TEST_ASSERT_EQUAL_INT(committed.count, rc);
/* Back to the pre-debit baseline -- same vector instance, re-read. */
TEST_ASSERT_EQUAL_FLOAT(0.0f, data[UNCOMMIT_RESIDX]);
/* Same per-host vector instance is returned again on a second lookup
* (get-or-create semantics unchanged by the reversal). */
uncomit = NULL;
rc = sched_ctx_get_uncomit_host(&uncomit, &ctx, UNCOMMIT_HOST_IDX);
TEST_ASSERT_EQUAL_INT(1, rc);
TEST_ASSERT_EQUAL_FLOAT(0.0f, ((float*)uncomit->data)[UNCOMMIT_RESIDX]);
}
int main(void)
{
UNITY_BEGIN();
@ -737,5 +833,6 @@ int main(void)
RUN_TEST(test_reconcile_job_resource_usage_charges_unreserved_job_full_usage);
RUN_TEST(test_reconcile_job_resource_usage_noop_when_mem_not_a_registered_resource);
RUN_TEST(test_reconcile_job_resource_usage_null_args_return_einval);
RUN_TEST(test_ctx_uncommit_host_reverses_provisional_debit);
return UNITY_END();
}