fix(scheduler): sim mode force-dispatched recorded decisions past live capacity (bug-1836, bug-1838)
User-reported: replaying a captured session dispatched 44 single-slot jobs onto a 22-slot host. Root cause traced through two layers: - bug-1836: sim_reconcile_unaccounted_jobs() (the "Recorded-Decision Reconciliation" backstop) force-applied any recorded DISPATCH decision it found, with zero live capacity check. Added sim_reconciled_alloc_fits_capacity(), reusing the same sched_check_host_satisfy()/sched_check_shared_satisfy() admission checks the live picker/allocator path already uses. - bug-1838: even with that gate, reconciliation still preempted every freshly-PEND job before the live picker/allocator got a turn each round -- unnecessary now that scheduler-sim-live-allocation gave sim mode the same real, capacity-checked picker/allocator threads normal mode uses. Narrowed reconciliation's trigger back to JOB_IS_RUN-only (undoing a 2026-09-11 widening whose own justification no longer holds), so a freshly-pushed job now always flows through the same live, verified decision path as normal mode. Reconciliation now only ever fires for a job already RUN from before the replay's own observation window -- the one case structurally invisible to the live picker. Also logged (bug-1837, not fixed here, needs its own design decision): sim_reconcile_unaccounted_jobs() calls a picker's on_dispatch() hook from the driver thread instead of the allocator thread plugin.h documents it belonging to -- currently inert since the only real picker plugin happens to lock internally, but not contract-safe for a future lock-free one. Fixing bug-1838 required re-examining every test exercising the old PEND-inclusive trigger; several needed their fixtures corrected to register a real worker (harmless before any capacity check existed), and others needed their actual intent re-derived and their assertions inverted or restated now that a PEND job is never eligible for reconciliation. Dated correction notes added in place to the scheduler-sim-eager-reconciliation spec, matching its own existing 2026-09-11 correction-note precedent. Verified: full scheduler suite 85/85, independently re-confirmed by running the four most affected binaries directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
cd9ecb4159
commit
94115b0024
44 changed files with 2769 additions and 333 deletions
|
|
@ -113,6 +113,7 @@
|
|||
#include "resobj.h"
|
||||
#include "scheduler.h"
|
||||
#include "sim.h"
|
||||
#include "xcealloc.h"
|
||||
#include "xstring.h"
|
||||
|
||||
/* write_capnp_message (recording.c) has no public header declaration --
|
||||
|
|
@ -379,6 +380,95 @@ static void append_end_of_stream_marker(FILE* f)
|
|||
capn_free(&c);
|
||||
}
|
||||
|
||||
/* bug-1838 regression: same jobStateChange wire shape test_sim_contention_
|
||||
* parity.c's own append_job_state_change_event() establishes. */
|
||||
static void append_jobstatechange_event(FILE* f, uint64_t ts, uint64_t jobid,
|
||||
uint32_t old_state,
|
||||
uint32_t new_state)
|
||||
{
|
||||
job_state_change_event_t jsc;
|
||||
replay_event_t src;
|
||||
struct capn c;
|
||||
struct capn_segment* cs;
|
||||
ReplayEvent_ptr p;
|
||||
|
||||
memset(&jsc, 0, sizeof(jsc));
|
||||
jsc.jobid = jobid;
|
||||
jsc.old_state = old_state;
|
||||
jsc.new_state = new_state;
|
||||
jsc.exit_code = 0;
|
||||
|
||||
memset(&src, 0, sizeof(src));
|
||||
src.ts = ts;
|
||||
src.body_which = ReplayEvent_body_jobStateChange;
|
||||
src.body.job_state_change = &jsc;
|
||||
|
||||
capn_init_malloc(&c);
|
||||
cs = capn_root(&c).seg;
|
||||
encode_ReplayEvent_ptr(cs, &p, &src);
|
||||
capn_setp(capn_root(&c), 0, p.p);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(0, write_capnp_message(f, &c));
|
||||
capn_free(&c);
|
||||
}
|
||||
|
||||
/* run_sim_and_capture_stdout - same dup2(STDOUT_FILENO)-redirect technique
|
||||
* test_sim_verify.c's own copy establishes, to capture sim_log_mismatch()'s
|
||||
* ERROR()-logged "MISMATCH" text for a --sim-verify assertion. Takes the
|
||||
* caller's own static xce_sched_sim_driver_t* (never a stack-local one --
|
||||
* see this file's own header comment, "bug-1598/1632/1763 fixture-safety
|
||||
* requirement", for why: sched_sim_driver_run() spawns two real,
|
||||
* never-joined threads that keep dereferencing this pointer for the rest
|
||||
* of the process's life). */
|
||||
static char* run_sim_and_capture_stdout(xce_sched_sim_driver_t* sim,
|
||||
const char* path, bool verify,
|
||||
int* out_rc)
|
||||
{
|
||||
sched_sim_args_t args;
|
||||
char tmp[] = "/tmp/xce_simlivealloc_run_XXXXXX";
|
||||
int fd = mkstemp(tmp);
|
||||
int saved_stdout;
|
||||
off_t len;
|
||||
char* buf;
|
||||
|
||||
TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must "
|
||||
"succeed");
|
||||
|
||||
fflush(stdout);
|
||||
saved_stdout = dup(STDOUT_FILENO);
|
||||
TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0, "dup(STDOUT_FILENO) must "
|
||||
"succeed");
|
||||
TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0,
|
||||
"dup2 redirect of STDOUT_FILENO must succeed");
|
||||
|
||||
memset(&args, 0, sizeof(args));
|
||||
args.replay_path = path;
|
||||
args.sim_start_ts = SCHED_SIM_START_TS_LATEST;
|
||||
args.sim_speed = 0;
|
||||
args.verify = verify;
|
||||
|
||||
*out_rc = sched_sim_driver_run(sim, &args);
|
||||
|
||||
fflush(stdout);
|
||||
dup2(saved_stdout, STDOUT_FILENO);
|
||||
close(saved_stdout);
|
||||
|
||||
len = lseek(fd, 0, SEEK_CUR);
|
||||
TEST_ASSERT_TRUE(len >= 0);
|
||||
|
||||
buf = (char*)calloc(1, (size_t)len + 1);
|
||||
TEST_ASSERT_NOT_NULL(buf);
|
||||
TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0);
|
||||
if (len > 0) {
|
||||
ssize_t nread = read(fd, buf, (size_t)len);
|
||||
TEST_ASSERT_TRUE(nread == len);
|
||||
}
|
||||
close(fd);
|
||||
unlink(tmp);
|
||||
|
||||
return buf;
|
||||
}
|
||||
|
||||
/* write_header_and_snapshot - produces a valid ReplayHeader+SnapshotEntry
|
||||
* prefix at a fresh temp path via the real sched_record_start(), same
|
||||
* technique test_sim_event_loop.c's own write_header_and_snapshot()/
|
||||
|
|
@ -516,9 +606,200 @@ test_sim_driver_run_dispatches_a_job_with_available_resources(void)
|
|||
"state flip with no resource assignment behind it");
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* bug-1838 regression: --sim-verify across a dispatch-then-later- */
|
||||
/* confirmation gap, via GENUINE live dispatch (not reconciliation). */
|
||||
/* ---------------------------------------------------------------------- */
|
||||
|
||||
/*
|
||||
* test_sim_verify_no_false_mismatch_after_live_dispatch_and_later_
|
||||
* confirmation -- scheduler-sim-eager-reconciliation's original Requirement
|
||||
* 4.2 ("a dispatch decision and its RUN-state confirmation recorded several
|
||||
* rounds apart must not produce a false --sim-verify mismatch"), now proved
|
||||
* via the mechanism that actually handles this case in production since
|
||||
* bug-1838 (scheduler-driver-equivalence: sim_reconcile_unaccounted_jobs()
|
||||
* narrowed back to JOB_IS_RUN-only, undoing scheduler-sim-eager-
|
||||
* reconciliation's 2026-09-11 PEND-inclusive widening).
|
||||
*
|
||||
* The ORIGINAL version of this Observable lived in test_sim_verify.c, which
|
||||
* deliberately stubs sched_job_picker()/sched_allocator() to a PEND-only
|
||||
* pair (see that file's own header comment) -- so BEFORE bug-1838, that
|
||||
* file could only ever prove this Requirement via reconciliation itself
|
||||
* force-applying the recorded DISPATCH (the PEND-only stub could never
|
||||
* produce one). Under the new RUN-only trigger, job 601's fixture there
|
||||
* (freshly PEND at round 1) no longer reaches reconciliation at all -- and
|
||||
* that file's stub allocator has nothing else to offer, so the scenario is
|
||||
* no longer reachable there. This file links the REAL scheduler_srcs (see
|
||||
* this file's own header comment), so it is the correct home for a live
|
||||
* proof of the same property: job TEST_JOBID is pushed PEND and dispatched
|
||||
* by the REAL sched_job_picker()/sched_allocator() pair at round 1
|
||||
* (matching the fixture the file's own first test already establishes),
|
||||
* --sim-verify is enabled, and the job's SEPARATE, later-replayed
|
||||
* JOB_STATE_CHANGE-to-RUN confirmation (round 2) is a redundant, safe
|
||||
* no-op re-affirmation (core_events.c's own unconditional `job->state =
|
||||
* new_state` reassignment) -- neither round may report a MISMATCH.
|
||||
*
|
||||
* A second, independent `xce_sched_sim_driver_t` (g_sim2, NOT g_sim) is
|
||||
* required here even though this file's own single-RUN_TEST() convention
|
||||
* means only one of these two tests actually runs per process invocation
|
||||
* under normal `meson test` execution -- see this file's own header
|
||||
* comment ("bug-1598/1632/1763 fixture-safety requirement"): sharing g_sim
|
||||
* across two independently-invocable RUN_TEST() entries would leave a
|
||||
* SECOND call's sched_sim_driver_start() memset()ing state a FIRST call's
|
||||
* still-live, never-joined threads could concurrently be dereferencing, if
|
||||
* a future harness change ever runs both tests in the same process. Static
|
||||
* storage (not stack-local) for the identical reason g_sim already is.
|
||||
*/
|
||||
static xce_sched_sim_driver_t g_sim2;
|
||||
|
||||
static void
|
||||
test_sim_verify_no_false_mismatch_after_live_dispatch_and_later_confirmation(
|
||||
void)
|
||||
{
|
||||
char path[PATH_MAX];
|
||||
FILE* f;
|
||||
xce_resource_t* slots_res;
|
||||
xce_resource_t* res_ptrs[1];
|
||||
xce_worker_t* worker_ptrs[1];
|
||||
xce_queue_t* queue;
|
||||
xce_job_t* job_ptrs[1];
|
||||
xce_sched_job_t* sjob;
|
||||
char* captured;
|
||||
int rc;
|
||||
|
||||
f = write_header_and_snapshot(path, sizeof(path));
|
||||
|
||||
slots_res = (xce_resource_t*)calloc(1, sizeof(*slots_res));
|
||||
TEST_ASSERT_NOT_NULL(slots_res);
|
||||
slots_res->name = strdup("slots");
|
||||
res_ptrs[0] = slots_res;
|
||||
append_resourcespush_event(f, 1001ULL, res_ptrs, 1);
|
||||
|
||||
worker_ptrs[0] = make_wire_worker(WORKER_NAME, WORKER_SLOTS);
|
||||
append_workerspush_event(f, 1002ULL, worker_ptrs, 1);
|
||||
|
||||
queue = make_wire_queue(QUEUE_NAME, 1);
|
||||
append_queueupdate_event(f, 1003ULL, queue);
|
||||
|
||||
job_ptrs[0] = make_push_job(TEST_JOBID, TEST_RESREQ, (char*)"alice",
|
||||
(char*)QUEUE_NAME, (char*)"proj1", 1, 1);
|
||||
append_jobspush_event(f, 1004ULL, job_ptrs, 1);
|
||||
|
||||
/* Round 1: empty recorded decisions -- --sim-verify compares the live
|
||||
* picker/allocator's OWN freshly-drained DISPATCH decision against
|
||||
* this round's decoded_session, which carries nothing recorded at all.
|
||||
* A live decision with no corresponding recorded entry is ITSELF
|
||||
* reported as a mismatch by sim_loop.c's own drain loop (expected=NULL)
|
||||
* -- so, unlike test_sim_driver_run_dispatches_a_job_with_available_
|
||||
* resources above, THIS test must record the SAME DISPATCH the live
|
||||
* path is about to independently produce, proving they agree, not
|
||||
* merely that the live path ran. Built the same way test_sim_
|
||||
* contention_parity.c's own make_dispatch_alloc()-based fixtures do:
|
||||
* one host allocation naming WORKER_NAME, wanting exactly what a
|
||||
* 1-task job's TEST_RESREQ ("rusage[mem=1]", auto-injected 1 slot per
|
||||
* task) actually requests.
|
||||
*
|
||||
* sim_verify_alloc_eq() (sim_verify.c) compares xce_alloc_t
|
||||
* field-by-field, including instid and duration -- both must match
|
||||
* EXACTLY what the real seqjob_allocator()->sched_make_allocs()-
|
||||
* >make_alloc() chain (scheduler.c) will independently produce for
|
||||
* this exact fixture, not arbitrary placeholders:
|
||||
* - instid=1: sched_get_id() returns core->nextid++, and core.c
|
||||
* initializes nextid to 1 (core.c:127) -- this fixture's job is
|
||||
* the FIRST (and only) allocation ever made against this fresh
|
||||
* core, so it is deterministically the first ID issued.
|
||||
* - duration=-1: seqjob_allocator.c's own sched_make_allocs() call
|
||||
* passes a NULL duration array ("FIXME: no duration now"), so
|
||||
* sched_make_allocs() (scheduler.c) always defaults every entry's
|
||||
* duration to -1 today.
|
||||
* `from` (wall-clock time(NULL), set unconditionally by make_alloc()
|
||||
* itself) is NOT independently reproduced here -- it is expected to
|
||||
* agree with the live path's own call by virtue of both happening
|
||||
* within the same sub-second test execution window, not by
|
||||
* construction; this is a real, if practically negligible, source of
|
||||
* flakiness inherent to comparing a genuinely-live-dispatched
|
||||
* decision's wall-clock-stamped fields via --sim-verify, orthogonal
|
||||
* to anything bug-1838 changed. */
|
||||
{
|
||||
xce_alloc_t* a = make_alloc((char*)"slots", NULL, 1, 1.0,
|
||||
-1);
|
||||
xce_alloc_t** allocs = make_allocs(1, a);
|
||||
xce_host_alloc_t* h = make_host_alloc((char*)WORKER_NAME, 1, allocs);
|
||||
xce_host_alloc_t** hallocs = make_host_allocs(1, h);
|
||||
xce_job_alloc_t* alloc = make_job_alloc(TEST_JOBID, 1, hallocs);
|
||||
sched_dispatch_decision_t dispatch;
|
||||
replay_decision_t d_dispatch;
|
||||
replay_decision_t* decisions[1];
|
||||
replay_session_t session;
|
||||
struct capn c;
|
||||
struct capn_segment* cs;
|
||||
ReplaySession_ptr sp;
|
||||
|
||||
memset(&dispatch, 0, sizeof(dispatch));
|
||||
dispatch.alloc = alloc;
|
||||
|
||||
memset(&d_dispatch, 0, sizeof(d_dispatch));
|
||||
d_dispatch.body_which = ReplayDecision_body_dispatch;
|
||||
d_dispatch.body.dispatch = &dispatch;
|
||||
d_dispatch.ts = 2000ULL;
|
||||
|
||||
decisions[0] = &d_dispatch;
|
||||
|
||||
memset(&session, 0, sizeof(session));
|
||||
session.begin_round = 2000ULL;
|
||||
session.decisions = decisions;
|
||||
session.n_decisions = 1;
|
||||
session.summary = NULL;
|
||||
|
||||
capn_init_malloc(&c);
|
||||
cs = capn_root(&c).seg;
|
||||
encode_ReplaySession_ptr(cs, &sp, &session);
|
||||
capn_setp(capn_root(&c), 0, sp.p);
|
||||
TEST_ASSERT_EQUAL_INT(0, write_capnp_message(f, &c));
|
||||
capn_free(&c);
|
||||
}
|
||||
|
||||
/* Round 2: job's separately-recorded RUN confirmation, several rounds
|
||||
* (here: one) after its own round-1 DISPATCH -- a redundant, safe
|
||||
* no-op by the time it replays (the live path already made the job
|
||||
* RUN at round 1). Recorded decisions empty, matching what a genuinely
|
||||
* idle round produces. */
|
||||
append_jobstatechange_event(f, 2500ULL, TEST_JOBID, JOB_STAT_PEND,
|
||||
JOB_STAT_RUN);
|
||||
append_replay_session(f, 3000ULL);
|
||||
|
||||
append_end_of_stream_marker(f);
|
||||
fclose(f);
|
||||
|
||||
captured = run_sim_and_capture_stdout(&g_sim2, path, true, &rc);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
0, rc, "sched_sim_driver_run() must complete both rounds and reach "
|
||||
"its own end-of-stream marker");
|
||||
TEST_ASSERT_NULL_MESSAGE(
|
||||
strstr(captured, "MISMATCH"),
|
||||
"a dispatch decision and its RUN-state confirmation recorded in a "
|
||||
"later round must not produce a false --sim-verify mismatch "
|
||||
"(scheduler-sim-eager-reconciliation Requirement 4.2), now proved "
|
||||
"via genuine live dispatch (bug-1838) rather than reconciliation");
|
||||
|
||||
sjob = sched_get_job(&(g_sim2.core), TEST_JOBID);
|
||||
TEST_ASSERT_NOT_NULL(sjob);
|
||||
TEST_ASSERT_EQUAL_UINT_MESSAGE(
|
||||
JOB_STAT_RUN, sjob->state,
|
||||
"the job must genuinely be RUN via the live picker/allocator's own "
|
||||
"round-1 dispatch, independent of round 2's redundant confirmation "
|
||||
"event");
|
||||
TEST_ASSERT_NOT_NULL(sjob->allocs);
|
||||
|
||||
free(captured);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_sim_driver_run_dispatches_a_job_with_available_resources);
|
||||
RUN_TEST(
|
||||
test_sim_verify_no_false_mismatch_after_live_dispatch_and_later_confirmation);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue