Bug #26495619: PERSISTED PLUGIN VARIABLES ARE NOT CONSIDERED DURING

Bharathy Satish · Bharathy Satish · commit d18087817ee0 · 2017-12-01T05:55:28.000+01:00
SERVER RESTART
Problem: System variables can be set either from command line or through
sql using SET statement. Variables set as part of command line does not
go through runtime validation, whereas variables set using SET statement
are validated using check() function. Thus read only variables are set as
part of command line and dynamic variables are set using SET statement.

Variables persisted using SET PERSIST or PERSIST_ONLY syntax will ensure
that variables are validated using check() when server is restarted, except
with an exception for read only variables because they are appended to command
line during server startup.

There are certain group replication(GR) specific variables which are defined
to be DYNAMIC variables, but cannot be changed once GR is up and ONLINE. This
kind of behaviour confuses the existing SET statement execution logic. Thus
when GR variables are persisted, and later when server is started persisted
GR variables could not be set early before GR plugin is loaded. This will not
make GR to come ONLINE on boot process.

ex:
SET PERSIST group_replication_group_name="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
            group_replication_group_seeds="localhost:13011,localhost:13123",
            group_replication_local_address="localhost:13123",
	        group_replication_bootstrap_group= ON,
            group_replication_start_on_boot=ON;

With above variables persisted, GR should start on boot when server is
restarted. This does not happen because SET statement logic expects these
variables to be readonly so that these can be set early before plugin is
loaded. To obtain this semantics variables needs to be decleared as READ
ONLY, and for certain reasons these variables cannot be changed to readonly.

Fix: A new flag named PLUGIN_VAR_PERSIST_AS_READ_ONLY is introduced which
needs to be set in variable declaration to indicate that these are variables
which can be changed before plugin is loaded but not after that. This flag
will be checked in SET statement logic and will treat them as a read only
variables. Thus when server is restarted these variables can be appended to
command line, making GR to start on boot.
diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h
@@ -90,7 +90,7 @@ typedef struct st_mysql_xid MYSQL_XID;
   Plugin API. Common for all plugin types.
 */
 
-#define MYSQL_PLUGIN_INTERFACE_VERSION 0x0108
+#define MYSQL_PLUGIN_INTERFACE_VERSION 0x0109
 
 /*
   The allowable types of plugins
@@ -179,6 +179,12 @@ __MYSQL_DECLARE_PLUGIN(NAME, \
 #define PLUGIN_VAR_MEMALLOC     0x8000 /* String needs memory allocated */
 #define PLUGIN_VAR_NOPERSIST    0x10000 /* SET PERSIST_ONLY is prohibited
                                            for read only variables */
+/**
+  There can be some variables which needs to be set before plugin is loaded but
+  not after plugin is loaded. ex: GR specific variables. Below flag must be set
+  for these kind of variables.
+*/
+#define PLUGIN_VAR_PERSIST_AS_READ_ONLY    0x20000
 
 struct st_mysql_sys_var;
 struct st_mysql_value;
@@ -232,7 +238,8 @@ typedef void (*mysql_var_update_func)(MYSQL_THD thd,
         (PLUGIN_VAR_READONLY | PLUGIN_VAR_NOSYSVAR | \
          PLUGIN_VAR_NOCMDOPT | PLUGIN_VAR_NOCMDARG | \
          PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_MEMALLOC | \
-         PLUGIN_VAR_NODEFAULT | PLUGIN_VAR_NOPERSIST)
+         PLUGIN_VAR_NODEFAULT | PLUGIN_VAR_NOPERSIST | \
+         PLUGIN_VAR_PERSIST_AS_READ_ONLY)
 
 #define MYSQL_PLUGIN_VAR_HEADER \
   int flags;                    \
diff --git a/rapid/plugin/group_replication/src/plugin.cc b/rapid/plugin/group_replication/src/plugin.cc
diff --git a/rapid/plugin/group_replication/tests/mtr/r/gr_start_with_persisted_variables.result b/rapid/plugin/group_replication/tests/mtr/r/gr_start_with_persisted_variables.result
@@ -0,0 +1,58 @@
+include/group_replication.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+Note	####	Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
+[connection server1]
+
+############################################################
+# 1. Persist configuration on server1.
+[connection server1]
+SET PERSIST group_replication_group_name= "GROUP_REPLICATION_GROUP_NAME";
+SET PERSIST group_replication_group_seeds= @@GLOBAL.group_replication_group_seeds;
+SET PERSIST group_replication_local_address= @@GLOBAL.group_replication_local_address;
+SET PERSIST group_replication_start_on_boot= ON;
+SET PERSIST group_replication_bootstrap_group= ON;
+
+############################################################
+# 2. Restart server1, it must bootstrap the group.
+# restart
+include/rpl_reconnect.inc
+include/gr_wait_for_member_state.inc
+SELECT variable_name, variable_source FROM performance_schema.variables_info WHERE variable_source IN ('PERSISTED');
+variable_name	variable_source
+group_replication_bootstrap_group	PERSISTED
+group_replication_group_name	PERSISTED
+group_replication_group_seeds	PERSISTED
+group_replication_local_address	PERSISTED
+group_replication_start_on_boot	PERSISTED
+
+############################################################
+# 3. Persist configuration on server2.
+[connection server2]
+SET PERSIST group_replication_group_name= "GROUP_REPLICATION_GROUP_NAME";
+SET PERSIST group_replication_group_seeds= @@GLOBAL.group_replication_group_seeds;
+SET PERSIST group_replication_local_address= @@GLOBAL.group_replication_local_address;
+SET PERSIST group_replication_start_on_boot= ON;
+
+############################################################
+# 4. Restart server2, it must join the group.
+# restart
+include/rpl_reconnect.inc
+include/gr_wait_for_member_state.inc
+include/rpl_gr_wait_for_number_of_members.inc
+SELECT variable_name, variable_source FROM performance_schema.variables_info WHERE variable_source IN ('PERSISTED');
+variable_name	variable_source
+group_replication_group_name	PERSISTED
+group_replication_group_seeds	PERSISTED
+group_replication_local_address	PERSISTED
+group_replication_start_on_boot	PERSISTED
+
+############################################################
+# 5. Clean up.
+[connection server1]
+RESET PERSIST;
+SET GLOBAL group_replication_start_on_boot= OFF;
+[connection server2]
+RESET PERSIST;
+SET GLOBAL group_replication_start_on_boot= OFF;
+include/group_replication_end.inc
diff --git a/rapid/plugin/group_replication/tests/mtr/t/gr_start_with_persisted_variables.test b/rapid/plugin/group_replication/tests/mtr/t/gr_start_with_persisted_variables.test
@@ -0,0 +1,86 @@
+################################################################################
+# This test is to test if group replication can be started with needed variables
+# persisted into mysqld-auto.cnf file.
+# Test:
+# 1. Persist configuration on server1.
+# 2. Restart server1, it must bootstrap the group.
+# 3. Persist configuration on server2.
+# 4. Restart server2, it must join the group.
+# 5. Clean up.
+################################################################################
+--source include/big_test.inc
+--source ../inc/have_group_replication_plugin.inc
+--let $rpl_skip_group_replication_start= 1
+--source ../inc/group_replication.inc
+
+
+--echo
+--echo ############################################################
+--echo # 1. Persist configuration on server1.
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+--replace_result $group_replication_group_name GROUP_REPLICATION_GROUP_NAME
+--eval SET PERSIST group_replication_group_name= "$group_replication_group_name"
+--eval SET PERSIST group_replication_group_seeds= @@GLOBAL.group_replication_group_seeds
+--eval SET PERSIST group_replication_local_address= @@GLOBAL.group_replication_local_address
+SET PERSIST group_replication_start_on_boot= ON;
+SET PERSIST group_replication_bootstrap_group= ON;
+
+
+--echo
+--echo ############################################################
+--echo # 2. Restart server1, it must bootstrap the group.
+--let $allow_rpl_inited= 1
+--source include/restart_mysqld.inc
+--let $rpl_server_number= 1
+--source include/rpl_reconnect.inc
+
+--let $group_replication_member_state= ONLINE
+--source ../inc/gr_wait_for_member_state.inc
+
+SELECT variable_name, variable_source FROM performance_schema.variables_info WHERE variable_source IN ('PERSISTED');
+
+
+--echo
+--echo ############################################################
+--echo # 3. Persist configuration on server2.
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+--replace_result $group_replication_group_name GROUP_REPLICATION_GROUP_NAME
+--eval SET PERSIST group_replication_group_name= "$group_replication_group_name"
+--eval SET PERSIST group_replication_group_seeds= @@GLOBAL.group_replication_group_seeds
+--eval SET PERSIST group_replication_local_address= @@GLOBAL.group_replication_local_address
+SET PERSIST group_replication_start_on_boot= ON;
+
+
+--echo
+--echo ############################################################
+--echo # 4. Restart server2, it must join the group.
+--let $allow_rpl_inited= 1
+--source include/restart_mysqld.inc
+--let $rpl_server_number= 2
+--source include/rpl_reconnect.inc
+
+--let $group_replication_member_state= ONLINE
+--source ../inc/gr_wait_for_member_state.inc
+
+--let $group_replication_number_of_members= 2
+--source ../inc/gr_wait_for_number_of_members.inc
+
+SELECT variable_name, variable_source FROM performance_schema.variables_info WHERE variable_source IN ('PERSISTED');
+
+
+--echo
+--echo ############################################################
+--echo # 5. Clean up.
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+RESET PERSIST;
+SET GLOBAL group_replication_start_on_boot= OFF;
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+RESET PERSIST;
+SET GLOBAL group_replication_start_on_boot= OFF;
+
+--source ../inc/group_replication_end.inc
diff --git a/sql/persisted_variable.cc b/sql/persisted_variable.cc
@@ -256,7 +256,8 @@ void Persisted_variables_cache::set_variable(THD *thd, set_var *setvar)
   /* modification to in-memory must be thread safe */
   mysql_mutex_lock(&m_LOCK_persist_variables);
   /* if present update variable with new value else insert into hash */
-  if (setvar->type == OPT_PERSIST_ONLY && setvar->var->is_readonly())
+  if ((setvar->type == OPT_PERSIST_ONLY && setvar->var->is_readonly()) ||
+    setvar->var->is_plugin_var_read_only())
     m_persist_ro_variables[tmp_var.key]= tmp_var.value;
   else
   {
diff --git a/sql/set_var.h b/sql/set_var.h
@@ -194,6 +194,11 @@ class sys_var
     timestamp = 0;
   }
   virtual bool is_non_persistent() {return flags & NOTPERSIST; }
+  /**
+    Check if plugin variable is persisted as a read only variable. For
+    server variables always return false.
+  */
+  virtual bool is_plugin_var_read_only() { return 0; }
 
   /**
      Update the system variable with the default value from either
diff --git a/sql/sql_plugin_var.h b/sql/sql_plugin_var.h
@@ -277,6 +277,15 @@ class sys_var_pluginvar: public sys_var
   { return (plugin_var->flags & PLUGIN_VAR_NOPERSIST); }
   void set_is_plugin(bool val)
   { is_plugin= val; }
+  /**
+    Check if plugin variable is persisted as a read only variable.
+
+     @return
+       @retval true
+       @retval false
+  */
+  bool is_plugin_var_read_only()
+  { return (plugin_var->flags & PLUGIN_VAR_PERSIST_AS_READ_ONLY); }
 };
 
 /*
