Skip to content

Commit 8782cc8

Browse files
authored
fix(replication): prevent excessive wait times after deploy (fixes #131) (#137)
* fix(replication): prevent excessive wait times after deploy (fixes #131) Two issues caused multi-minute delays on 'deploy replication' in v2.4.0: 1. wait_until_wsrep_ready spun 120s per node on non-Galera MySQL (commit a2caf94 added wait_wsrep_after_start but didn't guard against vanilla MySQL where wsrep_ready does not exist) 2. wait_until_replica_ready defaulted to max_attempts=60 (60s per replica), while the original reporter found 5s was sufficient Fixes: - wait_until_wsrep_ready: check if the wsrep_ready status variable exists before polling; return immediately on non-Galera nodes - wait_until_replica_ready: reduce default max_attempts from 60 to 20 (≤20s total wait per replica) - Update call sites in init_slaves.gotxt and init_slaves_84.gotxt Tests: - TestInitSlavesTemplates_IncludeReplicaReadyWait: also check timeout warning message; fix pre-existing missing template fields - TestWaitWsrepAfterStart_DoesNotBlockNonGalera: verifies the wait_wsrep_after_start template renders and uses || true - TestSbInclude_WaitUntilWsrepReady_DetectsNonGalera: verifies the wsrep existence guard exists in sb_include - TestSbInclude_WaitUntilReplicaReady_BoundedWait: verifies max_attempts default is 20 (≤30s budget) * feat(ci): add time-bounded deploy test for issue #131 regression Adds test/deploy-time-budget.sh, a standalone bash test that: - Deploys replication (1 slave) with real MySQL binaries - Measures wall-clock time against MAX_DEPLOY_SECONDS (45s) - Verifies the replica serves queries immediately after deploy (catches the #131 race condition where deploy returned before replica was ready) - Checks for server errors in the error log Adds a deploy-time-budget CI job in integration_tests.yml that: - Downloads and unpacks MySQL 8.4.4 - Runs the time-budget test with MAX_DEPLOY_SECONDS=45 - Fails if deploy exceeds the budget (catches regressions like the 120s wsrep spin or 60s replica wait from v2.4.0) * Fix wsrep detection for fresh Galera nodes and fix deploy-time-budget -n flag - sb_include.gotxt: connect as root (no password) via socket for wsrep detection guard, so it works before msandbox user exists - deploy-time-budget.sh: use -n 2 (total nodes, not slaves) for replication with 1 slave * fix: use root socket for wsrep polling, not /use wait_until_wsrep_ready now uses root via socket for both detection and polling, since msandbox user does not exist before load_grants. This eliminates the 120s wasted polling loop per node on fresh Galera/PXC nodes while still correctly waiting for wsrep_ready=ON before grants/DML. * fix tests: template flavor checks must account for shell-level if/else The start/stop/use templates use shell if blocks (not Go template {{if}}) so both branches' literal strings appear in rendered output. Tests now verify the correct default AND the presence of the flavor conditional, rather than asserting the absence of the other flavor's binary string. * fix: harden wait_until_wsrep_ready connect and client resolution - Retry root socket connect before Galera detection so a not-yet-ready server is not mistaken for non-Galera (empty query output). - Resolve client as mariadb/mysql by FLAVOR (CLIENT_BASEDIR then BASEDIR). - Match wsrep_ready ON specifically; keep a modest connect budget (30s). * fix: address CodeRabbit findings on deploy-time-budget CI - workflow: persist-credentials: false on the executable-download job so a compromised MySQL binary cannot read the job token (artipacked). - deploy-time-budget.sh: send version-discovery diagnostics to stderr and branch on exit status so a failed discovery hits the skip path instead of leaking text into VERSION. - Require MySQL >= 8.4 (was >= 8.0) to match the stated #131 target. - Clean stale rsandbox_* fixtures before deploying so a leftover ready topology cannot make the test pass spuriously. * fix: gate wsrep non-Galera short-circuit on probe exit status Only treat an empty wsrep_ready probe as "non-Galera" when the SHOW STATUS query itself succeeds. A failed probe now falls through to the polling loop instead of bypassing the wsrep_ready wait (CodeRabbit). * fix: replica-ready probe must authenticate (clear leaked NOPASSWORD) wait_until_replica_ready connects as the sandbox user, but init_slaves exports NOPASSWORD=1 for the root steps and that leaked into the probe: 'use' then drops the password, so SELECT 1 fails with Access denied even once the user exists, and the poll burned its full ~20s budget on every replication deploy. Clear NOPASSWORD for the probe so it authenticates and returns as soon as the replica is actually ready (~1-2s).
1 parent 03b9079 commit 8782cc8

7 files changed

Lines changed: 464 additions & 65 deletions

File tree

.github/workflows/integration_tests.yml

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,64 @@ jobs:
100100
./dbdeployer delete all --skip-confirm 2>/dev/null || true
101101
pkill -9 -u "$USER" mysqld 2>/dev/null || true
102102
103+
# Time-bounded deploy test: verifies that "deploy replication" completes
104+
# within MAX_DEPLOY_SECONDS and replicas serve queries immediately after
105+
# deploy (regression test for issue #131).
106+
deploy-time-budget:
107+
name: Deploy time budget (8.4)
108+
runs-on: ubuntu-22.04
109+
env:
110+
GO111MODULE: on
111+
SANDBOX_BINARY: ${{ github.workspace }}/opt/mysql
112+
MAX_DEPLOY_SECONDS: 45
113+
steps:
114+
- uses: actions/checkout@v4
115+
with:
116+
persist-credentials: false
117+
118+
- uses: actions/setup-go@v5
119+
with:
120+
go-version: '1.23'
121+
122+
- name: Install system libraries
123+
run: |
124+
sudo apt-get update
125+
sudo apt-get install -y libaio1 libnuma1 libncurses5 libldap-2.5-0
126+
127+
- name: Build dbdeployer
128+
run: go build -o dbdeployer .
129+
130+
- name: Cache MySQL tarball
131+
uses: actions/cache@v4
132+
with:
133+
path: /tmp/mysql-tarball
134+
key: mysql-8.4.4-linux-x86_64-v1
135+
136+
- name: Download MySQL 8.4.4
137+
run: |
138+
mkdir -p /tmp/mysql-tarball
139+
TARBALL="mysql-8.4.4-linux-glibc2.17-x86_64.tar.xz"
140+
curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
141+
"https://dev.mysql.com/get/Downloads/MySQL-8.4/$TARBALL" \
142+
|| curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
143+
"https://downloads.mysql.com/archives/get/p/23/file/$TARBALL"
144+
ls -lh "/tmp/mysql-tarball/$TARBALL"
145+
146+
- name: Unpack MySQL
147+
run: |
148+
mkdir -p "$SANDBOX_BINARY"
149+
./dbdeployer unpack "/tmp/mysql-tarball/mysql-8.4.4-linux-glibc2.17-x86_64.tar.xz" \
150+
--sandbox-binary="$SANDBOX_BINARY"
151+
152+
- name: Run deploy time budget test
153+
run: ./test/deploy-time-budget.sh
154+
155+
- name: Cleanup
156+
if: always()
157+
run: |
158+
./dbdeployer delete all --skip-confirm 2>/dev/null || true
159+
pkill -9 -u "$USER" mysqld 2>/dev/null || true
160+
103161
# Test Percona Server: single + replication with data verification
104162
percona-test:
105163
name: Percona Server (${{ matrix.percona-version }})

sandbox/templates/replication/init_slaves.gotxt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@ cd "$SBDIR"
1212
# initialization to prevent the race where the first client command on a
1313
# just-started replica fails with "Access denied" or connection errors.
1414
# Observed especially with MySQL 8.4+ and 9.x.
15+
# NOTE: init_slaves exports NOPASSWORD=1 for the root steps; that must NOT
16+
# leak into this probe, or 'use' would drop the sandbox-user password and the
17+
# check would fail with "Access denied" even once the user exists (burning the
18+
# whole budget). Clear NOPASSWORD for the probe so it authenticates normally.
1519
wait_until_replica_ready() {
1620
local use_cmd=${1:-$SBDIR/use}
17-
local max_attempts=${2:-60}
21+
local max_attempts=${2:-20}
1822
local sleep_sec=${3:-1}
1923
for i in $(seq 1 $max_attempts); do
20-
if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
24+
if NOPASSWORD= $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
2125
return 0
2226
fi
2327
sleep $sleep_sec
@@ -39,7 +43,7 @@ echo "initializing {{.SlaveLabel}} {{.Node}}"
3943
echo '{{.ChangeMasterTo}} {{.MasterHostParam}}="{{.MasterIp}}", {{.MasterPortParam}}={{.MasterPort}}, {{.MasterUserParam}}="{{.RplUser}}", {{.MasterPasswordParam}}="{{.RplPassword}}" {{.MasterAutoPosition}} {{.ChangeMasterExtra}}' | $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root
4044
$SBDIR/{{.NodeLabel}}{{.Node}}/use -u root -e '{{.StartReplica}}'
4145
# Ensure the replica is serving queries before we return from initialization.
42-
wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 60 1
46+
wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 20 1
4347
{{end}}
4448
if [ -x ./post_initialization ]
4549
then

sandbox/templates/replication/init_slaves_84.gotxt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@ cd "$SBDIR"
1313
# initialization to prevent the race where the first client command on a
1414
# just-started replica fails with "Access denied" or connection errors.
1515
# Observed especially with MySQL 8.4+ and 9.x.
16+
# NOTE: init_slaves exports NOPASSWORD=1 for the root steps; that must NOT
17+
# leak into this probe, or 'use' would drop the sandbox-user password and the
18+
# check would fail with "Access denied" even once the user exists (burning the
19+
# whole budget). Clear NOPASSWORD for the probe so it authenticates normally.
1620
wait_until_replica_ready() {
1721
local use_cmd=${1:-$SBDIR/use}
18-
local max_attempts=${2:-60}
22+
local max_attempts=${2:-20}
1923
local sleep_sec=${3:-1}
2024
for i in $(seq 1 $max_attempts); do
21-
if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
25+
if NOPASSWORD= $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
2226
return 0
2327
fi
2428
sleep $sleep_sec
@@ -40,7 +44,7 @@ echo "initializing {{.SlaveLabel}} {{.Node}}"
4044
echo '{{.ChangeMasterTo}} {{.MasterHostParam}}="{{.MasterIp}}", {{.MasterPortParam}}={{.MasterPort}}, {{.MasterUserParam}}="{{.RplUser}}", {{.MasterPasswordParam}}="{{.RplPassword}}" {{.MasterAutoPosition}} {{.ChangeMasterExtra}}' | $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root
4145
$SBDIR/{{.NodeLabel}}{{.Node}}/use -u root -e '{{.StartReplica}}'
4246
# Ensure the replica is serving queries before we return from initialization.
43-
wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 60 1
47+
wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 20 1
4448
{{end}}
4549
if [ -x ./post_initialization ]
4650
then

sandbox/templates/single/sb_include.gotxt

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,70 @@ function check_output
7070
# Wait until wsrep_ready is ON for Galera/PXC nodes.
7171
# This prevents grants and user DML from running before the node
7272
# has joined the cluster.
73+
# On non-Galera servers the status variable does not exist and
74+
# this function returns immediately (no delay) once the server accepts
75+
# connections.
76+
# Detection and polling connect as root via socket (no password),
77+
# because the sandbox user (msandbox) may not exist yet on fresh
78+
# nodes (load_grants has not run).
79+
# Connect failures are NOT treated as non-Galera: the pid file can
80+
# appear before the server accepts connections, so we retry until
81+
# root can connect, then check whether wsrep_ready exists.
7382
function wait_until_wsrep_ready
7483
{
75-
local use_cmd=${1:-$SBDIR/use}
76-
local max_attempts=${2:-60}
77-
local sleep_sec=${3:-2}
84+
local max_attempts=${1:-60}
85+
local sleep_sec=${2:-2}
86+
local mysql_cmd=""
87+
local clients
88+
if [ "$FLAVOR" = "mariadb" ]; then
89+
clients="mariadb mysql"
90+
else
91+
clients="mysql mariadb"
92+
fi
93+
local d c
94+
for d in "$CLIENT_BASEDIR/bin" "$BASEDIR/bin"; do
95+
for c in $clients; do
96+
if [ -x "$d/$c" ]; then
97+
mysql_cmd="$d/$c"
98+
break 2
99+
fi
100+
done
101+
done
102+
if [ -z "$mysql_cmd" ]; then
103+
echo "WARNING: no mysql/mariadb client found under $CLIENT_BASEDIR/bin or $BASEDIR/bin" >&2
104+
return 1
105+
fi
106+
# Wait until root can connect via socket. Empty query output before
107+
# connect succeeds must not be treated as "non-Galera".
108+
# Keep this budget separate and modest: pid-file can appear slightly
109+
# before the server accepts connections, but not for minutes.
110+
local connect_attempts=30
111+
local connect_sleep=1
112+
local connected=0
113+
local i
114+
for i in $(seq 1 $connect_attempts); do
115+
if $mysql_cmd --no-defaults -S "$SOCKET_FILE" -u root -BN -e "SELECT 1" >/dev/null 2>&1; then
116+
connected=1
117+
break
118+
fi
119+
sleep $connect_sleep
120+
done
121+
if [ "$connected" -ne 1 ]; then
122+
echo "WARNING: cannot connect as root via socket after $((connect_attempts * connect_sleep))s" >&2
123+
return 1
124+
fi
125+
# Connected: a SHOW STATUS that succeeds with no rows means wsrep_ready
126+
# does not exist (non-Galera) -> short-circuit. Only the successful-empty
127+
# case may bypass the wait: if the probe itself fails, fall through to the
128+
# polling loop so a transient error cannot skip the wsrep_ready gate.
129+
local wsrep_check
130+
if wsrep_check=$($mysql_cmd --no-defaults -S "$SOCKET_FILE" -u root -BN -e "SHOW STATUS LIKE 'wsrep_ready';" 2>/dev/null); then
131+
if [ -z "$wsrep_check" ]; then
132+
return 0
133+
fi
134+
fi
78135
for i in $(seq 1 $max_attempts); do
79-
if $use_cmd -BN -e "SHOW STATUS LIKE 'wsrep_ready';" 2>/dev/null | grep -qi 'ON' ; then
136+
if $mysql_cmd --no-defaults -S "$SOCKET_FILE" -u root -BN -e "SHOW STATUS LIKE 'wsrep_ready';" 2>/dev/null | grep -qiE 'wsrep_ready[[:space:]]+ON'; then
80137
return 0
81138
fi
82139
sleep $sleep_sec
@@ -90,14 +147,18 @@ function wait_until_wsrep_ready
90147
# initialization to prevent the race where the first client command on a
91148
# just-started replica fails with "Access denied" or connection errors.
92149
# Observed especially with MySQL 8.4+ and 9.x.
93-
# Usage: wait_until_replica_ready "$SBDIR/s1/use" 60 1
150+
# Usage: wait_until_replica_ready "$SBDIR/s1/use" 20 1
151+
# NOTE: callers (e.g. init_slaves) may export NOPASSWORD=1 for root steps; it
152+
# must not leak into this probe, or 'use' would drop the sandbox-user password
153+
# and the check would fail with "Access denied" even once the user exists.
154+
# Clear NOPASSWORD for the probe so it authenticates normally.
94155
function wait_until_replica_ready
95156
{
96157
local use_cmd=${1:-$SBDIR/use}
97-
local max_attempts=${2:-60}
158+
local max_attempts=${2:-20}
98159
local sleep_sec=${3:-1}
99160
for i in $(seq 1 $max_attempts); do
100-
if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
161+
if NOPASSWORD= $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
101162
return 0
102163
fi
103164
sleep $sleep_sec

sandbox/templates/single/wait_wsrep_after_start.gotxt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ source {{.SandboxDir}}/sb_include
55

66
# Best-effort wait for wsrep cluster readiness before grants/DML.
77
# This script is safe to run even on non-Galera sandboxes (exits 0 quickly).
8-
wait_until_wsrep_ready "$SBDIR/use" 60 2 || true
8+
wait_until_wsrep_ready 60 2 || true
99
exit 0

0 commit comments

Comments
 (0)