MDEV-25472 : Server crashes when wsrep_cluster_address set to unkown address and wsrep_slave_threads to 0bb-10.2-MDEV-25472

There were two problems
* If cluster address was set incorrectly we silently accepted it.
Fixed by giving a error. Note that connection to old cluster
address is disconnected.
* If we are not anymore connected to Galera cluster or wsrep is
disabled we should not allow changing wsrep_slave_threads.

8 files changed, 139 insertions, 15 deletions

diff --git a/mysql-test/suite/galera/r/galera_ssl_upgrade.result b/mysql-test/suite/galera/r/galera_ssl_upgrade.result
index 8aab135c6a2..d6e148f44a2 100644
--- a/mysql-test/suite/galera/r/galera_ssl_upgrade.result
+++ b/mysql-test/suite/galera/r/galera_ssl_upgrade.result
@@ -1,14 +1,13 @@
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown");
+connection node_1;
 SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment';
 VARIABLE_VALUE = 'Synced'
 1
 SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
 VARIABLE_VALUE = 2
 1
-connection node_1;
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown.*");
+call mtr.add_suppression("WSREP: write_handler.*");
 connection node_2;
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown.*");
+call mtr.add_suppression("WSREP: write_handler.*");
 connection node_1;
 connection node_2;
 connection node_1;
diff --git a/mysql-test/suite/galera/r/galera_var_cluster_address.result b/mysql-test/suite/galera/r/galera_var_cluster_address.result
index 378d8ca84f5..725ffe88070 100644
--- a/mysql-test/suite/galera/r/galera_var_cluster_address.result
+++ b/mysql-test/suite/galera/r/galera_var_cluster_address.result
@@ -2,6 +2,7 @@ connection node_1;
 connection node_2;
 connection node_2;
 SET GLOBAL wsrep_cluster_address = 'foo://';
+ERROR 42000: Variable 'wsrep_cluster_address' can't be set to the value of 'foo://'
 SET SESSION wsrep_sync_wait=0;
 SELECT COUNT(*) > 0 FROM INFORMATION_SCHEMA.GLOBAL_STATUS;
 COUNT(*) > 0
diff --git a/mysql-test/suite/galera/r/mdev-25472.result b/mysql-test/suite/galera/r/mdev-25472.result
new file mode 100644
index 00000000000..d9c3def85d3
--- /dev/null
+++ b/mysql-test/suite/galera/r/mdev-25472.result
@@ -0,0 +1,42 @@
+connection node_1;
+connection node_2;
+connection node_2;
+call mtr.add_suppression("WSREP:.*Invalid backend URI: a");
+call mtr.add_suppression("WSREP: gcs connect failed: Invalid argument");
+# This will cause separation from cluster
+SET GLOBAL wsrep_cluster_address='a';
+ERROR 42000: Variable 'wsrep_cluster_address' can't be set to the value of 'a'
+SET GLOBAL wsrep_slave_threads=0;
+ERROR HY000: Node is not connected to Galera cluster. Can't change value of wsrep_slave_threads.
+SET SESSION wsrep_sync_wait=0;
+SHOW STATUS LIKE 'wsrep_ready';
+Variable_name	Value
+wsrep_ready	OFF
+SHOW STATUS LIKE 'wsrep_cluster_status';
+Variable_name	Value
+wsrep_cluster_status	non-Primary
+SHOW STATUS LIKE 'wsrep_local_state';
+Variable_name	Value
+wsrep_local_state	0
+SHOW STATUS LIKE 'wsrep_local_state_comment';
+Variable_name	Value
+wsrep_local_state_comment	Initialized
+# Join back to cluster
+SHOW STATUS LIKE 'wsrep_ready';
+Variable_name	Value
+wsrep_ready	ON
+SHOW STATUS LIKE 'wsrep_cluster_status';
+Variable_name	Value
+wsrep_cluster_status	Primary
+SHOW STATUS LIKE 'wsrep_local_state';
+Variable_name	Value
+wsrep_local_state	4
+SHOW STATUS LIKE 'wsrep_local_state_comment';
+Variable_name	Value
+wsrep_local_state_comment	Synced
+SET SESSION wsrep_on=OFF;
+SET GLOBAL wsrep_slave_threads=2;
+ERROR HY000: Galera is not enabled. Can't change value of wsrep_slave_threads
+SELECT @@wsrep_slave_threads;
+@@wsrep_slave_threads
+1
diff --git a/mysql-test/suite/galera/t/galera_ssl_upgrade.test b/mysql-test/suite/galera/t/galera_ssl_upgrade.test
index 33c5a43df9d..d69bad62db1 100644
--- a/mysql-test/suite/galera/t/galera_ssl_upgrade.test
+++ b/mysql-test/suite/galera/t/galera_ssl_upgrade.test
@@ -8,15 +8,13 @@
 --source include/have_innodb.inc
 --source include/have_ssl_communication.inc
 
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown");
-
+--connection node_1
 SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment';
 SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
+call mtr.add_suppression("WSREP: write_handler.*");
 
---connection node_1
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown.*");
 --connection node_2
-call mtr.add_suppression("WSREP: write_handler(): protocol is shutdown.*");
+call mtr.add_suppression("WSREP: write_handler.*");
 
 # Setup galera ports
 --connection node_1
diff --git a/mysql-test/suite/galera/t/galera_var_cluster_address.test b/mysql-test/suite/galera/t/galera_var_cluster_address.test
index 6d99d35cdac..08d0e778f11 100644
--- a/mysql-test/suite/galera/t/galera_var_cluster_address.test
+++ b/mysql-test/suite/galera/t/galera_var_cluster_address.test
@@ -16,6 +16,9 @@
 
 --connection node_2
 --let $wsrep_cluster_address_node2 = `SELECT @@wsrep_cluster_address`
+
+# foo:// is not correct Galera cluster address
+--error ER_WRONG_VALUE_FOR_VAR
 SET GLOBAL wsrep_cluster_address = 'foo://';
 
 # With wsrep_sync_wait, this returns an error
diff --git a/mysql-test/suite/galera/t/mdev-25472.test b/mysql-test/suite/galera/t/mdev-25472.test
new file mode 100644
index 00000000000..ece649179ed
--- /dev/null
+++ b/mysql-test/suite/galera/t/mdev-25472.test
@@ -0,0 +1,42 @@
+--source include/galera_cluster.inc
+
+# Save original auto_increment_offset values.
+--let $node_1=node_1
+--let $node_2=node_2
+--source include/auto_increment_offset_save.inc
+
+--connection node_2
+call mtr.add_suppression("WSREP:.*Invalid backend URI: a");
+call mtr.add_suppression("WSREP: gcs connect failed: Invalid argument");
+--let $wsrep_cluster_address_orig = `SELECT @@wsrep_cluster_address`
+
+--echo # This will cause separation from cluster
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL wsrep_cluster_address='a';
+--error ER_WRONG_ARGUMENTS
+SET GLOBAL wsrep_slave_threads=0;
+
+SET SESSION wsrep_sync_wait=0;
+SHOW STATUS LIKE 'wsrep_ready';
+SHOW STATUS LIKE 'wsrep_cluster_status';
+SHOW STATUS LIKE 'wsrep_local_state';
+SHOW STATUS LIKE 'wsrep_local_state_comment';
+
+--echo # Join back to cluster
+
+--disable_query_log
+--eval SET GLOBAL wsrep_cluster_address = '$wsrep_cluster_address_orig';
+--enable_query_log
+
+--source include/wait_until_connected_again.inc
+
+SHOW STATUS LIKE 'wsrep_ready';
+SHOW STATUS LIKE 'wsrep_cluster_status';
+SHOW STATUS LIKE 'wsrep_local_state';
+SHOW STATUS LIKE 'wsrep_local_state_comment';
+
+SET SESSION wsrep_on=OFF;
+--error ER_WRONG_ARGUMENTS
+SET GLOBAL wsrep_slave_threads=2;
+SELECT @@wsrep_slave_threads;
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index e4de3d8d0aa..85dba477f35 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -5023,7 +5023,7 @@ static Sys_var_ulong Sys_wsrep_slave_threads(
        GLOBAL_VAR(wsrep_slave_threads), CMD_LINE(REQUIRED_ARG),
        VALID_RANGE(1, 512), DEFAULT(1), BLOCK_SIZE(1),
        &PLock_wsrep_slave_threads, NOT_IN_BINLOG,
-       ON_CHECK(NULL),
+       ON_CHECK(wsrep_slave_threads_check),
        ON_UPDATE(wsrep_slave_threads_update));
 
 static Sys_var_charptr Sys_wsrep_dbug_option(
diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc
index 45f929de9de..8ab0e1e5212 100644
--- a/sql/wsrep_var.cc
+++ b/sql/wsrep_var.cc
@@ -468,12 +468,13 @@ bool wsrep_cluster_address_check (sys_var *self, THD* thd, set_var* var)
 bool wsrep_cluster_address_update (sys_var *self, THD* thd, enum_var_type type)
 {
   bool wsrep_on_saved;
+  bool ret=false;
 
   /* Do not proceed if wsrep provider is not loaded. */
   if (!wsrep)
   {
     WSREP_INFO("wsrep provider is not loaded, can't re(start) replication.");
-    return false;
+    return ret;
   }
 
   wsrep_on_saved= thd->variables.wsrep_on;
@@ -501,19 +502,25 @@ bool wsrep_cluster_address_update (sys_var *self, THD* thd, enum_var_type type)
   {
     wsrep_create_rollbacker();
     WSREP_DEBUG("Cluster address update creating %ld applier threads running %lu",
-	    wsrep_slave_threads, wsrep_running_applier_threads);
+                wsrep_slave_threads, wsrep_running_applier_threads);
     wsrep_create_appliers(wsrep_slave_threads);
   }
+  else
+  {
+    my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), self->name.str,
+             wsrep_cluster_address);
+    ret= true; // error
+  }
 
   thd->variables.wsrep_on= wsrep_on_saved;
 
-  return false;
+  return ret;
 }
 
 void wsrep_cluster_address_init (const char* value)
 {
-  WSREP_DEBUG("wsrep_cluster_address_init: %s -> %s", 
-              (wsrep_cluster_address) ? wsrep_cluster_address : "null", 
+  WSREP_DEBUG("wsrep_cluster_address_init: %s -> %s",
+              (wsrep_cluster_address) ? wsrep_cluster_address : "null",
               (value) ? value : "null");
 
   my_free((void*) wsrep_cluster_address);
@@ -620,6 +627,38 @@ bool wsrep_slave_threads_update (sys_var *self, THD* thd, enum_var_type type)
   return res;
 }
 
+/** We need this function because SET GLOBAL wsrep_slave_threads=0
+actually will call ON_CHECK and ON_UPDATE functions if present
+with value 1 (min). Furthermore, we should avoid starting
+more than default amount of slave threads if we are not connected
+to Galera cluster (e.g. SET GLOBAL wsrep_cluster_address='a';)
+or wsrep is turned OFF (wsrep_on=OFF). */
+bool wsrep_slave_threads_check(sys_var *self, THD* thd, set_var* var)
+{
+  ulong slave_threads= (ulong)var->save_result.ulonglong_value;
+  bool not_wsrep= (!WSREP(thd) && slave_threads > 1);
+
+  if (!wsrep_connected)
+  {
+    my_message(ER_WRONG_ARGUMENTS,
+               "Node is not connected to Galera cluster. "
+               "Can't change value of wsrep_slave_threads.", MYF(0));
+
+    return true;
+  }
+
+  if (not_wsrep)
+  {
+    my_message(ER_WRONG_ARGUMENTS,
+               "Galera is not enabled. "
+               "Can't change value of wsrep_slave_threads", wsrep_slave_threads);
+
+    return true;
+  }
+
+  return false;
+}
+
 bool wsrep_desync_check (sys_var *self, THD* thd, set_var* var)
 {
   if (wsrep == NULL)