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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
Rongsong Shen 2026-08-27 06:02:08 +00:00
parent c971f7dc23
commit ab07791640
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 153 additions and 2 deletions

View file

@ -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.

View file

@ -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();
}