Bug #25423650: QUICK UNINSTALL PLUGIN WHILE START GR CAN RESULT INTO DEADLOCK

Jaideep Karande · Jaideep Karande · commit 92bf147ab5ad · 2017-06-26T12:56:12.000+02:00
Problem: GR Uninstall &amp; GR Start lock each other

Description:
Try executing UNINSTALL PLUGIN command while START G.R. is in progress.
1. Start command takes LOCK_group_replication_handler
2. Uninstall starts and locks SQL Queries - function "open_ltable()"
3. Uninstall waits for LOCK_group_replication_handler
4. Start command tries to set the server read only mode that waits on 2)

Resolution:
Group_replication_handler class has been removed along with
lock LOCK_group_replication_handler.
With use of my_plugin_lock_by_name, server will have better visibility over
GR operations and take decisions accordingly.
diff --git a/include/mysql/group_replication_priv.h b/include/mysql/group_replication_priv.h
@@ -31,12 +31,9 @@
 
 
 /**
-  Server side initializations and cleanup.
+  Server side initializations.
 */
-int group_replication_init(const char* plugin_name);
-int group_replication_cleanup();
-int group_replication_start();
-int group_replication_stop();
+int group_replication_init();
 
 
 /**
diff --git a/rapid/plugin/group_replication/src/observer_server_state.cc b/rapid/plugin/group_replication/src/observer_server_state.cc
@@ -58,8 +58,7 @@ int group_replication_before_server_shutdown(Server_state_param*)
 int group_replication_after_server_shutdown(Server_state_param*)
 {
   server_shutdown_status= true;
-  if (plugin_is_group_replication_running())
-    group_replication_stop();
+  plugin_group_replication_stop();
 
   return 0;
 }
diff --git a/rapid/plugin/group_replication/src/plugin.cc b/rapid/plugin/group_replication/src/plugin.cc
@@ -349,6 +349,12 @@ int plugin_group_replication_start()
 
   Mutex_autolock auto_lock_mutex(&plugin_running_mutex);
 
+  DBUG_EXECUTE_IF("group_replication_wait_on_start",
+                 {
+                   const char act[]= "now signal signal.start_waiting wait_for signal.start_continue";
+                   DBUG_ASSERT(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
+                 });
+
   int error= 0;
   bool enabled_super_read_only= false;
   bool read_only_mode= false, super_read_only_mode=false;
@@ -909,7 +915,7 @@ int plugin_group_replication_init(MYSQL_PLUGIN plugin_info)
 
   plugin_info_ptr= plugin_info;
 
-  if (group_replication_init(group_replication_plugin_name))
+  if (group_replication_init())
   {
     /* purecov: begin inspected */
     log_message(MY_ERROR_LEVEL,
@@ -976,7 +982,7 @@ int plugin_group_replication_init(MYSQL_PLUGIN plugin_info)
   }
 
   plugin_is_auto_starting= start_group_replication_at_boot_var;
-  if (start_group_replication_at_boot_var && group_replication_start())
+  if (start_group_replication_at_boot_var && plugin_group_replication_start())
   {
     log_message(MY_ERROR_LEVEL,
                 "Unable to start Group Replication on boot");
@@ -994,10 +1000,9 @@ int plugin_group_replication_deinit(void *p)
   plugin_is_being_uninstalled= true;
   int observer_unregister_error= 0;
 
-  //plugin_group_replication_stop will be called from this method stack
-  if (group_replication_cleanup())
+  if (plugin_group_replication_stop())
     log_message(MY_ERROR_LEVEL,
-                "Failure when cleaning Group Replication server state");
+                "Failure when stopping Group Replication on plugin uninstall");
 
   if (group_member_mgr != NULL)
   {
@@ -1513,7 +1518,7 @@ static int check_if_server_properly_configured()
   //Struct that holds startup and runtime requirements
   Trans_context_info startup_pre_reqs;
 
-  get_server_startup_prerequirements(startup_pre_reqs, true);
+  get_server_startup_prerequirements(startup_pre_reqs, !plugin_is_auto_starting);
 
   if(!startup_pre_reqs.binlog_enabled)
   {
diff --git a/rapid/plugin/group_replication/tests/mtr/r/gr_concurrent_start_uninstall.result b/rapid/plugin/group_replication/tests/mtr/r/gr_concurrent_start_uninstall.result
@@ -0,0 +1,41 @@
+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. Setup GR environment and execute START GR.
+
+[connection server1]
+SET GLOBAL group_replication_bootstrap_group=ON;
+SET GLOBAL group_replication_group_name = '8a94f357-bbbb-22aa-33bb-cccaaa420000';
+
+# 2. Block start, so we can execute UNINSTALL.
+
+SET @debug_save= @@GLOBAL.DEBUG;
+SET @@GLOBAL.DEBUG= 'd,group_replication_wait_on_start';
+START GROUP_REPLICATION;
+
+# 3. Execute UNINSTALL PLUGIN GR from second connection.
+
+[connection server_1]
+SET DEBUG_SYNC= "now WAIT_FOR signal.start_waiting";
+UNINSTALL PLUGIN group_replication;
+Warnings:
+Warning	1620	Plugin is busy and will be uninstalled on shutdown
+Wait for group replication plugin to be deleted.
+
+# 4. SIGNAL START GR to resume processing.
+
+SET DEBUG_SYNC= 'now SIGNAL signal.start_continue';
+[connection server1]
+
+# 5. Confirm plugin is UNINSTALLED.
+
+Wait for group replication plugin to be removed.
+
+# 6. Cleanup
+
+INSTALL PLUGIN group_replication SONAME 'GROUP_REPLICATION';
+SET @@GLOBAL.DEBUG= @debug_save;
+include/group_replication_end.inc
diff --git a/rapid/plugin/group_replication/tests/mtr/r/gr_concurrent_uninstall_stop.result b/rapid/plugin/group_replication/tests/mtr/r/gr_concurrent_uninstall_stop.result
@@ -16,6 +16,5 @@ UNINSTALL PLUGIN group_replication;
 STOP GROUP_REPLICATION;
 [connection server1]
 SET @@GLOBAL.read_only= 0;
-UNINSTALL PLUGIN group_replication;
 INSTALL PLUGIN group_replication SONAME 'GROUP_REPLICATION';
 include/group_replication_end.inc
diff --git a/rapid/plugin/group_replication/tests/mtr/t/gr_concurrent_start_uninstall.test b/rapid/plugin/group_replication/tests/mtr/t/gr_concurrent_start_uninstall.test
@@ -0,0 +1,80 @@
+################################################################################
+# This test resolves Deadlock. GR deadlocks when START GR and
+# UNINSTALL happens at same time.
+#
+# Test:
+# 0. The test requires one server.
+# 1. Setup GR environment and execute START GR.
+# 2. Block start, so we can execute UNINSTALL.
+# 3. Execute UNINSTALL PLUGIN GR.
+# 4. SIGNAL START GR to resume processing.
+# 5. Confirm plugin is UNINSTALLED
+# 6. Cleanup
+#
+################################################################################
+--source ../inc/have_group_replication_plugin.inc
+--let $rpl_skip_group_replication_start= 1
+--source ../inc/group_replication.inc
+
+--echo
+--echo # 1. Setup GR environment and execute START GR.
+--echo
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+SET GLOBAL group_replication_bootstrap_group=ON;
+SET GLOBAL group_replication_group_name = '8a94f357-bbbb-22aa-33bb-cccaaa420000';
+
+--echo
+--echo # 2. Block start, so we can execute UNINSTALL.
+--echo
+
+## If START GR gets lock and UNINSTALL blocks SQL Query execution,
+## GR used to deadlock.
+
+SET @debug_save= @@GLOBAL.DEBUG;
+SET @@GLOBAL.DEBUG= 'd,group_replication_wait_on_start';
+
+--send START GROUP_REPLICATION
+
+--echo
+--echo # 3. Execute UNINSTALL PLUGIN GR from second connection.
+--echo
+--let $rpl_connection_name= server_1
+--source include/rpl_connection.inc
+# Wait for the debug sync to be reached.
+SET DEBUG_SYNC= "now WAIT_FOR signal.start_waiting";
+
+UNINSTALL PLUGIN group_replication;
+
+--echo Wait for group replication plugin to be deleted.
+--let $wait_condition= SELECT COUNT(*)=1 from information_schema.plugins where PLUGIN_NAME like 'group_replication' and PLUGIN_STATUS like 'DELETED';
+--source include/wait_condition.inc
+
+--echo
+--echo # 4. SIGNAL START GR to resume processing.
+--echo
+SET DEBUG_SYNC= 'now SIGNAL signal.start_continue';
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+--reap
+
+--echo
+--echo # 5. Confirm plugin is UNINSTALLED.
+--echo
+
+--echo Wait for group replication plugin to be removed.
+--let $wait_condition= SELECT COUNT(*)=0 from information_schema.plugins where PLUGIN_NAME like 'group_replication';
+--source include/wait_condition.inc
+
+--echo
+--echo # 6. Cleanup
+--echo
+--replace_result $GROUP_REPLICATION GROUP_REPLICATION
+--eval INSTALL PLUGIN group_replication SONAME '$GROUP_REPLICATION'
+--enable_warnings
+
+SET @@GLOBAL.DEBUG= @debug_save;
+--source ../inc/group_replication_end.inc
diff --git a/rapid/plugin/group_replication/tests/mtr/t/gr_concurrent_uninstall_stop.test b/rapid/plugin/group_replication/tests/mtr/t/gr_concurrent_uninstall_stop.test
@@ -30,26 +30,29 @@ SET SESSION sql_log_bin= 1;
 
 --source ../inc/start_and_bootstrap_group_replication.inc
 
+## Warnings depends upon sequence of execution of
+## query "STOP GROUP_REPLICATION" and "UNINSTALL PLUGIN group_replication"
+--disable_warnings
+
 --send UNINSTALL PLUGIN group_replication
 
 --let $rpl_connection_name= server_1
 --source include/rpl_connection.inc
 --echo #
 --echo # STOP GROUP_REPLICATION in parallel.
 --echo #
+## Uninstall will not fail.
+## However, STOP GR may fail if UNINSTALL is executed first.
 --error 0, ER_GROUP_REPLICATION_CONFIGURATION
 STOP GROUP_REPLICATION;
 
 --let $rpl_connection_name= server1
 --source include/rpl_connection.inc
---error 0, ER_OPTION_PREVENTS_STATEMENT
 --reap
 
+--enable_warnings
+
 SET @@GLOBAL.read_only= 0;
-# The previous UNINSTALL attempt may have failed due to SUPER_READ_ONLY=1, as
-# such we uninstall the plugin again.
---error 0, ER_SP_DOES_NOT_EXIST
-UNINSTALL PLUGIN group_replication;
 
 --replace_result $GROUP_REPLICATION GROUP_REPLICATION
 --eval INSTALL PLUGIN group_replication SONAME '$GROUP_REPLICATION'
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
@@ -1136,7 +1136,6 @@ mysql_cond_t COND_server_started;
 mysql_mutex_t LOCK_reset_gtid_table;
 mysql_mutex_t LOCK_compress_gtid_table;
 mysql_cond_t COND_compress_gtid_table;
-mysql_mutex_t LOCK_group_replication_handler;
 #if !defined(_WIN32)
 mysql_mutex_t LOCK_socket_listener_active;
 mysql_cond_t COND_socket_listener_active;
@@ -3807,8 +3806,6 @@ static int init_thread_environment()
                    &LOCK_compress_gtid_table, MY_MUTEX_INIT_FAST);
   mysql_cond_init(key_COND_compress_gtid_table,
                   &COND_compress_gtid_table);
-  mysql_mutex_init(key_LOCK_group_replication_handler,
-                   &LOCK_group_replication_handler, MY_MUTEX_INIT_FAST);
   Events::init_mutexes();
 #if defined(_WIN32)
   mysql_mutex_init(key_LOCK_handler_count,
@@ -9472,7 +9469,6 @@ PSI_cond_key key_object_loading_cond; // TODO need to initialize
 PSI_mutex_key key_mts_temp_table_LOCK;
 PSI_mutex_key key_mts_gaq_LOCK;
 PSI_mutex_key key_thd_timer_mutex;
-PSI_mutex_key key_LOCK_group_replication_handler;
 PSI_mutex_key key_commit_order_manager_mutex;
 PSI_mutex_key key_mutex_slave_worker_hash;
 PSI_mutex_key
@@ -9561,7 +9557,6 @@ static PSI_mutex_info all_server_mutexes[]=
   { &key_mutex_slave_worker_hash, "Relay_log_info::slave_worker_hash_lock", 0, 0},
   { &key_LOCK_offline_mode, "LOCK_offline_mode", PSI_FLAG_GLOBAL, 0},
   { &key_LOCK_default_password_lifetime, "LOCK_default_password_lifetime", PSI_FLAG_GLOBAL, 0},
-  { &key_LOCK_group_replication_handler, "LOCK_group_replication_handler", PSI_FLAG_GLOBAL, 0},
   { &key_LOCK_mandatory_roles, "LOCK_mandatory_roles", PSI_FLAG_GLOBAL, 0}
 };
 
diff --git a/sql/mysqld.h b/sql/mysqld.h
@@ -375,7 +375,6 @@ extern PSI_mutex_key key_gtid_ensure_index_mutex;
 extern PSI_mutex_key key_mts_temp_table_LOCK;
 extern PSI_mutex_key key_mts_gaq_LOCK;
 extern PSI_mutex_key key_thd_timer_mutex;
-extern PSI_mutex_key key_LOCK_group_replication_handler;
 
 extern PSI_mutex_key key_commit_order_manager_mutex;
 extern PSI_mutex_key key_mutex_slave_worker_hash;
@@ -641,7 +640,6 @@ extern mysql_cond_t COND_manager;
 extern mysql_rwlock_t LOCK_sys_init_connect;
 extern mysql_rwlock_t LOCK_sys_init_slave;
 extern mysql_rwlock_t LOCK_system_variables_hash;
-extern mysql_mutex_t LOCK_group_replication_handler;
 
 extern char *opt_ssl_ca, *opt_ssl_capath, *opt_ssl_cert, *opt_ssl_cipher,
             *opt_ssl_key, *opt_ssl_crl, *opt_ssl_crlpath, *opt_tls_version;
diff --git a/sql/rpl_group_replication.cc b/sql/rpl_group_replication.cc
diff --git a/sql/rpl_group_replication.h b/sql/rpl_group_replication.h

Original file line number	Diff line number	Diff line change
`@@ -58,8 +58,7 @@ int group_replication_before_server_shutdown(Server_state_param*)`
`58`	`58`	`int group_replication_after_server_shutdown(Server_state_param*)`
`59`	`59`	`{`
`60`	`60`	`server_shutdown_status= true;`
`61`		`- if (plugin_is_group_replication_running())`
`62`		`- group_replication_stop();`
	`61`	`+ plugin_group_replication_stop();`
`63`	`62`
`64`	`63`	`return 0;`
`65`	`64`	`}`