fix(xce-test-ha): bug-1420 harness jobid collision + bug-1419 regression coverage

xce_test's cmd_jsub hardcoded idgen(&idctx, 0, 0, ...) (clusterId=apiserverId=0),
and run-ha-soak.sh's submit_jobs() spawns a fresh xce_test process per retry
attempt, resetting idgen's static sequence counter each time. Two unrelated
job submissions from separate process launches landing in the same
wall-clock second collided on an identical jobid, surfacing as a spurious
APPLY_DIVERGENT fence in chaotic soak that was initially misattributed to
bug-1419's controller-side dedup logic.

Confirmed not a production risk: apiserver's GenerateJobID holds one
process-lifetime idgen_ctx_t via sync.Once, and each deployed instance has
its own distinct apiserverId, so config.h's jobid bit layout already
partitions the production namespace. Fixed by folding getpid() into the
harness's apiserverId so cross-process collisions become structurally
impossible there too.

Also adds test_jobsubmit_resubmit_after_full_resolution_rejected_no_second_job
to test_commit_barrier_reply.c, proving the post-resolution dedup window
(retry arriving after commit+apply+reply+release) is airtight, complementing
the existing before-resolution test -- both known bug-1419 dedup windows are
now provably race-free, confirming those fixes were never the cause of this
soak failure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgT3n4vTv4ZjCuCTmzed1F
This commit is contained in:
Rongsong Shen 2026-07-08 04:49:41 +00:00
parent 0eb105431b
commit 1b6041acef
3 changed files with 137 additions and 4 deletions

File diff suppressed because one or more lines are too long

View file

@ -1865,7 +1865,19 @@ int cmd_jobsub(int argc, char* argv[])
submit.mintasks = min;
submit.maxtasks = max;
idgen(&idctx, 0, 0, arsize, &(jobsubmit.jobid));
/* apiserverId=0 here let two unrelated jobs collide on the same jobid
* whenever the harness spawns a fresh xce_test process within the same
* wall-clock second (idgen_ctx_init resets sequence to 0 each time,
* and ts/tag/seq alone were then identical for both processes' first
* submission). Production apiserver instances never hit this: each
* holds one process-lifetime idgen_ctx_t (clib.go's sync.Once) and is
* assigned its own distinct apiserverId, so the jobid namespace is
* already partitioned by config.h's bit layout. Folding this process's
* pid into apiserverId gives each xce_test invocation its own slice of
* that same partitioning, making a same-second cross-process collision
* structurally impossible here too. */
idgen(&idctx, 0, (size_t)((uint64_t)getpid() & MAX_APISERVER_ID), arsize,
&(jobsubmit.jobid));
if (NULLSTR(submit.start_info->execuser)) {
submit.start_info->execuser = strdup(submit.user);

View file

@ -1305,6 +1305,82 @@ static void test_jobsubmit_resubmit_before_resolution_rejected_no_dup_wal_entry(
printf("test_jobsubmit_resubmit_before_resolution_rejected_no_dup_wal_entry: PASS\n");
}
/* bug-1419 residual: does the SAME dedup guard still hold AFTER the first
* attempt has fully resolved (committed, applied, replied, in-flight claim
* released via jobsubmit_async_ctx_release) -- not just DURING the
* still-unresolved window the test above proves. A live chaotic-soak run
* (2026-07-08) showed a genuine WAL-level jobid duplicate (two committed
* WAL_JOB_SUBMIT entries, same term, same primary) that neither known guard
* should have let through if the retry arrived strictly before OR strictly
* after resolution -- this test pins down whether the "after" window is as
* airtight as jobid_range_exists()'s own doc comment claims, by driving the
* first attempt all the way to a fully-applied, fully-replied,
* fully-released state before firing the retry. */
static void test_jobsubmit_resubmit_after_full_resolution_rejected_no_second_job(void)
{
const uint64_t jobid = 9150;
uint64_t lsn_before, lsn_before_second, watermark, watermark_second;
cbrp_reply_obs_t obs = { 0 };
int rc_second;
cbrp_setup(2000);
cbrp_arrange_jobsubmit(jobid, 1);
pthread_mutex_lock(&g_hc.persist.lock);
lsn_before = g_hc.persist.current_lsn;
pthread_mutex_unlock(&g_hc.persist.lock);
/* First attempt: append, commit (ACK delivered), apply, and reply --
* drive it all the way to the fully-resolved, fully-released state. */
g_op_arg.fn = handle_jobsubmit;
g_op_arg.req = &g_jobsub_req;
g_op_arg.rc = -9999;
TEST_ASSERT_EQUAL_INT(0, pthread_create(&g_op_tid, NULL,
cbrp_op_thread_fn, &g_op_arg));
TEST_ASSERT_EQUAL_INT(0, pthread_join(g_op_tid, NULL));
watermark = cbrp_poll_append(&g_hc.persist, lsn_before, 1000);
TEST_ASSERT_TRUE_MESSAGE(watermark > 0,
"the first attempt must append its WAL entry");
repl_on_ack(&g_hc.persist.repl, &g_hep_a, watermark);
cbrp_poll_reply(&g_req_ep, 2000, &obs);
TEST_ASSERT_TRUE_MESSAGE(obs.n_replies > 0,
"the first attempt must reach a replied, fully-applied state "
"before the retry below fires");
TEST_ASSERT_TRUE_MESSAGE(cbrp_job_exists(jobid),
"the first attempt's job must be visible in c->jobmgr before the "
"retry below fires -- otherwise this test does not actually "
"exercise the AFTER-resolution window");
/* Second attempt: the retry arrives strictly AFTER the first has fully
* committed, applied, replied, and released its in-flight claim -- the
* window jobid_range_exists()'s own doc comment claims is "airtight
* against a genuinely completed prior submission". */
cbrp_arrange_jobsubmit(jobid, 1);
pthread_mutex_lock(&g_hc.persist.lock);
lsn_before_second = g_hc.persist.current_lsn;
pthread_mutex_unlock(&g_hc.persist.lock);
rc_second = handle_jobsubmit(&g_hc, &g_req_ep, &g_jobsub_req);
watermark_second = cbrp_poll_append(&g_hc.persist, lsn_before_second, 300);
cbrp_teardown();
TEST_ASSERT_EQUAL_INT_MESSAGE(-EEXIST, rc_second,
"a resubmit for a jobid already fully committed+applied must be "
"rejected -EEXIST via jobid_range_exists()");
TEST_ASSERT_EQUAL_INT_MESSAGE(0, (int)watermark_second,
"a rejected post-resolution resubmit must NOT append a second "
"WAL_JOB_SUBMIT entry for the same jobid");
printf("test_jobsubmit_resubmit_after_full_resolution_rejected_no_second_job:"
" PASS\n");
}
/* The explicit-FAILED (non-timeout) sibling of the repeated-timeout test
* above: repeats the truncation-forced FAILED path
* CBRP_JOBSUBMIT_REPEAT_N times in the same fixture, proving the
@ -3270,6 +3346,7 @@ void test_commit_barrier_reply(void)
RUN_TEST(test_jobsubmit_truncation_forces_failed_no_phantom_job);
RUN_TEST(test_jobsubmit_repeated_timeout_failures_no_phantom_jobs);
RUN_TEST(test_jobsubmit_resubmit_before_resolution_rejected_no_dup_wal_entry);
RUN_TEST(test_jobsubmit_resubmit_after_full_resolution_rejected_no_second_job);
RUN_TEST(test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs);
RUN_TEST(test_jobsubmit_array_mixed_outcome_no_phantom_job_for_failed_elements);
/* handle_jobsubmit success path via the ordered apply pump