diff --git a/src/scheduler/src/plugins/parjob_allocator.c b/src/scheduler/src/plugins/parjob_allocator.c index 7caabe07..ace0a58b 100644 --- a/src/scheduler/src/plugins/parjob_allocator.c +++ b/src/scheduler/src/plugins/parjob_allocator.c @@ -98,12 +98,11 @@ static int parjob_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) return -EINVAL; } - if (obj->tag != XCE_RESREQ_PRIMARY) { - /* we just take care of parallel job */ - return 0; - } - - primary = obj->u.primary->resreq; + /* bug-1768: the primary being allocated is ctx->primary -- for a + * PRIMARY obj it is obj->u.primary->resreq, and for a COMPOUND/ALTER + * obj it is the nested primary sched_ctx_set_primary() just set. Gating + * on obj->tag skipped every nested primary's slots/rusage check. */ + primary = ctx->primary; if (primary == NULL) { return -EINVAL; } diff --git a/src/scheduler/src/plugins/seqjob_allocator.c b/src/scheduler/src/plugins/seqjob_allocator.c index aabc2e10..433a67db 100644 --- a/src/scheduler/src/plugins/seqjob_allocator.c +++ b/src/scheduler/src/plugins/seqjob_allocator.c @@ -61,12 +61,11 @@ static int seqjob_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) return -EINVAL; } - if (obj->tag != XCE_RESREQ_PRIMARY) { - /* we just take care of sequence job */ - return 0; - } - - primary = obj->u.primary->resreq; + /* bug-1768: the primary being allocated is ctx->primary -- for a + * PRIMARY obj it is obj->u.primary->resreq, and for a COMPOUND/ALTER + * obj it is the nested primary sched_ctx_set_primary() just set. Gating + * on obj->tag skipped every nested primary's slots/rusage check. */ + primary = ctx->primary; if (primary == NULL) { return -EINVAL; } diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index af954a40..3a7fe5d8 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -531,6 +531,11 @@ void sched_ctx_fini(xce_sched_ctx_t* ctx) void sched_ctx_set_primary(xce_sched_ctx_t* ctx, xce_primary_resreq_t* primary) { ctx->primary = primary; + /* bug-1768: a COMPOUND sets one primary after another on the same ctx; + * a host left over from the previous primary would make the prepare + * below compute a per-host want instead of this primary's job-level + * (shared) want */ + ctx->worker = NULL; /* scheduler-sim-live-allocation spec, task 2.1 (Core-Safe Allocation * Chain): ctx->sched is now honestly typed xce_sched_core_t* (set by diff --git a/src/scheduler/testsuite/test_parjob_e2e_multislot.c b/src/scheduler/testsuite/test_parjob_e2e_multislot.c index 364ba277..4df5839e 100644 --- a/src/scheduler/testsuite/test_parjob_e2e_multislot.c +++ b/src/scheduler/testsuite/test_parjob_e2e_multislot.c @@ -701,6 +701,102 @@ static void test_alter_resreq_job_dispatches_via_real_allocator(void) TEST_ASSERT_EQUAL_UINT64(jobid, decision->u.alloc->jobid); } +/* bug-1768 helpers: register a queue + the two 2-slot workers (4 slots + * total) through real ingest, push one job, begin a session. */ +static void e2e_push_queue_and_workers(xce_scheduler_t* sched, + const char* qname) +{ + xce_core_event_t ev; + xce_worker_t* worker_ptrs[2]; + + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_QUEUE_UPDATE; + ev.u.queue_update.queue = make_wire_queue(qname, 1); + TEST_ASSERT_TRUE(xce_core_push_events(&(sched->core), &ev, 1) >= 0); + + worker_ptrs[0] = make_wire_worker(WORKER_A_NAME, WORKER_SLOTS); + worker_ptrs[1] = make_wire_worker(WORKER_B_NAME, WORKER_SLOTS); + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_WORKERS_PUSH; + ev.u.workers_push.nworkers = 2; + ev.u.workers_push.workers = worker_ptrs; + TEST_ASSERT_TRUE(xce_core_push_events(&(sched->core), &ev, 1) >= 0); +} + +static xce_decision_t* e2e_dispatch_resreq(xce_scheduler_t* sched, + uint64_t jobid, const char* qname, + const char* resreq) +{ + xce_core_event_t ev; + xce_job_t* job_ptrs[1]; + + job_ptrs[0] = make_push_job(jobid, (char*)resreq, "e2e_user", + (char*)qname, "e2e_proj", 1, 1); + memset(&ev, 0, sizeof(ev)); + ev.type = XCE_CORE_EVENT_JOBS_PUSH; + ev.u.jobs_push.njobs = 1; + ev.u.jobs_push.jobs = job_ptrs; + TEST_ASSERT_TRUE(xce_core_push_events(&(sched->core), &ev, 1) >= 0); + TEST_ASSERT_NOT_NULL(sched_get_job(&(sched->core), jobid)); + TEST_ASSERT_TRUE(xce_core_begin_session(&(sched->core)) >= 0); + + return dispatch_and_wait(sched, jobid); +} + +static double e2e_alloc_total_slots(xce_job_alloc_t* alloc) +{ + double total = 0.0; + int i, j; + + for (i = 0; i < alloc->nhosts; i++) { + for (j = 0; j < alloc->allocs[i]->nallocs; j++) { + xce_alloc_t* a = alloc->allocs[i]->allocs[j]; + + if ((a->resname != NULL) && (strcmp(a->resname, "slots") == 0)) { + total += a->value; + } + } + } + return total; +} + +/* + * bug-1768: seqjob/parjob prepare_allocation no-op'd whenever the schedobj + * was not PRIMARY, so a primary nested in a COMPOUND (or ALTER) never had + * its slots/rusage checked -- a 5-task compound dispatched onto 4 slots. + */ +static void test_compound_primary_needing_more_slots_than_exist_pends(void) +{ + xce_scheduler_t* sched = start_sched(); + xce_decision_t* decision; + + e2e_push_queue_and_workers(sched, "cmp_queue"); + decision = e2e_dispatch_resreq(sched, JOB_ID + 3, "cmp_queue", + "5*{rusage[mem=1]}"); + TEST_ASSERT_NOT_NULL(decision); + TEST_ASSERT_EQUAL_INT_MESSAGE(XCE_DECISION_PEND, decision->op, + "5 tasks cannot fit on 2 x 2 slots -- the nested primary's slot " + "want must be checked"); +} + +/* bug-1768 + bug-1769: the first ALTER alternative cannot fit, so the + * allocation must fall back to the second (1 slot), not claim the first. */ +static void test_alter_falls_back_to_alternative_that_fits(void) +{ + xce_scheduler_t* sched = start_sched(); + xce_decision_t* decision; + + e2e_push_queue_and_workers(sched, "alt2_queue"); + decision = e2e_dispatch_resreq(sched, JOB_ID + 4, "alt2_queue", + "{5*{rusage[mem=1]}} || {1*{rusage[mem=1]}}"); + TEST_ASSERT_NOT_NULL(decision); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_DISPATCH, decision->op); + TEST_ASSERT_NOT_NULL(decision->u.alloc); + TEST_ASSERT_EQUAL_FLOAT_MESSAGE(1.0f, + (float)e2e_alloc_total_slots(decision->u.alloc), + "the job must be placed by the second alternative (1 slot)"); +} + int main(void) { UNITY_BEGIN(); @@ -708,5 +804,7 @@ int main(void) test_multihost_parallel_job_allocation_spans_two_hosts_via_real_dispatch); RUN_TEST(test_dispatch_of_job_with_unregistered_queue_does_not_crash); RUN_TEST(test_alter_resreq_job_dispatches_via_real_allocator); + RUN_TEST(test_compound_primary_needing_more_slots_than_exist_pends); + RUN_TEST(test_alter_falls_back_to_alternative_that_fits); return UNITY_END(); }