test(scheduler-sim-eager-reconciliation): task 1.2 capture-safety unit coverage
Adds 3 tests proving guarantees that must hold under the widened capture-eligibility gate: an already-captured job is never re-searched or re-captured regardless of state (test_reconcile_still_skips_already_ captured_job_regardless_of_state), a PEND job with no match this round stays uncaptured and remains eligible next round (test_reconcile_ no_match_leaves_pend_job_uncaptured_and_retries), and several simultaneously outstanding jobs are captured independently with zero cross-job interference (test_reconcile_multiple_jobs_captured_ independently_across_different_rounds). Required making the shared add_job() fixture helper tolerant of -EEXIST on a repeated queue registration (needed for the multi-job test, the first in this file to call add_job() twice against the same core) -- traced to kvtab_add()'s narrowly-scoped -EEXIST return, provably a no-op for all 11 pre-existing single-add_job()-call tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
0893fd50af
commit
c67382e6f1
2 changed files with 281 additions and 2 deletions
|
|
@ -9,7 +9,7 @@
|
||||||
- _Requirements: 1.1, 3.1_
|
- _Requirements: 1.1, 3.1_
|
||||||
- _Boundary: sim_reconcile_unaccounted_jobs_
|
- _Boundary: sim_reconcile_unaccounted_jobs_
|
||||||
|
|
||||||
- [ ] 1.2 Extend unit coverage for capture-safety guarantees under the widened eligibility
|
- [x] 1.2 Extend unit coverage for capture-safety guarantees under the widened eligibility
|
||||||
- Add a test proving a job whose allocation was already captured is never re-searched or re-captured on a later round, regardless of its state
|
- Add a test proving a job whose allocation was already captured is never re-searched or re-captured on a later round, regardless of its state
|
||||||
- Add a test proving a job with no matching recorded decision this round stays uncaptured and remains eligible to be found on a later round
|
- Add a test proving a job with no matching recorded decision this round stays uncaptured and remains eligible to be found on a later round
|
||||||
- Add a test with several simultaneously outstanding jobs whose recorded dispatch decisions land in different rounds, proving each job's captured allocation reflects only its own recorded decision with no cross-job interference
|
- Add a test with several simultaneously outstanding jobs whose recorded dispatch decisions land in different rounds, proving each job's captured allocation reflects only its own recorded decision with no cross-job interference
|
||||||
|
|
|
||||||
|
|
@ -260,6 +260,7 @@ static xce_sched_job_t* add_job(xce_sched_core_t* core, uint64_t jobid,
|
||||||
job_submit_data_t submit;
|
job_submit_data_t submit;
|
||||||
xce_job_t wire_job;
|
xce_job_t wire_job;
|
||||||
xce_sched_job_t* job;
|
xce_sched_job_t* job;
|
||||||
|
int queue_rc;
|
||||||
|
|
||||||
memset(&wire_queue, 0, sizeof(wire_queue));
|
memset(&wire_queue, 0, sizeof(wire_queue));
|
||||||
wire_queue.queue = (char*)"normal";
|
wire_queue.queue = (char*)"normal";
|
||||||
|
|
@ -267,7 +268,22 @@ static xce_sched_job_t* add_job(xce_sched_core_t* core, uint64_t jobid,
|
||||||
wire_queue.priority = 1;
|
wire_queue.priority = 1;
|
||||||
q = core_build_queue(&wire_queue);
|
q = core_build_queue(&wire_queue);
|
||||||
TEST_ASSERT_NOT_NULL(q);
|
TEST_ASSERT_NOT_NULL(q);
|
||||||
TEST_ASSERT_GREATER_OR_EQUAL(0, sched_add_queue(core, q));
|
/* Tolerate -EEXIST: a test placing MULTIPLE jobs on the same core (task
|
||||||
|
* 1.2's own multi-job isolation test is the first here to do so) calls
|
||||||
|
* add_job() more than once against the same `core`, and the "normal"
|
||||||
|
* queue this helper (re-)builds every call is already registered from
|
||||||
|
* the first call -- that is expected, not a fixture bug, so only a
|
||||||
|
* genuine failure (neither success nor EEXIST) is a hard assertion
|
||||||
|
* failure. The freshly-built-but-unused `q` on the EEXIST path is
|
||||||
|
* simply leaked, matching this whole suite's own "fixtures are not
|
||||||
|
* torn down, detect_leaks=0" convention documented in this file's own
|
||||||
|
* header comment. */
|
||||||
|
queue_rc = sched_add_queue(core, q);
|
||||||
|
TEST_ASSERT_TRUE_MESSAGE(
|
||||||
|
(queue_rc >= 0) || (queue_rc == -EEXIST),
|
||||||
|
"sched_add_queue must succeed, or fail with -EEXIST when this "
|
||||||
|
"fixture core's \"normal\" queue was already registered by an "
|
||||||
|
"earlier add_job() call against the same core");
|
||||||
|
|
||||||
memset(&submit, 0, sizeof(submit));
|
memset(&submit, 0, sizeof(submit));
|
||||||
submit.queue = (char*)"normal";
|
submit.queue = (char*)"normal";
|
||||||
|
|
@ -1102,6 +1118,264 @@ test_captured_pend_job_excluded_from_resinuse_until_run(void)
|
||||||
"test_captured_pend_job_excluded_from_resinuse_until_run: PASS\n");
|
"test_captured_pend_job_excluded_from_resinuse_until_run: PASS\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
/* Test 13 (scheduler-sim-eager-reconciliation task 1.2, Requirement 3.2): */
|
||||||
|
/* a job with job->accounts ALREADY set is never re-searched or */
|
||||||
|
/* re-captured on a later round's call, regardless of its state (PEND */
|
||||||
|
/* here, deliberately different from test_reconcile_skips_job_that_ */
|
||||||
|
/* already_has_accounts' RUN-state fixture above -- the state genuinely */
|
||||||
|
/* does not matter, only job->accounts != NULL does) -- even when a */
|
||||||
|
/* decoded session that round happens to also contain a matching-jobid */
|
||||||
|
/* DISPATCH entry. Proves the state-widened gate (task 1.1) did not */
|
||||||
|
/* weaken the pre-existing, untouched job->accounts != NULL guard */
|
||||||
|
/* (research.md's "Does removing the RUN-state gate risk fabricating or */
|
||||||
|
/* double-capturing?" log entry: this guard was never touched by task */
|
||||||
|
/* 1.1, so this is regression-proof coverage for an existing guarantee, */
|
||||||
|
/* not new-behavior coverage). */
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
|
||||||
|
static void
|
||||||
|
test_reconcile_still_skips_already_captured_job_regardless_of_state(void)
|
||||||
|
{
|
||||||
|
xce_sched_sim_driver_t sim;
|
||||||
|
xce_sched_job_t* job;
|
||||||
|
xce_job_account_t* accounts_before;
|
||||||
|
xce_job_alloc_t* allocs_before;
|
||||||
|
replay_session_t* decoded_round1;
|
||||||
|
replay_session_t* decoded_round2;
|
||||||
|
|
||||||
|
fixture_init(&sim);
|
||||||
|
|
||||||
|
/* PEND-state, already captured (mirrors add_job()'s own "already
|
||||||
|
* accounted" fixture shape, same technique
|
||||||
|
* test_reconcile_skips_job_that_already_has_accounts() uses -- here
|
||||||
|
* deliberately with a non-RUN state). */
|
||||||
|
job = add_job(&(sim.core), 7040ULL, JOB_STAT_PEND,
|
||||||
|
make_job_alloc(7040ULL, 0, NULL));
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->accounts, "fixture setup: job must already have accounts");
|
||||||
|
accounts_before = job->accounts;
|
||||||
|
allocs_before = job->allocs;
|
||||||
|
|
||||||
|
/* Round 1: nothing to do with this job either way -- establishes the
|
||||||
|
* "later round" framing the next call demonstrates. */
|
||||||
|
decoded_round1 = make_decoded_session(4000ULL, NULL);
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded_round1, NULL);
|
||||||
|
TEST_ASSERT_EQUAL_PTR(accounts_before, job->accounts);
|
||||||
|
|
||||||
|
/* Round 2 ("a later round"): a recorded DISPATCH entry for THIS job's
|
||||||
|
* own jobid is present. If the job->accounts != NULL guard were ever
|
||||||
|
* bypassed under the widened, state-independent trigger, this would be
|
||||||
|
* re-captured. It must not be. */
|
||||||
|
decoded_round2
|
||||||
|
= make_decoded_session(5000ULL, make_dispatch_entry(7040ULL));
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded_round2, NULL);
|
||||||
|
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
accounts_before, job->accounts,
|
||||||
|
"an already-captured PEND-state job must never be re-captured on a "
|
||||||
|
"later round, even with a matching-jobid recorded DISPATCH entry "
|
||||||
|
"present -- job->accounts != NULL remains the sole gate under the "
|
||||||
|
"widened, state-independent trigger");
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
allocs_before, job->allocs,
|
||||||
|
"job->allocs likewise must stay exactly what it already was -- "
|
||||||
|
"never swapped for the later round's entry");
|
||||||
|
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||||
|
1, decoded_round2->n_decisions,
|
||||||
|
"the matching entry must not be consumed either -- the trigger "
|
||||||
|
"never fires for an already-captured job at all");
|
||||||
|
|
||||||
|
printf("test_reconcile_still_skips_already_captured_job_regardless_of_"
|
||||||
|
"state: PASS\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
/* Test 14 (scheduler-sim-eager-reconciliation task 1.2, Requirement 4.4): */
|
||||||
|
/* a JOB_STAT_PEND job with NO matching recorded decision this round stays */
|
||||||
|
/* uncaptured (job->accounts stays NULL) after one call, and remains */
|
||||||
|
/* eligible -- demonstrated (per this task's own design.md guidance) by */
|
||||||
|
/* calling again with a NOW-matching decoded session and confirming */
|
||||||
|
/* capture succeeds then. Mirrors test_reconcile_is_safe_to_recheck_on_a_ */
|
||||||
|
/* later_round()'s "safe to retry" idiom above, but (unlike that sibling */
|
||||||
|
/* test, whose second round also has no match) this is the genuinely new */
|
||||||
|
/* behavior task 1.1 enables: a PEND job now gets multiple real chances */
|
||||||
|
/* across rounds and can actually succeed on a later one. */
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
|
||||||
|
static void
|
||||||
|
test_reconcile_no_match_leaves_pend_job_uncaptured_and_retries(void)
|
||||||
|
{
|
||||||
|
xce_sched_sim_driver_t sim;
|
||||||
|
xce_sched_job_t* job;
|
||||||
|
xce_job_alloc_t* jalloc;
|
||||||
|
replay_session_t* decoded_round1;
|
||||||
|
replay_session_t* decoded_round2;
|
||||||
|
|
||||||
|
fixture_init(&sim);
|
||||||
|
|
||||||
|
/* PEND-state, no known allocation, no recorded decision available yet
|
||||||
|
* -- the realistic in-flight case task 1.1's widened gate exists for. */
|
||||||
|
job = add_job(&(sim.core), 7041ULL, JOB_STAT_PEND, NULL);
|
||||||
|
TEST_ASSERT_NULL(job->accounts);
|
||||||
|
TEST_ASSERT_NULL(job->allocs);
|
||||||
|
|
||||||
|
decoded_round1 = make_decoded_session(4100ULL, NULL);
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded_round1, NULL);
|
||||||
|
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->accounts,
|
||||||
|
"no matching recorded decision this round -- a PEND job must stay "
|
||||||
|
"uncaptured (usage unavailable), never fabricated");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(job->allocs, "job->allocs likewise stays NULL");
|
||||||
|
|
||||||
|
/* Round 2: the job's recorded DISPATCH decision now shows up. The job
|
||||||
|
* remains eligible (was never given up on) -- capture succeeds this
|
||||||
|
* time, proving the previous failed attempt left no "give up" state
|
||||||
|
* behind. */
|
||||||
|
jalloc = make_test_job_alloc_with_host(7041ULL, "workerB", "slots", 1.0f);
|
||||||
|
decoded_round2 = make_decoded_session(
|
||||||
|
4200ULL, make_dispatch_entry_from_alloc(jalloc));
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded_round2, NULL);
|
||||||
|
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
jalloc, job->allocs,
|
||||||
|
"a job that stayed uncaptured with no match must remain fully "
|
||||||
|
"eligible on a later round -- capture succeeds as soon as a "
|
||||||
|
"matching recorded decision appears");
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->accounts, "job->accounts must be built once the retry finds a "
|
||||||
|
"match");
|
||||||
|
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||||
|
0, decoded_round2->n_decisions,
|
||||||
|
"the now-matched entry must be consumed exactly like any other "
|
||||||
|
"capture");
|
||||||
|
|
||||||
|
printf("test_reconcile_no_match_leaves_pend_job_uncaptured_and_retries: "
|
||||||
|
"PASS\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
/* Test 15 (scheduler-sim-eager-reconciliation task 1.2, Requirement 1.1, */
|
||||||
|
/* 3.1 under concurrent load): several simultaneously outstanding jobs, */
|
||||||
|
/* each with a DIFFERENT jobid, spread across three scenarios a single */
|
||||||
|
/* reconcile call must resolve with zero cross-job interference: */
|
||||||
|
/* - job_matched: PEND, its own DISPATCH entry IS this round. */
|
||||||
|
/* - job_no_match: PEND, no recorded entry at all this round. */
|
||||||
|
/* - job_already: captured on a hypothetical EARLIER round already */
|
||||||
|
/* (job->accounts/job->allocs preset directly via add_job()'s own */
|
||||||
|
/* "already accounted" shape, same technique */
|
||||||
|
/* test_reconcile_skips_job_that_already_has_accounts() and Test 13 */
|
||||||
|
/* above use) -- AND, to make the "no accidental overwrite" claim */
|
||||||
|
/* non-trivial, a matching-jobid DISPATCH entry for it is ALSO */
|
||||||
|
/* present this round. */
|
||||||
|
/* Asserts each job's own job->accounts reflects ONLY that job's own */
|
||||||
|
/* recorded decision, with no cross-job data or overwrite (the single-job */
|
||||||
|
/* tests above cannot distinguish "correct" from "correct by coincidence, */
|
||||||
|
/* only one candidate available", per design.md's own Testing Strategy). */
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
|
||||||
|
static void
|
||||||
|
test_reconcile_multiple_jobs_captured_independently_across_different_rounds(
|
||||||
|
void)
|
||||||
|
{
|
||||||
|
xce_sched_sim_driver_t sim;
|
||||||
|
xce_sched_job_t* job_matched;
|
||||||
|
xce_sched_job_t* job_no_match;
|
||||||
|
xce_sched_job_t* job_already;
|
||||||
|
xce_job_account_t* already_accounts_before;
|
||||||
|
xce_job_alloc_t* already_allocs_before;
|
||||||
|
xce_job_alloc_t* jalloc_matched;
|
||||||
|
xce_job_alloc_t* jalloc_already_new;
|
||||||
|
replay_session_t* decoded;
|
||||||
|
|
||||||
|
fixture_init(&sim);
|
||||||
|
|
||||||
|
job_matched = add_job(&(sim.core), 7050ULL, JOB_STAT_PEND, NULL);
|
||||||
|
job_no_match = add_job(&(sim.core), 7051ULL, JOB_STAT_PEND, NULL);
|
||||||
|
job_already = add_job(&(sim.core), 7052ULL, JOB_STAT_RUN,
|
||||||
|
make_job_alloc(7052ULL, 0, NULL));
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job_already->accounts,
|
||||||
|
"fixture setup: job_already must already have accounts, "
|
||||||
|
"simulating capture on an earlier round");
|
||||||
|
already_accounts_before = job_already->accounts;
|
||||||
|
already_allocs_before = job_already->allocs;
|
||||||
|
|
||||||
|
jalloc_matched
|
||||||
|
= make_test_job_alloc_with_host(7050ULL, "workerA", "slots", 5.0f);
|
||||||
|
jalloc_already_new = make_test_job_alloc_with_host(
|
||||||
|
7052ULL, "worker-should-not-apply", "slots", 99.0f);
|
||||||
|
|
||||||
|
decoded = (replay_session_t*)calloc(1, sizeof(*decoded));
|
||||||
|
decoded->begin_round = 6000ULL;
|
||||||
|
decoded->summary = NULL;
|
||||||
|
decoded->decisions
|
||||||
|
= (replay_decision_t**)calloc(2, sizeof(*(decoded->decisions)));
|
||||||
|
decoded->decisions[0] = make_dispatch_entry_from_alloc(jalloc_matched);
|
||||||
|
decoded->decisions[1]
|
||||||
|
= make_dispatch_entry_from_alloc(jalloc_already_new);
|
||||||
|
decoded->n_decisions = 2;
|
||||||
|
/* job_no_match (7051) intentionally has no entry in this round's
|
||||||
|
* decoded stream at all. */
|
||||||
|
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
|
||||||
|
|
||||||
|
/* job_matched: captured, and reflects EXACTLY its own jobid's data. */
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
jalloc_matched, job_matched->allocs,
|
||||||
|
"job_matched must capture its own recorded DISPATCH entry");
|
||||||
|
TEST_ASSERT_NOT_NULL(job_matched->accounts);
|
||||||
|
TEST_ASSERT_EQUAL_STRING_MESSAGE(
|
||||||
|
"workerA", job_matched->accounts->accounts[0]->host,
|
||||||
|
"job_matched's captured account must reflect its OWN entry's "
|
||||||
|
"host, not another job's");
|
||||||
|
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
|
||||||
|
5.0f, job_matched->accounts->accounts[0]->accounts[0]->reserved,
|
||||||
|
"job_matched's captured account must reflect its OWN entry's "
|
||||||
|
"reserved value, not another job's");
|
||||||
|
|
||||||
|
/* job_no_match: stays uncaptured -- no entry existed for it. */
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job_no_match->accounts,
|
||||||
|
"job_no_match has no matching recorded entry this round -- must "
|
||||||
|
"stay uncaptured");
|
||||||
|
TEST_ASSERT_NULL(job_no_match->allocs);
|
||||||
|
|
||||||
|
/* job_already: completely unchanged, despite a matching-jobid DISPATCH
|
||||||
|
* entry for it being present this round -- proving no accidental
|
||||||
|
* overwrite/cross-contamination from the wider per-round scan. */
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
already_accounts_before, job_already->accounts,
|
||||||
|
"job_already's accounts must be the IDENTICAL pointer from before "
|
||||||
|
"this call -- never overwritten, even with a matching entry "
|
||||||
|
"present this round");
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
already_allocs_before, job_already->allocs,
|
||||||
|
"job_already's allocs must likewise be the IDENTICAL pointer from "
|
||||||
|
"before this call");
|
||||||
|
|
||||||
|
/* job_already's own matching entry was never even searched for (its
|
||||||
|
* job->accounts != NULL guard skips it before the search begins), so
|
||||||
|
* it must remain fully intact and unconsumed in decoded_session -- only
|
||||||
|
* job_matched's entry was consumed. */
|
||||||
|
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||||
|
1, decoded->n_decisions,
|
||||||
|
"exactly one entry (job_matched's) must be consumed -- "
|
||||||
|
"job_already's entry is left untouched since that job was never "
|
||||||
|
"eligible to be searched for at all");
|
||||||
|
TEST_ASSERT_EQUAL_UINT64_MESSAGE(
|
||||||
|
7052ULL, decoded->decisions[0]->body.dispatch->alloc->jobid,
|
||||||
|
"the remaining entry must be job_already's own, untouched, entry");
|
||||||
|
TEST_ASSERT_EQUAL_PTR_MESSAGE(
|
||||||
|
jalloc_already_new, decoded->decisions[0]->body.dispatch->alloc,
|
||||||
|
"job_already's entry's alloc must be the SAME pointer it started "
|
||||||
|
"with -- never taken over, since that job was already captured");
|
||||||
|
|
||||||
|
printf("test_reconcile_multiple_jobs_captured_independently_across_"
|
||||||
|
"different_rounds: PASS\n");
|
||||||
|
}
|
||||||
|
|
||||||
int main(void)
|
int main(void)
|
||||||
{
|
{
|
||||||
UNITY_BEGIN();
|
UNITY_BEGIN();
|
||||||
|
|
@ -1120,5 +1394,10 @@ int main(void)
|
||||||
test_reconcile_reports_reconstructed_decision_through_existing_channel);
|
test_reconcile_reports_reconstructed_decision_through_existing_channel);
|
||||||
RUN_TEST(test_reconcile_null_args_are_a_safe_noop);
|
RUN_TEST(test_reconcile_null_args_are_a_safe_noop);
|
||||||
RUN_TEST(test_captured_pend_job_excluded_from_resinuse_until_run);
|
RUN_TEST(test_captured_pend_job_excluded_from_resinuse_until_run);
|
||||||
|
RUN_TEST(
|
||||||
|
test_reconcile_still_skips_already_captured_job_regardless_of_state);
|
||||||
|
RUN_TEST(test_reconcile_no_match_leaves_pend_job_uncaptured_and_retries);
|
||||||
|
RUN_TEST(
|
||||||
|
test_reconcile_multiple_jobs_captured_independently_across_different_rounds);
|
||||||
return UNITY_END();
|
return UNITY_END();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue