fix(scheduler): bug-1569 three stacked crashes blocking ALL job dispatch
No job could ever be scheduled in normal mode -- every round produced decision=(nil), forever. Three independent bugs were stacked on the same symptom, each masking the next (only reachable once the prior one was fixed): 1. scheduler.c's sched_init_queues() -- the scheduler's one-time synchronous initial-config-push handler -- only fired the legacy job_elect_fn-era plugin_queue_update() fan-out. builtin_fcfs (the default picker) has no .hooks, only picker_iface, so on_queue_update() was never called for any queue from the initial cluster config. Its own candidate registry stayed empty forever, so next_job() found zero candidates on every session. 2. Once #1 was fixed and a job first reached the allocator, core_events.c's core_build_job() never constructed job->schedobj -- a documented but incorrect assumption from the scheduler-core rewrite ("no picker hook reads job->schedobj... belongs to a separate allocation spec" -- no such spec exists in the live code). sched_allocator() dereferences it directly with no NULL check; reproduced live as a SIGSEGV in limit_job_election(). 3. Once #2 was fixed and a job reached genuine dispatch, resource.c's put_resvals() crashed during post-dispatch resource charging: `idxs[i] <= 0` treated resource index 0 ("slots", almost always the first registered resource) as an invalid sentinel, leaving its value slot NULL for an unconditional write two lines later. All three root-caused via gdb `bt full` on live SIGSEGVs, not guesswork, and live-verified end-to-end: a 3-element job array now runs PEND -> DISPATCH -> RUN -> DONE with the scheduler staying alive throughout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
c672e3c2bf
commit
7041d988d1
3 changed files with 187 additions and 10 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue