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

fix(controller): WAL_JOB_PREASON loses job.pend_reasons on replay (bug-1692)

Browse source 
Last of the five field-parity gaps from the original audit (bug-1678,
bug-1687, bug-1689, bug-1690, bug-1691). set_preason()/clear_preason()
set job.pend_reasons (host/resource-based pending-job explanations), but
persist_job_hook_fun's default: arm only ever captured {jobid, state}.
Neither apply seam restored pend_reasons after a restart.

Reuses the ALREADY-EXISTING JobPendReason/HostBasedReason/ResourceBasedReason
capnp structs -- no schema change needed, unlike bug-1690's WAL_JOB_RUSAGE.
Safety-checked against bug-1690's lesson (RTTI/makeString()-backed fields
need merge-not-replace, not free-and-replace): pend_reasons is plain
malloc/strdup-backed throughout (erase_job_preason is FREEUP()-only,
add_host_preason/add_res_preason use strdup()/calloc()), so a
straightforward free-and-replace is safe here, matching bug-1678/1687's
pattern instead.

New wrinkle beyond the four prior fixes: pend_reasons has a genuine CLEAR
operation, and "trailing bytes present or absent" can't distinguish a
legitimate clear from an old-format payload -- added an explicit presence
byte (0=cleared/adopt NULL, 1=capnp bytes follow), verified by two
dedicated "clear survives restart" tests.

11 new tests in test_wal_job_preason_fact.c; RED/GREEN independently
verified (7 genuine pre-fix failures, one test's own wrong premise about
capnp's garbage-byte behavior caught and corrected during RED/GREEN
iteration). Full controller suite: 1178 tests, 2 failures -- both the
pre-existing, unrelated bug-1561 overlayfs sandbox flake. No ASAN findings.

This closes the field-complete apply parity invariant class the
xce-audit-invariants controller dimension 7 was added for.

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-25 13:14:28 +00:00
parent 1b86e04078
commit d0e5f85aa8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
7 changed files with 1465 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

49
src/controller/job.c
View file

@ -1067,6 +1067,55 @@ int restore_job_ckpt_state(cluster_t* c, uint64_t jobid, uint32_t state,
return 0;
}
/* bug-1692: WAL_JOB_PREASON's own sibling of restore_job_ckpt_state() above,
* for apply_wal_entry()'s standby catch-up / replicated-apply seam.
*
* has_field distinguishes a pre-fix bare {jobid,state} payload (has_field=0,
* pend_reasons left untouched -- full backward compatibility) from a
* post-fix payload that explicitly carries pend_reasons' new value
* (has_field=1): preason non-NULL adopts a freshly-decoded value, preason
* NULL means the field was genuinely cleared (do_clr_preason) and
* pend_reasons should be reset to NULL too -- pend_reasons is the one
* field-parity fact type in this family (bug-1678/1687/1689/1690/1691) with
* a real CLEAR operation, so unlike restore_job_ckpt_state()/
* restore_job_rusage_state() a NULL decoded value is NOT automatically
* "nothing to adopt".
*
* preason (when non-NULL) is always freshly heap-allocated by
* decode_jobpreason() (persist.c) -- plain calloc()/strdup() throughout
* (verified via erase_job_preason()'s FREEUP-only release and every
* add_*_preason() constructor in src/lib/preason.c, plus the
* capnp-generated decode_HostBasedReason/decode_ResourceBasedReason using
* STRING_DUP+calloc), NOT xstring/RTTI-backed like ckptinfo or rusage's
* host_name -- so a straightforward free-old-then-adopt-new via
* free_JobPendReason_ptr() is safe here, no merge-not-replace treatment
* needed (unlike bug-1690's WAL_JOB_RUSAGE). Ownership of preason always
* transfers to job->job.pend_reasons on success; released here if the job
* is gone. */
int restore_job_preason_state(cluster_t* c, uint64_t jobid, uint32_t state,
int has_field, xce_job_preason_t* preason)
{
job_t* j = NULL;
lock(c->jobmgr, RTTI_LOCK_READ);
key_find(c->jobmgr, (void*)jobid, (void**)&j);
unlock(c->jobmgr);
if (!j) {
if (preason != NULL)
free_JobPendReason_ptr(&preason);
return -ENOENT;
}
lock(j, RTTI_LOCK_WRITE);
j->job.state = (int)state;
if (has_field) {
if (j->job.pend_reasons != NULL)
free_JobPendReason_ptr(&(j->job.pend_reasons));
j->job.pend_reasons = preason; /* NULL (cleared) or the decoded value */
}
unlock(j);
return 0;
}
static int get_nslots_in_alloc(xce_host_alloc_t* alloc)
{
int i;