lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(scheduler-sim-reconcile-terminal-guard): task 1.2 - bug-1764 terminal-state guard

Browse source 
sim_reconcile_unaccounted_jobs()'s per-job trigger check (sim_loop.c)
now also excludes JOB_IS_DONE(job->state) || JOB_IS_EXIT(job->state),
alongside the existing job->accounts != NULL skip. A terminal job is
never searched for a matching recorded DISPATCH entry, so a leftover
or duplicate DISPATCH decision from a later replay round can no
longer resurrect allocs/accounts on a job that has already completed
- the exact mechanism behind info worker's assigned-jobs listing
showing a finished job as still consuming a worker's resources.

The exclusion mirrors job_state_regresses_terminal()'s existing
"terminal is permanent" invariant (core_events.c) rather than
inventing a new rule. The function's doc comment gains a paragraph
clarifying the terminal exclusion is complementary to, not a reversal
of, the existing PEND/RUN state-independence rationale that motivated
this trigger's original widening.

Both RED-phase tests (task 1.1) pass; the pre-existing PEND
eager-capture regression test and core_events.c's JOBS_PUSH
resync-merge terminal-state tests are unaffected. The analogous
resync-merge branch was investigated and confirmed already correct.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
Rongsong Shen 2026-09-03 07:15:42 +00:00
parent ff05c37bca
commit b722de4f08
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
3 changed files with 72 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

66
src/scheduler/src/sim_loop.c
View file

@ -586,26 +586,49 @@ static void* sim_picker_thread_fn(void* arg)
* site's own comment below. Still directly unit-testable standalone too
* (test_sim_reconcile_unaccounted.c), per design.md's own Testing Strategy.
*
* Trigger condition (scheduler-sim-eager-reconciliation spec, task 1.1):
* `job->accounts == NULL` is now the SOLE capture-eligibility gate --
* state-independent, reused here as both the trigger AND (now that the
* FOUND branch below populates job->accounts) the double-application guard.
* Any job in `sim->core.jobs` with `job->accounts == NULL` is eligible to be
* searched for a matching recorded DISPATCH decision this round, regardless
* of its current `job->state` -- a job's recorded DISPATCH decision and its
* RUN-state confirmation can legitimately land many rounds apart in the
* original recording (the dominant real-world case), and this function is
* called unconditionally every round, so a not-yet-running job is simply
* re-checked round after round until its originating decision is found (or
* the replay ends). Actual resource charging remains fully deferred to
* sched_account_jobs()'s own unchanged `JOB_IS_RUN` gate (scheduler.c) --
* capturing a job's allocation here while it is not yet RUN is inert until
* that gate later includes it; this function performs no charging itself.
* Requirement 3.3's "report only the fresh one" guarantee needs no separate
* mechanism here: a job that independently receives a fresh live decision in
* this same round already has job->accounts set by sched_commit_decision()
* (scheduler.c, unchanged) by the time this trigger check runs, so this
* function simply finds nothing to do for it.
* Trigger condition (scheduler-sim-eager-reconciliation spec, task 1.1;
* narrowed by scheduler-sim-reconcile-terminal-guard task 1.2, bug-1764):
* `job->accounts == NULL` is the primary capture-eligibility gate --
* state-independent WITH RESPECT TO PEND vs. RUN, reused here as both the
* trigger AND (now that the FOUND branch below populates job->accounts) the
* double-application guard. Any job in `sim->core.jobs` with `job->accounts
* == NULL` is eligible to be searched for a matching recorded DISPATCH
* decision this round, regardless of whether it is PEND or RUN -- a job's
* recorded DISPATCH decision and its RUN-state confirmation can legitimately
* land many rounds apart in the original recording (the dominant real-world
* case), and this function is called unconditionally every round, so a
* not-yet-running job is simply re-checked round after round until its
* originating decision is found (or the replay ends). Actual resource
* charging remains fully deferred to sched_account_jobs()'s own unchanged
* `JOB_IS_RUN` gate (scheduler.c) -- capturing a job's allocation here while
* it is not yet RUN is inert until that gate later includes it; this
* function performs no charging itself. Requirement 3.3's "report only the
* fresh one" guarantee needs no separate mechanism here: a job that
* independently receives a fresh live decision in this same round already
* has job->accounts set by sched_commit_decision() (scheduler.c, unchanged)
* by the time this trigger check runs, so this function simply finds
* nothing to do for it.
*
* This PEND/RUN state-independence does NOT extend to a job that has
* already reached a terminal state: `JOB_IS_DONE(job->state) ||
* JOB_IS_EXIT(job->state)` is an additional, unconditional exclusion,
* checked alongside `job->accounts != NULL` in the same trigger `if`. A
* terminal job's `job->accounts`/`job->allocs` are freed by
* xce_core_push_events()'s own end-of-life cleanup on the terminal
* transition (core_events.c's JOB_STATE_CHANGE/JOB_UPDATE cases), which
* leaves `job->accounts == NULL` -- the exact same shape as a legitimately
* not-yet-captured PEND/RUN job. Without this exclusion, a later round's
* decoded_session carrying a stale/duplicate DISPATCH entry for that same
* jobid (e.g. a retried dispatch attempt for one element of an array job)
* would be indistinguishable from a genuine early-dispatch case and would
* resurrect the allocation on an already-dead job (bug-1764). The two rules
* compose without contradiction: state-independence is about WHEN a
* (still-live) job's own DISPATCH decision happens to replay relative to
* its RUN confirmation -- it says nothing about a job that can never
* legitimately be reconsidered again because it has already finished. This
* mirrors job_state_regresses_terminal()'s (core_events.c) existing
* "terminal is permanent" invariant, applied here at the allocation layer
* instead of the state-transition layer.
*
* For each such job, searches `decoded_session->decisions` for a matching
* DISPATCH entry by jobid -- a job can only ever be captured via a recorded
@ -687,7 +710,8 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
break;
}
if ((job == NULL) || (job->accounts != NULL)) {
if ((job == NULL) || (job->accounts != NULL) || JOB_IS_DONE(job->state)
|| JOB_IS_EXIT(job->state)) {
continue;
}