From 601b245e5a27078dd542c196638604cbe789ad82 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 25 Sep 2026 13:27:08 +0000 Subject: [PATCH] fix(scheduler): a failed COMPOUND leaves no partial allocation (bug-1766) When a later primary of a COMPOUND failed, sched_make_compound_alloc_ still moved the earlier primaries' merged allocation onto the schedobj, so a job decided PEND carried a partial allocation that the next successful round overwrote and leaked. The merged allocation is now published only when every primary succeeds and freed otherwise. No resource-state rollback is needed: the primaries' uncommitted debits live in ctx.uncomitted, per-attempt scratch freed by sched_ctx_fini(), and schedobj->alloc's only reader is the dispatch path. The ALTER loop's per-alternative cleanup (added with bug-1769) is now redundant and removed. Test: "1*{..} + 5*{..}" on 2x2 slots PENDs with schedobj->alloc NULL (held the first primary's allocation before). Scheduler suite 87/87. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/scheduler/src/scheduler.c | 17 ++++++----- .../testsuite/test_parjob_e2e_multislot.c | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index 59d30aeb..269a4159 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -315,8 +315,17 @@ static int sched_make_compound_alloc_(xce_sched_core_t* core, } } + /* bug-1766: a compound is allocated only if EVERY primary is -- when a + * later primary fails, drop the earlier primaries' allocation instead + * of leaving a partial one on obj. Their uncommitted debits live in + * ctx.uncomitted, per-attempt scratch freed by sched_ctx_fini(), so + * nothing else needs undoing. */ if (merged != NULL) { - MOVE_PTR(obj->alloc, merged); + if (rc > 0) { + MOVE_PTR(obj->alloc, merged); + } else { + free_JobAlloc_ptr(&merged); + } } if (ctx.reason != NULL) { @@ -356,12 +365,6 @@ static int sched_make_alter_allocation(xce_sched_core_t* core, if (rc > 0) { return rc; } - - /* a failed alternative may leave a partial allocation (no - * rollback yet, bug-1766) -- drop it before trying the next one */ - if (obj->alloc != NULL) { - free_JobAlloc_ptr(&(obj->alloc)); - } } /* FIXME: merge pend reason from all children */ diff --git a/src/scheduler/testsuite/test_parjob_e2e_multislot.c b/src/scheduler/testsuite/test_parjob_e2e_multislot.c index 62dac25a..d438d84b 100644 --- a/src/scheduler/testsuite/test_parjob_e2e_multislot.c +++ b/src/scheduler/testsuite/test_parjob_e2e_multislot.c @@ -847,6 +847,34 @@ static void test_compound_primaries_on_same_host_merge(void) "entry"); } +/* + * bug-1766: when a later primary of a COMPOUND fails, the earlier + * primaries' (merged) allocation was still moved onto the schedobj, so a + * job reported PEND carried a partial allocation that the next successful + * round overwrote and leaked. A failed compound must leave no allocation + * (its uncommitted debits are per-attempt scratch, freed with the ctx). + */ +static void test_failed_compound_leaves_no_partial_allocation(void) +{ + xce_scheduler_t* sched = start_sched(); + xce_decision_t* decision; + xce_sched_job_t* sjob; + + e2e_push_queue_and_workers(sched, "cmp4_queue"); + decision = e2e_dispatch_resreq(sched, JOB_ID + 7, "cmp4_queue", + "1*{rusage[mem=1]} + 5*{rusage[mem=1]}"); + TEST_ASSERT_NOT_NULL(decision); + TEST_ASSERT_EQUAL_INT_MESSAGE(XCE_DECISION_PEND, decision->op, + "the 5-task primary cannot fit on 4 slots"); + + sjob = sched_get_job(&(sched->core), JOB_ID + 7); + TEST_ASSERT_NOT_NULL(sjob); + TEST_ASSERT_NULL_MESSAGE(sjob->schedobj->alloc, + "a failed compound must not keep the first primary's partial " + "allocation"); + TEST_ASSERT_NULL(sjob->allocs); +} + int main(void) { UNITY_BEGIN(); @@ -858,5 +886,6 @@ int main(void) RUN_TEST(test_alter_falls_back_to_alternative_that_fits); RUN_TEST(test_compound_allocation_includes_every_primary); RUN_TEST(test_compound_primaries_on_same_host_merge); + RUN_TEST(test_failed_compound_leaves_no_partial_allocation); return UNITY_END(); }