Merge pull request #132 from ProxySQL/fix/131-replica-ready-after-deploy

fix(replication): wait for replicas to be ready to serve queries after deploy (fixes #131)

Author: renecannao

sandbox/sandbox_test.go

@@ -626,7 +626,39 @@ func testCreateReplicationSandbox(t *testing.T) {
 		t.Fatalf(globals.ErrCreatingSandbox, err)
 	}
 
+	// #131 fix test: replicas must answer queries immediately (exercises wait in initialize_slaves*)
 	sandboxDir := path.Join(sandboxDef.SandboxDir, defaults.Defaults().MasterSlavePrefix+pathVersion)
+	for _, n := range []string{"1", "2"} {
+		use := path.Join(sandboxDir, defaults.Defaults().NodePrefix+n, "use")
+		if !common.ExecExists(use) {
+			t.Fatalf("expected replica use script at %s", use)
+		}
+		// RunCmdWithArgs (RunCmd takes only the cmd string)
+		out, err := common.RunCmdWithArgs(use, []string{"-BN", "-e", "SELECT 1;"}); // tests #131 fix: replica must be ready immediately (no sleep) after deploy
+		if err != nil {
+			t.Fatalf("replica n%s not ready immediately after deploy replication: %v\noutput: %s", n, err, out)
+		}
+		if strings.TrimSpace(out) != "1" {
+			t.Fatalf("replica n%s: expected '1', got %q", n, out)
+		}
+	}
+	t.Logf("ok - both replicas accepted queries immediately after deploy (no manual sleep)")
+
+	// Directly assert the generated script contains the waits (the actual fix).
+	initSlavesScript := path.Join(sandboxDir, "initialize_slaves")
+	initContent, rerr := os.ReadFile(initSlavesScript)
+	if rerr != nil {
+		t.Fatalf("could not read initialize_slaves: %v", rerr)
+	}
+	initStr := string(initContent)
+	if !strings.Contains(initStr, `wait_until_replica_ready "$SBDIR/n1/use" 60 1`) {
+		t.Errorf("initialize_slaves missing wait_until_replica_ready for n1 (the #131 fix)")
+	}
+	if !strings.Contains(initStr, `wait_until_replica_ready "$SBDIR/n2/use" 60 1`) {
+		t.Errorf("initialize_slaves missing wait_until_replica_ready for n2 (the #131 fix)")
+	}
+
+	sandboxDir = path.Join(sandboxDef.SandboxDir, defaults.Defaults().MasterSlavePrefix+pathVersion)
 	okDirExists(t, sandboxDir)
 	dirs := []string{
 		defaults.Defaults().MasterName,

sandbox/templates/replication/init_slaves.gotxt

@@ -6,6 +6,26 @@
 # This script is called by 'start_all' when needed
 SBDIR={{.SandboxDir}}
 cd "$SBDIR"
+
+# Wait until a replica is ready to accept queries from the sandbox user.
+# This is called after START REPLICA (or START SLAVE) during replication
+# initialization to prevent the race where the first client command on a
+# just-started replica fails with "Access denied" or connection errors.
+# Observed especially with MySQL 8.4+ and 9.x.
+wait_until_replica_ready() {
+    local use_cmd=${1:-$SBDIR/use}
+    local max_attempts=${2:-60}
+    local sleep_sec=${3:-1}
+    for i in $(seq 1 $max_attempts); do
+        if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
+            return 0
+        fi
+        sleep $sleep_sec
+    done
+    echo "WARNING: replica not ready to accept queries after $((max_attempts * sleep_sec))s (cmd: $use_cmd)" >&2
+    return 1
+}
+
 # workaround for Bug#89959
 $SBDIR/{{.MasterLabel}}/use -h {{.MasterIp}} -u {{.RplUser}} -p{{.RplPassword}} -e 'set @a=1'
 if [ ! -f needs_initialization ]
@@ -18,6 +38,8 @@ fi
 echo "initializing {{.SlaveLabel}} {{.Node}}"
 echo '{{.ChangeMasterTo}}  {{.MasterHostParam}}="{{.MasterIp}}",  {{.MasterPortParam}}={{.MasterPort}},  {{.MasterUserParam}}="{{.RplUser}}",  {{.MasterPasswordParam}}="{{.RplPassword}}" {{.MasterAutoPosition}} {{.ChangeMasterExtra}}' | $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root
 $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root -e '{{.StartReplica}}'
+# Ensure the replica is serving queries before we return from initialization.
+wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 60 1
 {{end}}
 if [ -x ./post_initialization ]
 then

sandbox/templates/replication/init_slaves_84.gotxt

@@ -7,6 +7,26 @@
 # Uses MySQL 8.4+ replication syntax (CHANGE REPLICATION SOURCE TO)
 SBDIR={{.SandboxDir}}
 cd "$SBDIR"
+
+# Wait until a replica is ready to accept queries from the sandbox user.
+# This is called after START REPLICA (or START SLAVE) during replication
+# initialization to prevent the race where the first client command on a
+# just-started replica fails with "Access denied" or connection errors.
+# Observed especially with MySQL 8.4+ and 9.x.
+wait_until_replica_ready() {
+    local use_cmd=${1:-$SBDIR/use}
+    local max_attempts=${2:-60}
+    local sleep_sec=${3:-1}
+    for i in $(seq 1 $max_attempts); do
+        if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
+            return 0
+        fi
+        sleep $sleep_sec
+    done
+    echo "WARNING: replica not ready to accept queries after $((max_attempts * sleep_sec))s (cmd: $use_cmd)" >&2
+    return 1
+}
+
 # workaround for Bug#89959
 $SBDIR/{{.MasterLabel}}/use -h {{.MasterIp}} -u {{.RplUser}} -p{{.RplPassword}} -e 'set @a=1'
 if [ ! -f needs_initialization ]
@@ -19,6 +39,8 @@ fi
 echo "initializing {{.SlaveLabel}} {{.Node}}"
 echo '{{.ChangeMasterTo}}  {{.MasterHostParam}}="{{.MasterIp}}",  {{.MasterPortParam}}={{.MasterPort}},  {{.MasterUserParam}}="{{.RplUser}}",  {{.MasterPasswordParam}}="{{.RplPassword}}" {{.MasterAutoPosition}} {{.ChangeMasterExtra}}' | $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root
 $SBDIR/{{.NodeLabel}}{{.Node}}/use -u root -e '{{.StartReplica}}'
+# Ensure the replica is serving queries before we return from initialization.
+wait_until_replica_ready "$SBDIR/{{.NodeLabel}}{{.Node}}/use" 60 1
 {{end}}
 if [ -x ./post_initialization ]
 then

sandbox/templates/single/sb_include.gotxt

@@ -84,3 +84,24 @@ function wait_until_wsrep_ready
     echo "WARNING: wsrep_ready not ON after $((max_attempts * sleep_sec))s" >&2
     return 1
 }
+
+# Wait until a replica is ready to accept queries from the sandbox user.
+# This is called after START REPLICA (or START SLAVE) during replication
+# initialization to prevent the race where the first client command on a
+# just-started replica fails with "Access denied" or connection errors.
+# Observed especially with MySQL 8.4+ and 9.x.
+# Usage: wait_until_replica_ready "$SBDIR/s1/use" 60 1
+function wait_until_replica_ready
+{
+    local use_cmd=${1:-$SBDIR/use}
+    local max_attempts=${2:-60}
+    local sleep_sec=${3:-1}
+    for i in $(seq 1 $max_attempts); do
+        if $use_cmd -BN -e "SELECT 1;" >/dev/null 2>&1 ; then
+            return 0
+        fi
+        sleep $sleep_sec
+    done
+    echo "WARNING: replica not ready to accept queries after $((max_attempts * sleep_sec))s (cmd: $use_cmd)" >&2
+    return 1
+}

sandbox/templates_flavor_test.go

@@ -177,3 +177,48 @@ func TestReplicationStopAndUse_DelegateToSingleTemplates(t *testing.T) {
 		}
 	}
 }
+
+// TestInitSlavesTemplates_IncludeReplicaReadyWait ensures that the
+// replication initialization templates (used by "deploy replication")
+// contain the wait_until_replica_ready logic introduced to fix #131.
+// This is a pure template test and does not require any MySQL binaries.
+func TestInitSlavesTemplates_IncludeReplicaReadyWait(t *testing.T) {
+	for name, tmplContent := range map[string]string{
+		"init_slaves":    initSlavesTemplate,
+		"init_slaves_84": initSlaves84Template,
+	} {
+		data := common.StringMap{
+			"ShellPath":          "/bin/bash",
+			"Copyright":          "# test",
+			"AppVersion":         "test",
+			"DateTime":           "now",
+			"TemplateName":       name,
+			"SandboxDir":         "/tmp/rsandbox_1234",
+			"MasterLabel":        "master",
+			"MasterIp":           "127.0.0.1",
+			"RplUser":            "rsandbox",
+			"RplPassword":        "rsandbox",
+			"MasterAutoPosition": "",
+			"ChangeMasterExtra":  "",
+			"StartReplica":       "START REPLICA",
+			"NodeLabel":          "n",
+			"SlaveLabel":         "slave",
+			"Slaves": []common.StringMap{
+				{"NodeLabel": "n", "Node": 1, "SlaveLabel": "slave"},
+				{"NodeLabel": "n", "Node": 2, "SlaveLabel": "slave"},
+			},
+		}
+		result := renderTemplate(t, tmplContent, data)
+
+		if !strings.Contains(result, "wait_until_replica_ready") {
+			t.Errorf("%s template must define the wait_until_replica_ready helper (regression for #131)", name)
+		}
+		// The call must appear for each slave after the START REPLICA line.
+		if !strings.Contains(result, `wait_until_replica_ready "$SBDIR/n1/use" 60 1`) {
+			t.Errorf("%s template must invoke wait for the first replica", name)
+		}
+		if !strings.Contains(result, `wait_until_replica_ready "$SBDIR/n2/use" 60 1`) {
+			t.Errorf("%s template must invoke wait for the second replica", name)
+		}
+	}
+}