wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

test(raft-apply-decoupling): update stale apply-before-commit assertions (task 7.3)

Browse source 
Full 79-group unit/integration suite re-run found exactly 2 stale tests in
test_commit_barrier_reply.c asserting task 3.2's documented interim
contract ("no observable effect until task 6 completes it"), which the
already-committed ordered apply pump (tasks 6.1/6.2) closed by construction.
Updated both to assert the real, deterministic apply-after-commit settled
state instead of the no-longer-true no-effect-yet state, matching the idiom
already used elsewhere in the same file. No production code changed.

Full suite: 0 failures across 79 groups. HA soak (--sequential, 5 rounds):
zero committed-job loss, zero divergence, all 3 nodes served as primary.
Both independently re-verified by adversarial review.
This commit is contained in:
Rongsong Shen 2026-07-07 03:38:05 +00:00
parent a5e29a6ffa
commit 16b890b1af
2 changed files with 74 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

137
src/controller/testsuite/test_commit_barrier_reply.c
View file

@ -922,21 +922,30 @@ static void test_jobsubmit_withheld_acks_no_phantom_job(void)
printf("test_jobsubmit_withheld_acks_no_phantom_job: PASS\n");
}
/* raft-apply-decoupling task 3.2: supersedes the pre-3.2
* test_jobsubmit_success_reply_and_job_visible_after_commit (which drove
* handle_jobsubmit through cbrp_drive_acked, a helper that asserts the
* handler blocks until its reply is observable — no longer true here by
* this task's own explicit contract). This test asserts ONLY what task 3.2
* itself makes observable on the committed-outcome path: the calling
* thread returns before the commit resolves, and the continuation hands
* the entry off to persist's pending-apply registry keyed by the correct
* lsn — NOT a reply, NOT a constructed job (task 6 owns both; neither
* exists yet). */
/* raft-apply-decoupling task 3.2 (original intent, superseded by task 6.1/
* 6.2 -- see raft-apply-decoupling task 7.3, Req 11.1): supersedes the
* pre-3.2 test_jobsubmit_success_reply_and_job_visible_after_commit (which
* drove handle_jobsubmit through cbrp_drive_acked, a helper that asserts
* the handler blocks until its reply is observable — no longer true here).
* This test still asserts task 3.2's own contract (the calling thread
* returns before the commit resolves, and the WAL entry is appended before
* that return); it no longer asserts task 3.2's INTERIM "no reply / no job
* yet" gap on the committed-outcome path, because the ordered apply pump
* built by task 6.1/6.2 now closes that gap: the pump consumes the
* pending-apply hand-off, applies the entry via apply_committed_entry(),
* and fires the client's success reply — all strictly after the entry
* commits, never before or during the calling thread's earlier return.
* (Catching persist_has_pending_apply()'s transient true window from a
* second thread is inherently racy against the pump's own consumption of
* it — proven flaky empirically across repeated runs of this test — so
* this test asserts the deterministic settled state instead, the same
* poll-for-reply idiom test_jobsubmit_success_reply_after_apply_carries_
* jobid already uses for the identical ordering proof.) */
static void test_jobsubmit_returns_before_commit_and_registers_pending_apply(void)
{
uint64_t lsn_before, watermark, t0, join_elapsed_ms, t1;
uint64_t lsn_before, watermark, t0, join_elapsed_ms;
cbrp_reply_obs_t obs = { 0 };
bool has_pending, job_exists;
bool job_exists;
cbrp_setup(2000); /* generous timeout: must never be needed */
cbrp_arrange_jobsubmit(9101, 1);
@ -961,27 +970,13 @@ static void test_jobsubmit_returns_before_commit_and_registers_pending_apply(voi
watermark = cbrp_poll_append(&g_hc.persist, lsn_before, 2000);
/* Deliver the covering ACK, then poll for the hand-off artifact
* (raft-apply-decoupling task 3.2's producer-side registration) —
* NOT for a reply or a constructed job, neither of which this task
* produces on the committed-outcome path. */
/* Deliver the covering ACK, then poll for the settled, end-to-end
* committed-outcome effect: the ordered apply pump applies the entry
* and fires the client's success reply (raft-apply-decoupling task
* 6.1/6.2). */
repl_on_ack(&g_hc.persist.repl, &g_hep_a, watermark);
cbrp_poll_reply(&g_req_ep, 2000, &obs);
has_pending = false;
t1 = cbrp_mono_ms();
while (cbrp_mono_ms() - t1 < 2000) {
if (persist_has_pending_apply(&g_hc.persist, watermark)) {
has_pending = true;
break;
}
cbrp_sleep_ms(5);
}
/* Give the (non-existent) rest of the pipeline a beat, then confirm
* nothing leaked through — the task's own explicit "no independently
* observable behavior" contract for the committed-outcome path. */
cbrp_sleep_ms(200);
cbrp_scan_replies(&g_req_ep, &obs);
job_exists = cbrp_job_exists(9101);
cbrp_teardown();
@ -992,16 +987,16 @@ static void test_jobsubmit_returns_before_commit_and_registers_pending_apply(voi
"delivered before this join");
TEST_ASSERT_TRUE_MESSAGE(watermark > 0,
"job submission must still append its WAL entry");
TEST_ASSERT_TRUE_MESSAGE(has_pending,
"a committed entry must be handed off to the ordered apply pump "
"via persist_register_pending_apply, keyed by its lsn "
"(raft-apply-decoupling task 3.2)");
TEST_ASSERT_EQUAL_INT_MESSAGE(0, obs.n_replies,
"no reply may be sent on the committed-outcome path until task 6's "
"apply pump exists (raft-apply-decoupling task 3.2's own contract)");
TEST_ASSERT_FALSE_MESSAGE(job_exists,
"no job may be constructed on the committed-outcome path until "
"task 6's apply pump exists (raft-apply-decoupling task 3.2)");
TEST_ASSERT_TRUE_MESSAGE(obs.n_replies > 0,
"a committed entry must reach the client with a reply once the "
"ordered apply pump consumes its pending-apply hand-off "
"(raft-apply-decoupling task 6.1/6.2, Req 2.2)");
TEST_ASSERT_EQUAL_INT_MESSAGE(0, obs.last_status,
"a committed, applied job submission must reply success");
TEST_ASSERT_TRUE_MESSAGE(job_exists,
"the job must be visible in the in-memory job table once the "
"ordered apply pump applies its committed entry "
"(raft-apply-decoupling task 6.1/6.2, Req 2.2)");
printf("test_jobsubmit_returns_before_commit_and_registers_pending_apply:"
" PASS\n");
@ -1315,15 +1310,34 @@ static void test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs(v
* exists yet"): element 1 commits (covering ACK) while elements 2 and 3
* are left to time out FAILED. Confirms the failed elements produce zero
* phantom jobs and that the committed element's independent resolution
* (hand-off to persist_register_pending_apply) is not blocked or skipped
* by its failing siblings -- task 3.2's per-element independence, proven
* by a real test rather than by code inspection alone. */
* is not blocked or skipped by its failing siblings -- task 3.2's
* per-element independence, proven by a real test rather than by code
* inspection alone.
*
* raft-apply-decoupling task 7.3 (Req 11.1) update: this test originally
* asserted (task 3.2, before task 6.1/6.2 existed) that the committed
* element had NO job yet, only a pending-apply hand-off -- catching that
* hand-off's transient window from a second thread is inherently racy
* against the ordered apply pump's own consumption of it (proven flaky
* empirically across repeated runs of this test: sometimes the window
* was caught, sometimes it was already consumed by the time this test
* polled). Now that the pump exists, the deterministic, settled effect is
* that element 1's mutation applies (job1 becomes visible in c->jobmgr)
* independently of its failing siblings, exactly like any other committed
* entry -- the per-element independence this test exists to prove now
* extends from "resolution" to "apply", per the design's own stated
* intent that a command's apply is per-entry and never gated on sibling
* array elements (collapsing that behavior is job-array-single-wal-
* record's job, not this feature's). The client-visible batch reply is
* unaffected: it is still the single failure reply from the timed-out
* siblings, because jobsubmit_reply_success_once() is gated on the same
* actx->replied flag as jobsubmit_reply_failure_once() (job.c), and the
* failure reply always wins that race here. */
static void test_jobsubmit_array_mixed_outcome_no_phantom_job_for_failed_elements(void)
{
const uint64_t base_jobid = 9140;
uint64_t lsn_before, watermark, lsn1, t1;
uint64_t lsn_before, watermark, lsn1;
int create_rc, join_rc;
bool has_pending1;
cbrp_reply_obs_t obs = { 0 };
bool job1_exists, job2_exists, job3_exists;
@ -1353,16 +1367,10 @@ static void test_jobsubmit_array_mixed_outcome_no_phantom_job_for_failed_element
* independently of element 1's success. */
repl_on_ack(&g_hc.persist.repl, &g_hep_a, lsn1);
has_pending1 = false;
t1 = cbrp_mono_ms();
while (cbrp_mono_ms() - t1 < 2000) {
if (persist_has_pending_apply(&g_hc.persist, lsn1)) {
has_pending1 = true;
break;
}
cbrp_sleep_ms(5);
}
/* Poll for the batch's single settled reply (the failing siblings'
* timeout), then read the deterministic end state of every element's
* job existence -- not a transient hand-off window (see doc comment
* above). */
cbrp_poll_reply(&g_req_ep, 300 + 1500, &obs);
job1_exists = cbrp_job_exists(base_jobid);
@ -1375,21 +1383,20 @@ static void test_jobsubmit_array_mixed_outcome_no_phantom_job_for_failed_element
TEST_ASSERT_EQUAL_INT_MESSAGE(0, join_rc, "pthread_join must succeed");
TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_before + 2, watermark,
"all three array elements must append before this poll returns");
TEST_ASSERT_TRUE_MESSAGE(has_pending1,
"a sibling array element that commits must still be handed off to "
"the pending-apply registry even though other siblings in the "
"same batch are failing (raft-apply-decoupling task 3.2's "
"per-element independent resolution)");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, obs.n_replies,
"a partially-failed array batch must produce exactly one failure "
"reply for the whole request (jobsubmit_reply_failure_once)");
TEST_ASSERT_EQUAL_INT_MESSAGE(-ETIMEDOUT, obs.last_status,
"the batch's single reply must carry the failing elements' "
"-ETIMEDOUT outcome");
TEST_ASSERT_FALSE_MESSAGE(job1_exists,
"no element may have a constructed job yet -- task 6's apply pump "
"does not exist, so even the committed element has no job; this "
"test proves independent per-element RESOLUTION, not apply");
TEST_ASSERT_TRUE_MESSAGE(job1_exists,
"the committed array element's mutation must apply independently "
"of its failing siblings, exactly like any other committed entry, "
"now that the ordered apply pump exists (raft-apply-decoupling "
"task 6.1/6.2) -- the client's batch reply still reports failure "
"because a failing sibling wins jobsubmit_async_ctx_t's "
"single-reply race, but that reply does not gate this element's "
"independent apply");
TEST_ASSERT_FALSE_MESSAGE(job2_exists,
"a failed array element must never have a phantom job");
TEST_ASSERT_FALSE_MESSAGE(job3_exists,