From 88bed529a700ea36efb5178a5f5033bb4f078cb5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 22 Aug 2026 11:40:14 +0000 Subject: [PATCH] test(scheduler-sim-decision-parity): regression suite for existing guarantees (task 4.2) Four checks in one run: (a) REPL-vs-batch decision equivalence unmodified and still passing; (b) --sim-verify reports no false mismatch for a recording with multiple already-resolved jobs across rounds (non-adjacent memmove compaction, no cross-round bleed); (c) a replay starting from a snapshot with a job already RUN-state and no in-window recorded decision degrades gracefully (no crash, no fabricated value, a second ordinary job unaffected) -- the first test to exercise reconciliation's "not found" path through the real sched_sim_driver_run()/xce_sched_core_load_snapshot() path rather than a hand-built core; (d) normal-mode behavior is byte-for- byte unchanged (zero production diff, the strongest possible evidence). Found and fixed a real test-authoring gap while building (b): builtin_fcfs re-offers a still-PEND job as a fresh PEND every round (confirmed pre-existing, unrelated, also visible in test_repl_equivalence.c's own fixture) -- the first draft's round-2 fixture omitted this recurrence. No production code touched (task 3 already completed the feature) -- git diff on sim_loop.c/resource.c/scheduler.c/driver.c/sim_verify.c is empty. 49/50 scheduler tests pass; the 1 failure is pre-existing bug-1560 (unrelated). Co-Authored-By: Claude Sonnet 5 --- .../scheduler-sim-decision-parity/tasks.md | 4 +- src/scheduler/testsuite/meson.build | 24 + .../test_sim_snapshot_reconcile_gap.c | 575 ++++++++++++++++++ src/scheduler/testsuite/test_sim_verify.c | 201 ++++++ 4 files changed, 803 insertions(+), 1 deletion(-) create mode 100644 src/scheduler/testsuite/test_sim_snapshot_reconcile_gap.c diff --git a/.kiro/specs/scheduler-sim-decision-parity/tasks.md b/.kiro/specs/scheduler-sim-decision-parity/tasks.md index 91b85175..508a8649 100644 --- a/.kiro/specs/scheduler-sim-decision-parity/tasks.md +++ b/.kiro/specs/scheduler-sim-decision-parity/tasks.md @@ -70,7 +70,7 @@ - _Depends: 3_ - _Boundary: Scheduler test suite (contention parity)_ -- [ ] 4.2 (P) Regression suite: existing guarantees still hold +- [x] 4.2 (P) Regression suite: existing guarantees still hold - Re-run the existing interactive-vs-batch decision-equivalence coverage unmodified and confirm it still passes - Extend decision-verification coverage with a recording containing already-resolved jobs and confirm no mismatch is reported as a result of this feature - Confirm a replay starting from a snapshot with jobs already running at snapshot time (no recorded decision available in-window) degrades gracefully rather than crashing or fabricating a value @@ -99,3 +99,5 @@ - (task 3) `get_resvals()` (resource.c) returns `nres` (the count filled in) on success, NOT `0` -- a test-authoring bug (bug-1622, not production) came from assuming the latter. If you write a test against `get_resvals()`, check its actual return-value contract first. - (task 3) Feature is now end-to-end wired and independently confirmed working (reviewer removed the call site itself and watched both integration tests fail for the right reason, then restored and watched them pass). Tasks 4.1-4.3 (Validation) can now exercise the real, complete path -- no more "not fully wired yet" caveats apply. - (task 4.1) `sim_picker_thread_fn()` is confirmed (by direct read, independently re-confirmed by the reviewer) to be structurally PEND-only -- it has no capacity-check/dispatch path for a job it evaluates fresh. This means task 1.2's fixture (or any single-round, freshly-arriving-jobs fixture) can NEVER be literally replayed-and-diffed for full dispatch/PEND parity -- sim's picker always PENDs a fresh job regardless of whether resource accounting is correct, so that comparison would be tautological for freshly-arriving jobs (though still meaningful for reconciled/already-resolved jobs). If a later task needs "real recording vs. sim replay" parity proof, build it the way task 4.1 did: a real driven normal-mode recording plus a hand-crafted sim-mode fixture modeling the identical facts (including at least one already-resolved-before-replay job to exercise reconciliation), not a literal single-file replay-and-diff. This is a real architectural limit, not a testing shortcut -- true dispatch-decision parity for freshly-arriving jobs is scheduler-allocation's (a separate, not-yet-designed spec) job, out of this feature's scope. +- (task 4.2) `builtin_fcfs`'s pend-handling (`builtin_fcfs_on_pend`/`begin_session`) never clears a job from `pendJobs` once it's added -- a still-genuinely-PEND job recurs as a fresh PEND decision in the recorded stream EVERY round/session, not just once. This is pre-existing, unrelated-to-this-feature behavior (also visible in `test_repl_equivalence.c`'s own existing fixture). If you build a multi-round `.replay` fixture with a job that stays PEND across rounds, expect and account for that job reappearing in every round's decoded decisions -- don't assume a PEND is a one-time event. +- (task 4.2) Not every reconciliation-adjacent test needs to be discriminating via the "disable the reconciliation call site" toggle to be worth keeping -- a test exercising the "not found, nothing to reconcile" branch through the REAL `sched_sim_driver_run()`/`xce_sched_core_load_snapshot()` path can be identical whether reconciliation runs or not, and still catch real bugs (fabrication, snapshot-load corruption, crashes) that a hand-built-core unit test (task 2.2's) can't reach. Judge a test's value by what regression classes it actually covers, not solely by one discriminating-power toggle. diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 9d841eeb..970ad578 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -810,6 +810,27 @@ sched_sim_round_boundary_reconciliation_test = executable( dependencies: [scheduler_test_deps], install: false) +# Regression coverage for task 4.2 (Req 4.1, 4.2): a replay starting from a +# SNAPSHOT that already has a job in RUN state, with no recorded decision +# for it anywhere in this replay's own window, must degrade gracefully -- +# job->accounts/allocs stay NULL (unavailable, never fabricated), the run +# completes cleanly (rc == 0, no --sim-verify mismatch), and a second, +# ordinary job elsewhere in the same replay is still decided normally. A +# different trigger shape from sched_sim_verify_test's own already-resolved- +# job case (that job transitions RUN mid-replay via a JOB_STATE_CHANGE +# event WITH a matching recorded DISPATCH in the same round -- reconciled, +# not left unavailable): this file's job is already RUN in the INITIAL +# SNAPSHOT itself, with no recorded decision anywhere in-window to +# reconcile from at all, so sim_reconcile_unaccounted_jobs()'s "not found" +# branch (task 2.2) is the only path that can ever touch it -- see this +# file's own header comment for the full scenario and rationale. +sched_sim_snapshot_reconcile_gap_test = executable( + 'sched_sim_snapshot_reconcile_gap_test', + ['test_sim_snapshot_reconcile_gap.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + # Record-then-replay-and-diff parity proof for task 4.1 (Req 1.4, 2.1, 2.2): # extends task 1.2's resource-contention scenario (worker capacity 10.0, # jobs wanting 8.0/5.0/2.0) to an actual round-by-round comparison between a @@ -1169,6 +1190,9 @@ test('scheduler sim resource contention test', test('scheduler sim round boundary reconciliation test', sched_sim_round_boundary_reconciliation_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +test('scheduler sim snapshot reconcile gap test', + sched_sim_snapshot_reconcile_gap_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler sim contention parity test', sched_sim_contention_parity_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/scheduler/testsuite/test_sim_snapshot_reconcile_gap.c b/src/scheduler/testsuite/test_sim_snapshot_reconcile_gap.c new file mode 100644 index 00000000..39862082 --- /dev/null +++ b/src/scheduler/testsuite/test_sim_snapshot_reconcile_gap.c @@ -0,0 +1,575 @@ +/* + * test_sim_snapshot_reconcile_gap.c - Regression coverage for scheduler- + * sim-decision-parity task 4.2 (Req 4.1, 4.2): a replay starting from a + * snapshot that already has a job in RUN state, with NO recorded decision + * anywhere in this replay's own window, must degrade gracefully -- report + * that job's usage as unavailable (job->accounts stays NULL, nothing + * fabricated), never crash, and continue processing the rest of the replay + * normally for every other job. + * + * This is a DIFFERENT trigger shape from test_sim_verify.c's own + * test_verify_with_already_resolved_job_reports_no_mismatch (task 3, Req + * 5.2): that test's already-resolved job (A) transitions PEND->RUN via a + * JOB_STATE_CHANGE event WITHIN this replay's own recorded window, with a + * matching recorded DISPATCH decision in the SAME round -- reconciliation + * finds a match and rebuilds job->allocs/job->accounts from it. This file's + * job is already RUN *in the initial snapshot itself* (the state a job is + * in when it was dispatched before this replay's own recording ever started + * capturing decisions) -- design.md's Implementation Notes calls this out + * explicitly: "xce_sched_core_load_snapshot() restores job->state from + * snapshot data but never touches job->accounts/job->allocs ... this is + * unambiguously Requirement 4's graceful-degradation case." No recorded + * DISPATCH for this job exists ANYWHERE in the replay stream (there is + * nothing to reconstruct from), so sim_reconcile_unaccounted_jobs()'s + * per-round search must run to exhaustion and take the "not found" branch + * (task 2.2) every round, forever -- not treated as an error, not + * fabricating an allocation, and not blocking a second, ordinary job in the + * same replay from being decided normally. + * + * task 2.2's own unit-level test (test_sim_reconcile_unaccounted.c) already + * proves the underlying "not found" branch directly, calling + * sim_reconcile_unaccounted_jobs() by hand against a synthetic core/session + * pair. This file adds the integration-level proof that same branch was + * missing until now: a job restored through a REAL + * xce_sched_core_load_snapshot() call (via the real `.replay` header+ + * snapshot prefix sched_record_start() writes, the same production path + * every operator-facing replay uses), reaching the round boundary through + * the real, unmodified sched_sim_driver_run(), with a genuinely empty + * search (the only recorded decision in the whole file belongs to a + * different, ordinary job) -- not a hand-built xce_sched_core_t the unit + * test constructs directly. It also confirms (via builtin_fcfs.c's own + * JOB_IS_PEND(job->state) guard in on_job_push, unchanged by this feature) + * that the RUN-state job restored from the snapshot never becomes a live + * picker candidate either, so the only path that can ever give it visible + * usage is reconciliation -- exactly the gap this feature closes. + * + * Fixture construction: build_fixture_scheduler_with_run_job() pushes a + * QUEUE_UPDATE + JOBS_PUSH (job 901, JOB_STAT_RUN -- core_build_job() + * (core_events.c) copies job->state verbatim from the pushed wire job; this + * is NOT a JOB_STATE_CHANGE transition) directly onto the fixture + * xce_scheduler_t's own core, via the real xce_core_push_events(), BEFORE + * calling sched_record_start() -- so the snapshot sched_record_start() + * writes (recording.c's own write_snapshot_entry(), unchanged) captures job + * 901 already in RUN state, with no job->accounts/allocs (core_build_job() + * only sets job->accounts when the pushed wire job's own `allocs` field is + * non-NULL, which it never is here; snapshot.c's own header comment + * separately confirms save/load never captures job->allocs/accounts either + * way). No picker plugin is registered on this throwaway fixture core (same + * "no picker needs to be key_add'ed" convention test_sim_verify.c's own + * build_fixture_scheduler() documents) -- xce_core_push_events()'s + * sched_resolve_job_picker() failure just WARNs and continues with a NULL + * picker (core_events.c), so job 901 is still built and added to core->jobs + * correctly. + * + * White-box #include set: identical to test_sim_verify.c's own (see that + * file's header comment for the full rationale of each entry). + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "controller.capnp.h" +#include "kvmgr.h" +#include "preason.h" +#include "scheduler.h" +#include "xstring.h" + +/* Pull in the real functions under test into this TU (white-box #include, + * same convention/set as test_sim_verify.c). */ +#include "../src/core.c" +#include "../src/core_clock.c" +#include "../src/core_events.c" +#include "../src/core_session.c" +#include "../src/decision.c" +#include "../src/plugin.c" +#include "../src/plugins/builtin_fcfs.c" +#include "../src/recording.c" +#include "../src/recording_session.c" +#include "../src/repl.c" +#include "../src/repl_break.c" +#include "../src/repl_info.c" +#include "../src/repl_params.c" +#include "../src/sim.c" +#include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ +#include "../src/sim_record.c" +#include "../src/sim_verify.c" +#include "../src/snapshot.c" +#include "../src/snapshot_load.c" +#include "../src/snapshot_wire.c" + +void kill_sched_job(void* arg) { (void)arg; } +void kill_sched_queue(void* arg) { (void)arg; } +void kill_sched_worker(void* arg) { (void)arg; } +void kill_sched_user(void* arg) { (void)arg; } +void kill_sched_project(void* arg) { (void)arg; } +void kill_sched_resval(xce_val_t* val) { (void)val; } + +/* core_reset_workers()'s (core_session.c) linker dependency -- same stub + * test_sim_verify.c already establishes; never invoked (no worker is ever + * added here). */ +void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; } + +/* plugin_prepare_allocation()'s (plugin.c, the "limit" plugin category) + * linker dependency -- never invoked (this file never drives real + * allocation). */ +int sched_fill_limit_requests(xce_scheduler_t* sched, xvec_idval_t* vec, + xlist_t* limits) +{ + (void)sched; + (void)vec; + (void)limits; + return 0; +} + +/* sched_sim_driver_run()'s (sim_loop.c, scheduler-sim-decision-parity task + * 2.1) round-boundary call to the relocated sched_account_jobs() (core.h) + * -- a no-op stand-in here since this file does not link + * scheduler.c/resource.c, same rationale test_sim_verify.c's own copy + * documents. This file's Observable does not depend on resInUse/ + * sharedInUse totals at all -- only on job->accounts staying NULL and the + * replay completing cleanly -- so a no-op is sufficient here. */ +int sched_account_jobs(xce_sched_core_t* core) +{ + (void)core; + return 0; +} + +/* Real reimplementation of schedobj.c's sched_resolve_job_picker -- same + * copy every white-box test in this directory that does not link + * schedobj.c already establishes. */ +int sched_resolve_job_picker(xce_sched_core_t* core, xce_sched_plugin_t** out) +{ + xce_sched_plugin_t* plugin = NULL; + int rc = 0; + + if ((core == NULL) || (out == NULL)) { + return -EINVAL; + } + + *out = NULL; + + if (core->job_pick_plugin == NULL) { + return -EINVAL; + } + + lock(core->plugins, RTTI_LOCK_READ); + rc = key_find(core->plugins, core->job_pick_plugin, (void**)&plugin); + unlock(core->plugins); + + if (rc <= 0) { + return (rc < 0) ? rc : -ENOENT; + } + + if ((plugin == NULL) || (plugin->picker_iface == NULL) + || (plugin->picker_iface->next_job == NULL) + || (plugin->picker_iface->begin_session == NULL) + || (plugin->picker_iface->end_session == NULL) + || (plugin->picker_iface->on_dispatch == NULL) + || (plugin->picker_iface->on_pend == NULL)) { + return -EINVAL; + } + + *out = plugin; + + return 0; +} + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* ---------------------------------------------------------------------- */ +/* Fixture helpers. */ +/* ---------------------------------------------------------------------- */ + +static void make_tmp_dir(char* out, size_t out_len, const char* suffix) +{ + char tmpl[PATH_MAX]; + + snprintf(tmpl, sizeof(tmpl), "/tmp/sched_simsnapgap%s_XXXXXX", suffix); + TEST_ASSERT_NOT_NULL(mkdtemp(tmpl)); + TEST_ASSERT_TRUE(strlen(tmpl) < out_len); + strcpy(out, tmpl); +} + +/* Same xce_job_t wire-job builder test_snapshot_save_load.c's own + * make_job() establishes -- reused here (this file's own no-shared-header + * convention, matching every other file in this directory) but with an + * EXPLICIT state parameter, not the JOB_STAT_PEND every other fixture in + * this directory hardcodes. */ +static xce_job_t* make_job_with_state(uint64_t jobid, uint32_t state, + const char* queue) +{ + xce_job_t* j = (xce_job_t*)calloc(1, sizeof(*j)); + + TEST_ASSERT_NOT_NULL(j); + j->jobid = jobid; + j->state = state; + j->sub_time = 1000; + j->queue_time = 1000; + j->start_time = (state == JOB_STAT_RUN) ? 1000 : 0; + j->submit = (job_submit_data_t*)calloc(1, sizeof(*(j->submit))); + TEST_ASSERT_NOT_NULL(j->submit); + j->submit->queue = (char*)queue; + j->submit->user = (char*)"alice"; + j->submit->mintasks = 1; + j->submit->maxtasks = 1; + + return j; +} + +/* + * build_fixture_scheduler_with_run_job - test_sim_verify.c's own + * build_fixture_scheduler() (throwaway xce_scheduler_t: xce_sched_core_init(), + * job_pick_plugin set to the NAME "builtin_fcfs" but no plugin object ever + * key_add'ed -- Req 5 AC 21, xce_sched_core_save_snapshot() does not + * require a resolvable picker), PLUS one queue and one job already in RUN + * state pushed directly onto `sched->core` via the real + * xce_core_push_events() -- BEFORE the caller's own sched_record_start() + * call snapshots it. This is what makes the snapshot sched_record_start() + * writes contain a RUN-state job with job->accounts == NULL from the very + * first byte of the replay stream, the exact shape design.md's + * Implementation Notes describes for a job "already running at the + * snapshot the replay starts from." + */ +static void build_fixture_scheduler_with_run_job(xce_scheduler_t* sched, + char* record_dir, + uint64_t run_jobid, + const char* queue_name) +{ + xce_queue_t wire_queue; + xce_job_t* run_job; + xce_job_t* jobs[1]; + xce_core_event_t events[2]; + xce_sched_job_t* job; + int rc; + + memset(sched, 0, sizeof(*sched)); + mylock_init(&(sched->record_lock)); + + rc = xce_sched_core_init(&(sched->core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + sched->core.job_pick_plugin = (char*)"builtin_fcfs"; + sched->record_dir = record_dir; + + memset(&wire_queue, 0, sizeof(wire_queue)); + wire_queue.queue = (char*)queue_name; + wire_queue.status = QUEUE_STAT_OPEN; + wire_queue.priority = 1; + + run_job = make_job_with_state(run_jobid, JOB_STAT_RUN, queue_name); + jobs[0] = run_job; + + memset(events, 0, sizeof(events)); + events[0].type = XCE_CORE_EVENT_QUEUE_UPDATE; + events[0].u.queue_update.queue = &wire_queue; + events[1].type = XCE_CORE_EVENT_JOBS_PUSH; + events[1].u.jobs_push.njobs = 1; + events[1].u.jobs_push.jobs = jobs; + + rc = xce_core_push_events(&(sched->core), events, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Sanity: job is really RUN-state with no accounts yet, in THIS + * fixture core, before it's ever snapshotted -- non-tautology guard, + * proving the pre-snapshot setup itself is correct. */ + job = sched_get_job(&(sched->core), run_jobid); + TEST_ASSERT_NOT_NULL(job); + TEST_ASSERT_TRUE(JOB_IS_RUN(job->state)); + TEST_ASSERT_NULL(job->accounts); +} + +/* Same "leave the file open, skip sched_record_stop()'s own endSession + * marker" technique test_sim_verify.c's own write_header_and_snapshot() + * establishes -- this copy snapshots a core carrying one already-RUN job + * (via build_fixture_scheduler_with_run_job() above), not an empty one. */ +static FILE* write_header_and_snapshot_with_run_job(char* out_path, + size_t out_path_len, + const char* suffix, + uint64_t run_jobid, + const char* queue_name) +{ + char record_dir[PATH_MAX]; + xce_scheduler_t sched; + int rc; + + make_tmp_dir(record_dir, sizeof(record_dir), suffix); + build_fixture_scheduler_with_run_job(&sched, record_dir, run_jobid, + queue_name); + + rc = sched_record_start(&sched, "fixture.replay"); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_TRUE(strlen(sched.record_file_path) < out_path_len); + strcpy(out_path, sched.record_file_path); + + return sched.record_file; +} + +/* Appends one ReplayEvent{jobsPush} message with exactly one JOB_STAT_PEND + * job -- same technique test_sim_verify.c's own append_jobs_push_event() + * establishes; this file's own ordinary/control job. */ +static void append_jobs_push_event(FILE* f, uint64_t ts, uint64_t jobid, + const char* queue_name) +{ + job_submit_data_t submit; + xce_job_t job; + xce_job_t* jobs[1]; + xce_jobs_data_t jobs_data; + replay_event_t src; + struct capn c; + struct capn_segment* cs; + ReplayEvent_ptr p; + + memset(&submit, 0, sizeof(submit)); + submit.queue = (char*)queue_name; + submit.user = (char*)"bob"; + submit.mintasks = 1; + submit.maxtasks = 1; + + memset(&job, 0, sizeof(job)); + job.jobid = jobid; + job.state = JOB_STAT_PEND; + job.submit = &submit; + + jobs[0] = &job; + + memset(&jobs_data, 0, sizeof(jobs_data)); + jobs_data.n_total = 1; + jobs_data.count = 1; + jobs_data.jobs = jobs; + + memset(&src, 0, sizeof(src)); + src.ts = ts; + src.body_which = ReplayEvent_body_jobsPush; + src.body.jobs_push = &jobs_data; + + 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); +} + +static void append_end_of_stream_marker(FILE* f) +{ + replay_event_t src; + struct capn c; + struct capn_segment* cs; + ReplayEvent_ptr p; + + memset(&src, 0, sizeof(src)); + src.body_which = ReplayEvent_body_endSession; + + 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); +} + +/* Appends one ReplaySession carrying exactly one recorded PendDecision for + * `jobid` -- same technique test_sim_verify.c's own + * append_replay_session_with_pend() establishes. Deliberately carries NO + * decision at all for the snapshot's own already-RUN job: this file's + * entire point is that no such recorded decision exists anywhere in this + * replay's own window. */ +static void append_replay_session_with_pend(FILE* f, uint64_t begin_round, + uint64_t jobid, int main_reason) +{ + xce_job_preason_t reason; + sched_pend_decision_t pend; + replay_decision_t decision; + replay_decision_t* decisions[1]; + replay_session_t session; + struct capn c; + struct capn_segment* cs; + ReplaySession_ptr sp; + + memset(&reason, 0, sizeof(reason)); + reason.main_reason = main_reason; + + memset(&pend, 0, sizeof(pend)); + pend.jobid = jobid; + pend.reason = &reason; + + memset(&decision, 0, sizeof(decision)); + decision.ts = begin_round; + decision.body_which = ReplayDecision_body_pend; + decision.body.pend = &pend; + + decisions[0] = &decision; + + memset(&session, 0, sizeof(session)); + session.begin_round = begin_round; + 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); +} + +/* Same dup()/dup2() STDOUT_FILENO capture technique test_sim_verify.c's own + * run_sim_and_capture_stdout() establishes -- also hands back the driver + * struct itself (`out_sim`) so the caller can inspect post-replay core + * state (job->accounts/job->state) directly, which test_sim_verify.c's own + * copy never needed to do. */ +static char* run_sim_and_capture_stdout(const char* path, + bool verify, + xce_sched_sim_driver_t* out_sim, + int* out_rc) +{ + sched_sim_args_t args; + char tmp[] = "/tmp/xce_simsnapgap_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; /* frozen -- deterministic, no wall-clock wait */ + args.verify = verify; + + *out_rc = sched_sim_driver_run(out_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; +} + +/* ---------------------------------------------------------------------- */ +/* The Observable: Requirement 4's graceful degradation, from a snapshot. */ +/* ---------------------------------------------------------------------- */ + +/* + * test_replay_from_snapshot_with_already_running_job_degrades_gracefully -- + * scheduler-sim-decision-parity spec, task 4.2 (Req 4.1, 4.2): a replay + * starting from a snapshot with a job (901) already RUN, with NO recorded + * decision for it anywhere in this replay's own window, must: (1) never + * crash or abort the run (rc == 0), (2) leave that job's usage genuinely + * unavailable rather than fabricate an allocation (job->accounts/allocs + * stay NULL, job->state stays RUN, exactly as the snapshot restored it), + * and (3) continue processing the rest of the replay normally -- a second, + * ordinary job (902) pushed fresh mid-replay must still be decided (PEND) + * exactly as it would be with no already-running job in the mix at all. + */ +static void +test_replay_from_snapshot_with_already_running_job_degrades_gracefully(void) +{ + char path[PATH_MAX]; + FILE* f; + int rc; + char* captured; + xce_sched_sim_driver_t sim; + xce_sched_job_t* run_job; + xce_sched_job_t* fresh_job; + + /* Snapshot carries job 901, already RUN, no accounts -- see + * build_fixture_scheduler_with_run_job()'s own doc comment. */ + f = write_header_and_snapshot_with_run_job(path, sizeof(path), "_gap", + 901ULL, "normal"); + TEST_ASSERT_NOT_NULL(f); + + /* Job 902: an ordinary, freshly-arriving job -- the real picker + * independently PENDs it, exactly like every other integration fixture + * in this directory. Its own recorded PEND decision is the ONLY + * decision anywhere in this replay's window -- job 901 has none. */ + append_jobs_push_event(f, 1500ULL, 902ULL, "normal"); + append_replay_session_with_pend(f, 2000ULL, 902ULL, 0); + append_end_of_stream_marker(f); + fclose(f); + + captured = run_sim_and_capture_stdout(path, true, &sim, &rc); + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, rc, + "a replay starting from a snapshot with an already-running job and " + "no in-window recorded decision for it must complete cleanly, not " + "abort (Requirement 4.2)"); + TEST_ASSERT_NULL_MESSAGE( + strstr(captured, "MISMATCH"), + "the genuinely unaccounted-for snapshot job must not produce a " + "spurious --sim-verify mismatch either"); + + /* Requirement 4.1: usage reported unavailable, nothing fabricated. */ + run_job = sched_get_job(&(sim.core), 901ULL); + TEST_ASSERT_NOT_NULL_MESSAGE( + run_job, "the snapshot-restored job must still exist after replay"); + TEST_ASSERT_TRUE_MESSAGE( + JOB_IS_RUN(run_job->state), + "the snapshot-restored job's RUN state must be preserved"); + TEST_ASSERT_NULL_MESSAGE( + run_job->accounts, + "no recorded decision exists in-window for this job -- " + "job->accounts must stay NULL (unavailable), never fabricated"); + TEST_ASSERT_NULL_MESSAGE( + run_job->allocs, + "job->allocs must likewise stay NULL -- nothing to reconstruct " + "from and nothing must be fabricated in its place"); + + /* Requirement 4.2: the rest of the replay is unaffected -- job 902 + * still gets its ordinary, real PEND decision. */ + fresh_job = sched_get_job(&(sim.core), 902ULL); + TEST_ASSERT_NOT_NULL_MESSAGE( + fresh_job, "the ordinary, freshly-arriving job must still exist"); + TEST_ASSERT_FALSE_MESSAGE( + JOB_IS_RUN(fresh_job->state), + "the ordinary job was only ever PEND -- must not have been " + "affected by the unrelated unaccounted job's own processing"); + + free(captured); + printf("test_replay_from_snapshot_with_already_running_job_degrades_" + "gracefully: PASS\n"); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST( + test_replay_from_snapshot_with_already_running_job_degrades_gracefully); + return UNITY_END(); +} diff --git a/src/scheduler/testsuite/test_sim_verify.c b/src/scheduler/testsuite/test_sim_verify.c index 769bcc55..df588608 100644 --- a/src/scheduler/testsuite/test_sim_verify.c +++ b/src/scheduler/testsuite/test_sim_verify.c @@ -831,6 +831,106 @@ static void append_replay_session_with_dispatch_and_pend( capn_free(&c); } +/* Appends one ReplaySession carrying THREE recorded decisions -- a DISPATCH + * for `dispatch_alloc_a`'s jobid, a PEND for `pend_jobid`, and a DISPATCH + * for `dispatch_alloc_c`'s jobid (in that order: the still-PEND job's own + * recorded PEND sits BETWEEN the two DISPATCH entries) -- scheduler-sim- + * decision-parity spec, task 4.2 regression coverage (Req 5.1, 5.2): proves + * sim_reconcile_unaccounted_jobs()'s memmove-based compaction correctly + * consumes TWO, non-adjacent entries from the same round's + * decoded_session->decisions (task 3's own + * append_replay_session_with_dispatch_and_pend() above only ever needed to + * consume a single, leading entry), leaving the middle PEND entry correctly + * repositioned for the positional --sim-verify walk. Reuses + * make_replay_dispatch_decision()/make_replay_pend_decision() the same way + * append_replay_session_with_dispatch_and_pend() does. */ +static void append_replay_session_with_two_dispatches_and_pend( + FILE* f, uint64_t begin_round, xce_job_alloc_t* dispatch_alloc_a, + uint64_t pend_jobid, int pend_main_reason, + xce_job_alloc_t* dispatch_alloc_c) +{ + replay_decision_t d_dispatch_a; + replay_decision_t d_pend; + replay_decision_t d_dispatch_c; + replay_decision_t* decisions[3]; + replay_session_t session; + struct capn c; + struct capn_segment* cs; + ReplaySession_ptr sp; + + d_dispatch_a = make_replay_dispatch_decision(dispatch_alloc_a); + d_dispatch_a.ts = begin_round; + d_pend = make_replay_pend_decision(pend_jobid, pend_main_reason); + d_pend.ts = begin_round; + d_dispatch_c = make_replay_dispatch_decision(dispatch_alloc_c); + d_dispatch_c.ts = begin_round; + + decisions[0] = &d_dispatch_a; + decisions[1] = &d_pend; + decisions[2] = &d_dispatch_c; + + memset(&session, 0, sizeof(session)); + session.begin_round = begin_round; + session.decisions = decisions; + session.n_decisions = 3; + 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); +} + +/* Appends one ReplaySession carrying TWO recorded PendDecisions -- scheduler- + * sim-decision-parity spec, task 4.2 regression coverage: a still-genuinely- + * pending job is re-offered (and re-PENDed) by the real picker EVERY round + * it remains unresolved, not just the round it first arrives in + * (builtin_fcfs.c's own pendJobs discipline: "A non-pending job is left + * removed and not reinserted" -- the converse holds too, a job that stays + * PEND is never removed from its queue's pendJobs, so next_job() keeps + * offering it every subsequent round). Used for this file's own second + * round, where the still-PEND job from round 1 recurs alongside a fresh + * job. */ +static void append_replay_session_with_two_pends(FILE* f, uint64_t begin_round, + uint64_t jobid1, + int main_reason1, + uint64_t jobid2, + int main_reason2) +{ + replay_decision_t d1; + replay_decision_t d2; + replay_decision_t* decisions[2]; + replay_session_t session; + struct capn c; + struct capn_segment* cs; + ReplaySession_ptr sp; + + d1 = make_replay_pend_decision(jobid1, main_reason1); + d1.ts = begin_round; + d2 = make_replay_pend_decision(jobid2, main_reason2); + d2.ts = begin_round; + + decisions[0] = &d1; + decisions[1] = &d2; + + memset(&session, 0, sizeof(session)); + session.begin_round = begin_round; + session.decisions = decisions; + session.n_decisions = 2; + 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); +} + /* Runs sched_sim_driver_run() against `path` with --sim-verify set to * `verify`, capturing everything written to stdout during the call (the * same dup()/dup2() technique capture_stdout_around() above uses, inlined @@ -1076,6 +1176,105 @@ static void test_verify_with_already_resolved_job_reports_no_mismatch(void) "PASS\n"); } +/* + * test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatch + * -- scheduler-sim-decision-parity spec, task 4.2 (Req 5.1, 5.2): extends + * task 3's own single-already-resolved-job proof + * (test_verify_with_already_resolved_job_reports_no_mismatch above) to a + * MULTI-job, MULTI-round scenario: + * + * Round 1: TWO already-resolved jobs (A=501, C=503) plus ONE genuinely + * still-PEND job (B=502). The round's recorded decisions are + * [DISPATCH(A), PEND(B), DISPATCH(C)] -- the still-pending job's own + * recorded PEND sits BETWEEN the two DISPATCH entries reconciliation + * must consume, exercising the memmove-based compaction's non-adjacent- + * removal path (task 3's own test only ever consumed a single, leading + * entry). + * + * Round 2: one ordinary, freshly-arriving job (D=504), no reconciliation + * needed at all -- proves the PRIOR round's compaction has no + * cross-round effect (design.md's Implementation Notes: "decoded_session + * is decoded fresh once per round ... no cross-round effect") and that + * --sim-verify's positional walk resumes correctly at di=0 for the new + * round. + * + * Zero mismatches across both rounds is the Observable. + */ +static void +test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatch( + void) +{ + char path[PATH_MAX]; + FILE* f; + int rc; + char* captured; + xce_alloc_t* res_a[1]; + xce_host_alloc_t* host_a[1]; + xce_job_alloc_t* alloc_a; + xce_alloc_t* res_c[1]; + xce_host_alloc_t* host_c[1]; + xce_job_alloc_t* alloc_c; + + f = write_header_and_snapshot(path, sizeof(path), "_reconciled_multi"); + TEST_ASSERT_NOT_NULL(f); + + append_queue_update_event(f, 1000ULL, "normal"); + + /* Jobs A (501) and C (503): both already resolved before this round's + * live picker ever considers them. Job B (502): genuinely still PEND + * -- the real picker independently re-pends it. */ + append_jobs_push_event(f, 1500ULL, 501ULL, "normal"); + append_jobs_push_event(f, 1500ULL, 502ULL, "normal"); + append_jobs_push_event(f, 1500ULL, 503ULL, "normal"); + append_job_state_change_event(f, 1800ULL, 501ULL, JOB_STAT_PEND, + JOB_STAT_RUN); + append_job_state_change_event(f, 1801ULL, 503ULL, JOB_STAT_PEND, + JOB_STAT_RUN); + + res_a[0] = make_test_alloc("slots", "run", 1, 4.0, 0, 0); + host_a[0] = make_test_host_alloc("hostA", res_a, 1); + alloc_a = make_test_job_alloc(501ULL, host_a, 1); + + res_c[0] = make_test_alloc("slots", "run", 1, 2.0, 0, 0); + host_c[0] = make_test_host_alloc("hostC", res_c, 1); + alloc_c = make_test_job_alloc(503ULL, host_c, 1); + + /* Recorded order: DISPATCH(A), PEND(B), DISPATCH(C) -- B's entry is + * sandwiched between the two entries reconciliation consumes. */ + append_replay_session_with_two_dispatches_and_pend(f, 2000ULL, alloc_a, + 502ULL, 0, alloc_c); + + /* Round 2: job B (502) is still genuinely PEND (never dispatched, never + * state-changed) so builtin_fcfs's pendJobs discipline re-offers it to + * the live picker again this round -- it is NOT an already-resolved + * job, so reconciliation has nothing to do with it; it simply recurs + * as an ordinary live PEND, same as job D (504), the fresh arrival. + * Recorded order [502, 504] matches queue_time ordering (502 queued at + * ts=1500, before 504's ts=2500) -- no already-resolved job is in the + * mix at all this round, proving no cross-round bleed from round 1's + * own compaction. */ + append_jobs_push_event(f, 2500ULL, 504ULL, "normal"); + append_replay_session_with_two_pends(f, 3000ULL, 502ULL, 0, 504ULL, 0); + + append_end_of_stream_marker(f); + fclose(f); + + captured = run_sim_and_capture_stdout(path, true, &rc); + + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_NULL_MESSAGE( + strstr(captured, "MISMATCH"), + "reconciling TWO already-resolved jobs in the same round (with a " + "genuinely-pending job's own recorded decision sandwiched between " + "them), followed by an ordinary second round with no reconciliation " + "needed, must not produce a false --sim-verify mismatch anywhere " + "(Requirement 5.1, 5.2)"); + + free(captured); + printf("test_verify_with_multiple_already_resolved_jobs_across_rounds_" + "reports_no_mismatch: PASS\n"); +} + int main(void) { UNITY_BEGIN(); @@ -1095,6 +1294,8 @@ int main(void) RUN_TEST(test_verify_against_altered_copy_reports_exact_mismatch); RUN_TEST(test_verify_disabled_reports_no_mismatches_even_when_altered); RUN_TEST(test_verify_with_already_resolved_job_reports_no_mismatch); + RUN_TEST( + test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatch); return UNITY_END(); }