fix(controller): size the config-push barrier for loaded hosts and admit its wait (bug-1914)
The configAck byte-delivery barrier waited 500 ms, below scheduling delay on an oversubscribed host, so a push to a healthy majority was rejected with -110 while its already-appended marker still committed. CONF_PUSH_TIMEOUT_MS is now 2000 ms, the wait holds a commit-wait admission slot (new commit_blocking_admit/release; a full budget fails fast with not-ready), and the timeout reply no longer claims the change was not committed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
431a6d709e
commit
06e99b9081
6 changed files with 101 additions and 4 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) */
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue