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(); }