diff --git a/src/controller/commit.c b/src/controller/commit.c index 75bf781a..89029347 100644 --- a/src/controller/commit.c +++ b/src/controller/commit.c @@ -282,6 +282,27 @@ static void wait_release_locked(commit_engine_t* ce) } } +int commit_blocking_admit(commit_engine_t* ce) +{ + int rc; + + if (ce == NULL) + return -EINVAL; + pthread_mutex_lock(&ce->lock); + rc = wait_admit_locked(ce); + pthread_mutex_unlock(&ce->lock); + return rc; +} + +void commit_blocking_release(commit_engine_t* ce) +{ + if (ce == NULL) + return; + pthread_mutex_lock(&ce->lock); + wait_release_locked(ce); + pthread_mutex_unlock(&ce->lock); +} + /* WAIT completion predicate (Req 1.1): own entry committed AND the * own-term gate (when set) reached. Caller holds ce->lock. */ static int wait_done_locked(const commit_engine_t* ce, uint64_t lsn) diff --git a/src/controller/config_repl.c b/src/controller/config_repl.c index f1538962..0acd50f1 100644 --- a/src/controller/config_repl.c +++ b/src/controller/config_repl.c @@ -1839,9 +1839,13 @@ static int do_upgrade_config_with_bytes(cluster_t *c, endpoint_t *ep, /* Forward the barrier rc (-ETIMEDOUT / -EINVAL) as the reply * status so the admin tool sees WHY the commit failed (task 5.2 * r1, reviewer finding 3). */ + /* bug-1914: the marker may already be in the log (it is appended + * before the byte-delivery barrier), so a timeout here does not + * mean the change is void -- say so instead of "not committed". */ send_upgrade_error(ep, req, c, rc, - "Config marker failed to reach committed prefix" - " -- upgrade not committed, no restart"); + "Config change not confirmed on a majority -- if it" + " was already ordered in the log it may still" + " activate; re-push the same config to confirm"); return rc; } diff --git a/src/controller/config_repl.h b/src/controller/config_repl.h index bc026231..ac247ff1 100644 --- a/src/controller/config_repl.h +++ b/src/controller/config_repl.h @@ -28,9 +28,14 @@ typedef struct persist_s persist_t; #define CONF_ENABLE_UPGRADE_CONFIG 1 #endif -/* Push timeout for standby fan-out (ms) */ +/* Push timeout for standby fan-out and the configAck majority barrier (ms). + * bug-1914: 500 ms was below normal scheduling delay on an oversubscribed host + * (40 busy loops on 32 cores: the ACK landed ~575 ms in), so a push to a + * healthy majority was rejected while its marker still committed. The + * barrier waits under the commit-wait admission reservation, so the longer + * bound cannot take workers reserved for liveness traffic. */ #ifndef CONF_PUSH_TIMEOUT_MS -#define CONF_PUSH_TIMEOUT_MS 500 +#define CONF_PUSH_TIMEOUT_MS 2000 #endif /* Certificate expiry warning threshold (days) */ diff --git a/src/controller/include/commit.h b/src/controller/include/commit.h index c642af2b..9857c466 100644 --- a/src/controller/include/commit.h +++ b/src/controller/include/commit.h @@ -150,6 +150,16 @@ void commit_set_pool(commit_engine_t* ce, vtaskmgr_t* pool); int commit_submit(commit_engine_t* ce, uint64_t lsn, commit_mode_t mode, uint32_t timeout_ms); +/* commit_blocking_admit / commit_blocking_release (bug-1914): take / return + * one slot of the same blocking-wait admission budget commit_submit's WAIT + * mode uses, for a bounded blocking wait that is not itself a commit wait + * (the configAck byte-delivery barrier, repl_wait_majority_ack). Bounded + * blocking on a vtask is allowed only under this reservation + * (ha-async-io.md). admit returns 0 or -XCE_ERR_NOT_READY (budget + * exhausted, never blocks); every successful admit needs one release. */ +int commit_blocking_admit(commit_engine_t* ce); +void commit_blocking_release(commit_engine_t* ce); + /* raft-apply-decoupling task 1.1: register a continuation for lsn instead of * parking the calling thread -- the event-driven equivalent of * commit_submit(..., COMMIT_MODE_WAIT, ...). Returns 0 on successful diff --git a/src/controller/repl.c b/src/controller/repl.c index 16c8c5fa..df906111 100644 --- a/src/controller/repl.c +++ b/src/controller/repl.c @@ -641,6 +641,17 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, return 0; } + /* bug-1914: this is a bounded blocking wait on a vtask, so it runs under + * the same admission reservation as a commit WAIT (ha-async-io.md); a + * full budget rejects fast with -XCE_ERR_NOT_READY instead of taking a + * worker reserved for liveness traffic. */ + int arc = commit_blocking_admit(r->ce); + if (arc != 0) { + WARN("repl_wait_majority_ack: configAck wait not admitted (rc=%d," + " push_tag=%016" PRIx64 ")", arc, push_tag); + return arc; + } + struct timespec deadline; clock_gettime(CLOCK_REALTIME, &deadline); deadline.tv_sec += (long)(timeout_ms / 1000); @@ -689,6 +700,7 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, } pthread_mutex_unlock(&r->ack_lock); + commit_blocking_release(r->ce); if (rc == 0) INFO("repl_wait_majority_ack: quorum reached (ack_op=%u)", ack_op); diff --git a/src/controller/testsuite/test_commit_barrier_rollout.c b/src/controller/testsuite/test_commit_barrier_rollout.c index 7af7468a..fa172d0b 100644 --- a/src/controller/testsuite/test_commit_barrier_rollout.c +++ b/src/controller/testsuite/test_commit_barrier_rollout.c @@ -1185,6 +1185,50 @@ static void test_configack_self_plus_one_covering_is_majority_of_three(void) printf("test_configack_self_plus_one_covering_is_majority_of_three: PASS\n"); } + +/* bug-1914: under heavy CPU load (40 busy loops on 32 cores) a standby's + * config ACK arrives a few hundred ms late; the 500 ms barrier rejected a push + * whose marker then committed anyway (client told -110, cluster activated + * it). An ACK landing 700 ms in must still meet the production barrier + * bound, and while it waits the barrier holds a commit-wait admission slot + * (bounded blocking on a vtask is allowed only under that reservation). */ +static atomic_int g_1914_inflight_seen; + +static void* b1914_late_config_ack(void* arg) +{ + (void)arg; + usleep(100 * 1000); + pthread_mutex_lock(&g_bce.lock); + atomic_store(&g_1914_inflight_seen, g_bce.inflight_waits); + pthread_mutex_unlock(&g_bce.lock); + usleep(600 * 1000); + repl_on_config_ack(&g_bp.repl, &g_bep_a, 0x100); + return NULL; +} + +static void test_configack_late_ack_under_load_meets_barrier(void) +{ + pthread_t th; + int rc; + + barrier_setup(3, 2, 0); + atomic_store(&g_1914_inflight_seen, -1); + TEST_ASSERT_EQUAL_INT(0, pthread_create(&th, NULL, b1914_late_config_ack, + NULL)); + rc = repl_wait_majority_ack(&g_bp.repl, 0x100, CONF_PUSH_TIMEOUT_MS, + (uint8_t)OPCode_upgradeConfigAck); + pthread_join(th, NULL); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, + "bug-1914: a config ACK 700 ms late (CPU-oversubscribed standby) must " + "still meet the config-push barrier"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_1914_inflight_seen), + "bug-1914: the configAck wait must hold a commit-wait admission slot"); + TEST_ASSERT_EQUAL_INT(0, g_bce.inflight_waits); + + barrier_teardown(); + printf("test_configack_late_ack_under_load_meets_barrier: PASS\n"); +} /* n_peers=1: single-member cluster — immediate success without consuming * the timeout. PIN: green today (via the n_active == 0 short-circuit) and * green after task 5.2 (explicit n_peers <= 1 rule). */ @@ -2530,6 +2574,7 @@ void test_commit_barrier_rollout(void) RUN_TEST(test_configack_previous_tag_ack_does_not_count); RUN_TEST(test_configack_no_active_standbys_times_out); RUN_TEST(test_configack_self_plus_one_covering_is_majority_of_three); + RUN_TEST(test_configack_late_ack_under_load_meets_barrier); RUN_TEST(test_configack_single_member_immediate_success); RUN_TEST(test_push_config_withheld_acks_returns_etimedout); RUN_TEST(test_push_config_majority_acks_returns_zero_and_commits);