diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index 8a81cc9f..fe76dd34 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -9,6 +9,7 @@ #include "sched/types.h" #include "utils.h" #include "xcealloc.h" +#include "xlist.h" #include "xstring.h" /* @@ -50,13 +51,119 @@ * sched_get_user()/sched_get_queue()/sched_get_project()/parse_resreq() * verbatim, not reinventing them. * - * core_build_job() intentionally does not construct job->schedobj (the - * resreq-tag-dispatch allocation-context object make_sched_job() builds via - * its own make_sched_obj() helper): no picker hook reads job->schedobj - * (verified against every builtin_fcfs.c hook body), and schedobj/ - * allocation-context construction belongs to the allocation path - * (scheduler-allocation spec), out of this task's event-interface scope. - */ + * bug-1569 thread B correction to this comment's own prior claim: it used + * to say core_build_job() "intentionally does not construct job->schedobj" + * because "no picker hook reads job->schedobj" and construction was + * deferred to a separate not-yet-designed "scheduler-allocation spec". That + * reasoning covered the PICKER's needs correctly but missed the ALLOCATOR's + * (scheduler.c's sched_allocator() thread): it calls plugin_job_election() + * (plugin.c) and sched_make_allocation() directly on job->schedobj with no + * NULL check, exactly as schedobj.c's own make_sched_job()-built jobs + * always provided a non-NULL one. Since NO deferred "scheduler-allocation + * spec" construction step exists anywhere in the live codebase, every job + * built via this JOBS_PUSH path has had job->schedobj == NULL since the + * scheduler-core rewrite -- masked entirely by the separate picker- + * candidate-registration gap (this same bug's thread A cause, scheduler.c's + * sched_init_queues(), now fixed) that meant no such job had ever reached + * the allocator to trigger it: live-confirmed as a reproducible SIGSEGV in + * limit_job_election() (plugins/limit.c's obj->configver dereference) the + * moment a real job first reached sched_allocator() after that fix landed. + * core_build_schedobj() below (+ its 3 tag-dispatch helpers) is the same + * core-scoped port of schedobj.c's make_sched_obj()/make_primary_schedobj()/ + * make_compound_schedobj()/make_alter_schedobj() this file's own header + * comment already establishes as the pattern for make_sched_job() itself + * (make_sched_obj()'s `xce_scheduler_t* sched` parameter is dead in its own + * body -- verified, never dereferenced -- so this port drops it rather than + * threading an unused core-boundary violation through). */ +static void core_build_primary_schedobj(xce_sched_obj_t* obj, + xce_primary_resreq_t* resreq) +{ + xce_primary_schedobj_t* primary + = (xce_primary_schedobj_t*)calloc(1, sizeof(*primary)); + + obj->tag = XCE_RESREQ_PRIMARY; + if (primary == NULL) { + return; + } + + primary->resreq = resreq; + primary->reason = NULL; + primary->owner = obj; + + obj->u.primary = primary; +} + +static void core_build_compound_schedobj(xce_sched_obj_t* obj, + xce_compound_resreq_t* resreq) +{ + xce_compound_schedobj_t* compound + = (xce_compound_schedobj_t*)calloc(1, sizeof(*compound)); + + obj->tag = XCE_RESREQ_COMPOUND; + if (compound == NULL) { + return; + } + + compound->resreq = resreq; + compound->owner = obj; + + obj->u.compound = compound; +} + +static void core_build_alter_schedobj(xce_sched_obj_t* obj, + xce_alter_resreq_t* resreq) +{ + int i; + + obj->tag = XCE_RESREQ_ALTER; + xlist_init(&(obj->u.alters), 4); + + for (i = 0; i < resreq->compounds.count; i++) { + xce_compound_schedobj_t* cobj = NULL; + xce_compound_resreq_t* cresreq = (xce_compound_resreq_t*)xlist_get( + &(resreq->compounds), i); + + if (cresreq == NULL) { + continue; + } + + cobj = (xce_compound_schedobj_t*)calloc(1, sizeof(*cobj)); + if (cobj == NULL) { + continue; + } + + cobj->resreq = cresreq; + cobj->owner = obj; + + xlist_store(&(obj->u.alters), cobj); + } +} + +static void core_build_schedobj(xce_sched_obj_t* obj, xce_job_t* j, + xce_resreq_t* resreq) +{ + obj->user = makeString(j->submit->user); + obj->queue = makeString(j->submit->queue); + obj->project = makeString(j->submit->project_name); + + xlist_init(&(obj->glimits), 8); + zarray_init(&(obj->hlimits), 32, NULL); + + switch (resreq->tag) { + case XCE_RESREQ_PRIMARY: + core_build_primary_schedobj(obj, resreq->u.primary); + break; + case XCE_RESREQ_COMPOUND: + core_build_compound_schedobj(obj, resreq->u.compound); + break; + case XCE_RESREQ_ALTER: + core_build_alter_schedobj(obj, resreq->u.alter); + break; + } + + obj->configver = 0; +} + static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j) { xce_sched_job_t* job; @@ -105,6 +212,14 @@ static xce_sched_job_t* core_build_job(xce_sched_core_t* core, xce_job_t* j) return NULL; } + job->schedobj = (xce_sched_obj_t*)calloc(1, sizeof(*(job->schedobj))); + if (job->schedobj == NULL) { + FREEUP(job->resreq); + FREEUP(job); + return NULL; + } + core_build_schedobj(job->schedobj, j, job->resreq); + job->mintasks = j->submit->mintasks; job->maxtasks = j->submit->maxtasks; job->priority = 0; /* FIXME: support job level priority (mirrors diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index 46a7804c..e13a4abd 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -68,8 +68,22 @@ int put_resvals(zarray_t* array, int nres, int* idxs, float* vals) return rc; } + /* bug-1569 thread B (3rd stacked cause): idx 0 is a legitimate resource + * index (e.g. "slots", almost always the FIRST resource registered -- + * confirmed live, resmgr assigns it idx 0), not a sentinel -- get_resvals() + * above has no such guard, and zarray_t is a plain 0-based index array + * throughout this codebase (core_events.c's zarray_put(&worker->resvals, + * r->idx, ...), same convention). `<= 0` skipped idx 0 here, leaving + * xvals[0] NULL for the "create a zero-value entry if missing" step + * below -- live-confirmed as a reproducible SIGSEGV (gdb: `xvals[i]->u. + * fvalue = vals[i]` on a NULL xvals[0]) the moment a job's resource + * charge-on-dispatch first reached this function, since no job had ever + * been genuinely dispatched before this bug's other two stacked causes + * (scheduler.c's sched_init_queues() picker-registration gap; + * core_events.c's core_build_job() never constructing job->schedobj) + * were fixed. */ for (i = 0; i < nres; i++) { - if (idxs[i] <= 0) { + if (idxs[i] < 0) { continue; } @@ -87,6 +101,12 @@ int put_resvals(zarray_t* array, int nres, int* idxs, float* vals) } for (i = 0; i < nres; i++) { + if (xvals[i] == NULL) { + /* idxs[i] < 0 (skipped above) or the calloc() above failed -- + * either way there is nowhere to write this value. */ + continue; + } + xvals[i]->u.fvalue = vals[i]; } diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index 5bd11781..06b90b63 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -866,14 +866,56 @@ static int sched_init_sharedInst(xce_scheduler_t* sched) static int sched_init_queues(xce_scheduler_t* sched) { - int i; - xce_conf4sched_t* config = sched->core.config; + int i; + xce_conf4sched_t* config = sched->core.config; + xce_sched_plugin_t* picker = NULL; + int rc; + + /* bug-1569 thread B: this is the scheduler's ONE-TIME synchronous + * initial-config-push path (handler.c's scheduler_req_handler, + * Request_body_configpushmsg -> sched_init_config() -> here), still on + * the pre-scheduler-core-rewrite plugin_queue_update() fan-out, which + * only reaches a plugin's OLD job_elect_fn-era `.hooks.queue_hook` + * (plugin.c's plugin_queue_update() body -- never touches + * `picker_iface`). The modern default picker (builtin_fcfs) has no + * `.hooks` at all -- it is driven entirely by `picker_iface`, whose + * on_queue_update() is what actually populates its own per-queue + * candidate registry (builtin_fcfs.c's fcfs_plugin_data_t.queues, + * consulted by next_job()). Every OTHER queue-registration path in this + * codebase (core_events.c's core_upsert_queue(), used by both + * CONFIG_PUSH for a later config upgrade and QUEUE_UPDATE) already + * fires `picker->picker_iface->on_queue_update()` -- this was the one + * remaining call site still skipping it, so with builtin_fcfs active + * every queue from the cluster's initial config was invisible to the + * picker forever: next_job() found an empty candidate set on every + * session from process start, so no job -- regardless of resources, + * regardless of readiness -- could ever be scheduled or even PEND with + * a picker-produced reason. Firing on_queue_update() here too (best + * effort: a picker resolution failure here would already have failed + * sched_init()'s own earlier synchronous resolve-and-validate gate, so + * this mirrors core_upsert_queue()'s same "log and continue on failure" + * posture rather than aborting config load over it) closes that gap; + * legacy job_elect_fn-only plugins are unaffected -- plugin_register() + * always wraps them in legacy_picker_wrapper_iface, whose + * on_queue_update is a real, harmless no-op + * (legacy_picker_queue_update_noop). */ + rc = sched_resolve_job_picker(&(sched->core), &picker); + if (rc < 0) { + WARN("%s failed to resolve job picker plugin (rc=%d); queues will " + "be invisible to the picker until the next config update", + __func__, rc); + picker = NULL; + } for (i = 0; i < config->n_queues; i++) { xce_sched_queue_t* q = make_sched_queue(sched, config->queues[i]); plugin_queue_update(sched, q); sched_add_queue(&(sched->core), q); + + if (picker != NULL) { + picker->picker_iface->on_queue_update(picker, q); + } } return 0;