From ab07791640739c7339df3c55781f26568f031cb0 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 27 Aug 2026 06:02:08 +0000 Subject: [PATCH] test(scheduler-sim-eager-reconciliation): task 3.2 sim-verify regression Adds test_sim_verify_no_false_mismatch_across_round_separated_dispatch: a job's DISPATCH decision at round 1, RUN-state confirmation event at round 4, --sim-verify enabled throughout, asserting zero mismatches. Fixture correctly accounts for a mechanically-verified system behavior: sim mode's live picker (sim_picker_thread_fn/builtin_fcfs) never emits on_dispatch() -- it unconditionally re-PENDs every candidate each round, including a job reconciliation already captured, until the job's recorded JOB_STATE_CHANGE event removes it from pendJobs. The fixture includes a matching PEND(jobid) entry at each intervening round to match this reality, not an assumption. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg --- .../tasks.md | 5 +- src/scheduler/testsuite/test_sim_verify.c | 150 ++++++++++++++++++ 2 files changed, 153 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/scheduler-sim-eager-reconciliation/tasks.md b/.kiro/specs/scheduler-sim-eager-reconciliation/tasks.md index db61ac77..3bc302c8 100644 --- a/.kiro/specs/scheduler-sim-eager-reconciliation/tasks.md +++ b/.kiro/specs/scheduler-sim-eager-reconciliation/tasks.md @@ -35,7 +35,7 @@ - _Boundary: sim_reconcile_unaccounted_jobs, sched_account_jobs_ - _Depends: 1.1_ -- [ ] 3.2 (P) Verify decision-verification mode reports no new mismatch across a round-separated dispatch +- [x] 3.2 (P) Verify decision-verification mode reports no new mismatch across a round-separated dispatch - Run decision-verification mode over a replay where a job's dispatch decision and its running-state confirmation are recorded several rounds apart - Confirm the verification pass reports zero mismatches for that job's decision - Observable: a decision-verification test run over a multi-round-separated dispatch scenario reports zero mismatches @@ -54,4 +54,5 @@ ## Implementation Notes - Task 1.2: the pre-existing `test_recordstart_symlink_escape_replies_error` failure (bug-1560, `test_admin_request.c`) is unrelated to this spec and appears in every task's full-suite run — do not treat it as a regression. -- Task 3.1: any NEW `xce_sched_sim_driver_t` fixture passed to `sched_sim_driver_run()` in a scheduler test file MUST be declared `static`, not stack-local — `sched_sim_driver_run()` spawns a picker thread that is never joined and keeps reading the struct for the life of the process, so a stack-local instance is a latent stack-use-after-return once the test function returns (already-documented class, bug-1598/bug-1632; this spec's own task 3.1 round-1 review caught a live recurrence). `sched_sim_driver_start()` unconditionally `memset()`s the struct first, so `static` storage is safe as long as each fixture is written by exactly one call site. +- Task 3.1: any NEW `xce_sched_sim_driver_t` fixture passed to `sched_sim_driver_run()` in a scheduler test file MUST be declared `static`, not stack-local — `sched_sim_driver_run()` spawns a picker thread that is never joined and keeps reading the struct for the life of the process, so a stack-local instance is a latent stack-use-after-return once the test function returns (already-documented class, bug-1598/bug-1632; this spec's own task 3.1 round-1 review caught a live recurrence). `sched_sim_driver_start()` unconditionally `memset()`s the struct first, so `static` storage is safe as long as each fixture is written by exactly one call site. Caveat (task 3.2): this `static` fix is NOT safe for a helper called multiple times per process (e.g. `run_sim_and_capture_stdout()`) — an earlier call's never-joined zombie picker thread could still be alive when a later call's `memset()` reuses the same static storage. Leave multiply-invoked fixtures stack-local; only single-invocation fixtures should convert. +- Task 3.2: sim mode's live picker (`sim_picker_thread_fn` + `builtin_fcfs`) never calls `on_dispatch()` — it unconditionally PENDs every candidate, including a job whose allocation `sim_reconcile_unaccounted_jobs()` already captured. Such a job stays in the picker's `pendJobs` and gets re-PENDed every round until its recorded `JOB_STATE_CHANGE` event actually removes it. Any `--sim-verify`/decision-stream fixture spanning a captured-but-not-yet-RUN job's intervening rounds must include a matching `PEND(jobid)` entry at each of those rounds, or the fixture will produce a false mismatch (or mask a real one) unrelated to the behavior under test. diff --git a/src/scheduler/testsuite/test_sim_verify.c b/src/scheduler/testsuite/test_sim_verify.c index df588608..ee91fd9d 100644 --- a/src/scheduler/testsuite/test_sim_verify.c +++ b/src/scheduler/testsuite/test_sim_verify.c @@ -931,6 +931,37 @@ static void append_replay_session_with_two_pends(FILE* f, uint64_t begin_round, capn_free(&c); } +/* Appends one ReplaySession carrying ZERO recorded decisions -- the same + * shape a genuinely idle round produces in a real recording + * (recording_session.c's own sched_record_session_finalize() writes + * decisions=NULL/n_decisions=0 whenever a round's ctx->n stays 0, i.e. no + * sched_record_session_add_decision() call happened that round). Used by + * scheduler-sim-eager-reconciliation task 3.2's round-separated-dispatch + * test below for the round a job's JOB_STATE_CHANGE-to-RUN confirmation + * removes it from the live picker's own pendJobs before that round's + * picker session ever runs, leaving nothing to record. */ +static void append_empty_replay_session(FILE* f, uint64_t begin_round) +{ + replay_session_t session; + struct capn c; + struct capn_segment* cs; + ReplaySession_ptr sp; + + memset(&session, 0, sizeof(session)); + session.begin_round = begin_round; + session.decisions = NULL; + session.n_decisions = 0; + 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 @@ -1275,6 +1306,123 @@ test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatc "reports_no_mismatch: PASS\n"); } +/* + * test_sim_verify_no_false_mismatch_across_round_separated_dispatch -- + * scheduler-sim-eager-reconciliation spec, task 3.2 (Req 2.1, 2.2, 2.3, + * 4.2): --sim-verify against a replay where a job's recorded DISPATCH + * decision (round 1) and its recorded JOB_STATE_CHANGE-to-RUN confirmation + * (round 4, three rounds later) are genuinely separated -- the multi-round + * shape task 1.1 (this spec) fixed sim_reconcile_unaccounted_jobs() to + * capture correctly. Under the OLD gate (`!JOB_IS_RUN(job->state)` in + * addition to `job->accounts != NULL`), job 601 was still PEND at round 1, + * so reconciliation skipped it entirely -- decoded_session's own + * DISPATCH(601) entry was NEVER consumed, left over for --sim-verify's + * positional walk to compare against a live stream that never produces a + * DISPATCH at all (sim mode's own picker is PEND-only) -- a false + * MISMATCH. research.md's own "Interaction with --sim-verify" research log + * entry documents this exact latent risk and why widening the gate to + * `job->accounts == NULL` (state-independent) removes it rather than + * introducing a new one. + * + * Round shape (mirrors test_sim_contention_parity.c's own task 3.1 fixture + * numbers -- job A: dispatch round 1, RUN-confirmed round 4, a 3-round + * gap): + * Round 1 (begin_round=2000): job 601 pushed PEND; recorded decisions = + * [DISPATCH(601), PEND(601)]. The recorded PEND(601) entry is not a + * contrivance: sim mode's own live picker (builtin_fcfs) has no way to + * learn a job was already dispatched in the original recording -- + * sim_reconcile_unaccounted_jobs() captures job->accounts but never + * calls picker_iface->on_dispatch() to retire the job from the + * picker's own internal pendJobs (design.md's Non-Goals/Revalidation + * Triggers explicitly leave live-picker DISPATCH emission to a future + * scheduler-allocation spec) -- so job 601, still genuinely + * JOB_STAT_PEND at this round's picker session, IS independently + * re-offered and re-PENDed by the real picker thread even in the very + * round its DISPATCH is recorded. The recorded stream must include + * that same PEND(601) entry to stay verify-clean; reconciliation + * consumes the DISPATCH(601) entry ahead of the verify walk + * (sim_loop.c's own fixed ordering: reconcile, then + * xce_core_signal_go(), then drain/compare), leaving exactly the + * PEND(601) entry to align with the live picker's own single decision + * this round. + * Round 2, 3 (begin_round=3000/4000): job 601 is still genuinely PEND + * (no DISPATCH/state-change event touches it this round) -- the real + * picker re-offers and re-PENDs it every round, the same "no-op, + * retried next round" contract test_sim_contention_parity.c's own + * filler rounds already establish; recorded decisions = [PEND(601)] + * each round. + * Round 4 (begin_round=5000): job 601's JOB_STATE_CHANGE to RUN is + * applied as part of this round's own pushed event batch, strictly + * BEFORE the round's picker session begins (sim_loop.c's event-then- + * reconcile-then-signal-go ordering) -- builtin_fcfs_on_job_state_change() + * removes job 601 from pendJobs the moment it leaves PEND state, so the + * live picker has nothing left to offer this round. Recorded decisions + * = [] (an empty ReplaySession, the same shape a genuinely idle round + * produces in a real recording). + * + * Zero mismatches across all four rounds is the Observable (Requirement + * 4.2). It is also consistent evidence for 2.1 (the DISPATCH decision IS + * reported, via reconciliation's own three-call sequence, at round 1 -- + * not silently dropped or deferred to round 4) and 2.3 (no duplicate + * report: the live stream never independently emits a DISPATCH for job + * 601 at all, so there is nothing for reconciliation's own report to + * collide with). + */ +static void +test_sim_verify_no_false_mismatch_across_round_separated_dispatch(void) +{ + char path[PATH_MAX]; + FILE* f; + int rc; + char* captured; + xce_alloc_t* res[1]; + xce_host_alloc_t* host[1]; + xce_job_alloc_t* alloc; + + f = write_header_and_snapshot(path, sizeof(path), "_roundsep"); + TEST_ASSERT_NOT_NULL(f); + + append_queue_update_event(f, 1000ULL, "normal"); + append_jobs_push_event(f, 1500ULL, 601ULL, "normal"); + + res[0] = make_test_alloc("slots", "run", 1, 4.0, 0, 0); + host[0] = make_test_host_alloc("hostA", res, 1); + alloc = make_test_job_alloc(601ULL, host, 1); + + /* Round 1: DISPATCH recorded, plus the matching PEND the live picker + * independently re-produces this same round (see header comment). */ + append_replay_session_with_dispatch_and_pend(f, 2000ULL, alloc, 601ULL, + 0); + + /* Round 2, 3: job 601 still genuinely PEND -- filler rounds widening + * the dispatch-to-confirmation gap to genuinely "several rounds". */ + append_replay_session_with_pend(f, 3000ULL, 601ULL, 0); + append_replay_session_with_pend(f, 4000ULL, 601ULL, 0); + + /* Round 4: job 601's RUN confirmation, 3 rounds after its own round-1 + * DISPATCH -- applied before this round's picker session runs, so the + * live picker has nothing left to offer. */ + append_job_state_change_event(f, 4500ULL, 601ULL, JOB_STAT_PEND, + JOB_STAT_RUN); + append_empty_replay_session(f, 5000ULL); + + 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"), + "a dispatch decision and its RUN-state confirmation recorded " + "several rounds apart must not produce a false --sim-verify " + "mismatch (Requirement 4.2)"); + + free(captured); + printf("test_sim_verify_no_false_mismatch_across_round_separated_" + "dispatch: PASS\n"); +} + int main(void) { UNITY_BEGIN(); @@ -1296,6 +1444,8 @@ int main(void) 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); + RUN_TEST( + test_sim_verify_no_false_mismatch_across_round_separated_dispatch); return UNITY_END(); }