MDEV-36663 Semi-sync Replica Can't Kill Dump Thread When Using SSL

When a replica stops an established semi-sync connection, it is
supposed to kill the corresponding binlog dump thread on the primary
server.  However, when connections are configured to use SSL, this new
connection created by the replica to kill the dump thread doesn't have
any logic to configure SSL options, and thereby the connection can't be
made, and the dump thread will never be killed.

This patch adds logic to configure the semi-sync kill connection with
SSL. The exising logic to set up the connection options for the regular
connection was extracted into a function that the semi-sync kill
connection invokes.

Co-author: Brandon Nesterenko <brandon.nesterenko@mariadb.com>

mysql-test/suite/rpl/r/rpl_semi_sync_ssl_stop.result

@@ -0,0 +1,53 @@
+# Skip starting the slave because we manually start with SSL later
+include/master-slave.inc
+[connection master]
+#
+# Setup
+connection master;
+CREATE USER replssl@localhost;
+GRANT REPLICATION SLAVE on *.* to replssl@localhost REQUIRE SSL;
+set @orig_master_enabled= @@GLOBAL.rpl_semi_sync_master_enabled;
+SET @@GLOBAL.rpl_semi_sync_master_enabled= 1;
+connection slave;
+CHANGE MASTER TO
+master_user='replssl',
+master_password='',
+master_ssl=1,
+master_ssl_ca='MYSQL_TEST_DIR/std_data/cacert.pem',
+master_ssl_cert='MYSQL_TEST_DIR/std_data/client-cert.pem',
+master_ssl_key='MYSQL_TEST_DIR/std_data/client-key.pem';
+set @orig_slave_enabled= @@GLOBAL.rpl_semi_sync_slave_enabled;
+SET @@GLOBAL.rpl_semi_sync_slave_enabled= 1;
+include/start_slave.inc
+connection master;
+# Verify Semi-Sync is active
+SHOW STATUS LIKE 'Rpl_semi_sync_master_clients';
+Variable_name	Value
+Rpl_semi_sync_master_clients	1
+# Create some table so slave can be seen as up-to-date and working
+connection master;
+CREATE TABLE t1 (a INT);
+connection slave;
+# Disconnect the slave and wait until the master's dump thread is gone
+connection slave;
+STOP SLAVE;
+connection master;
+# MDEV-36663: Verifying dump thread connection is killed..
+# ..done
+# Cleanup
+connection master;
+SET @@GLOBAL.rpl_semi_sync_master_enabled= @orig_master_enabled;
+DROP USER replssl@localhost;
+DROP TABLE t1;
+connection slave;
+SET @@GLOBAL.rpl_semi_sync_slave_enabled= @orig_slave_enabled;
+CHANGE MASTER TO
+master_user='root',
+master_ssl=0,
+master_ssl_ca='',
+master_ssl_cert='',
+master_ssl_key='';
+connection slave;
+include/start_slave.inc
+include/rpl_end.inc
+# End of rpl_semi_sync_ssl_stop.inc

mysql-test/suite/rpl/t/rpl_semi_sync_ssl_stop.test

@@ -0,0 +1,100 @@
+#
+#   This test verifies that semi-sync setups configured to use SSL can kill
+# the replication connection when the IO thread is stopped (e.g. from
+# STOP SLAVE). The way it should happen, is that the IO thread creates a new
+# connection to the primary which issues KILL on the connection id of the
+# replication connection. MDEV-36663 reported an issue where this new
+# kill-oriented connection could not connect to a primary when it requires
+# connections to use SSL.
+#
+#   This test sets up a semi-sync SSL master-slave topology, and stops the
+# slave IO thread. It then validates that the connection was killed by using
+# the wait_condition.inc utility to wait for the binlog dump thread to die,
+# and also validates that the status variable Rpl_semi_sync_master_clients
+# reports as 0.
+#
+# References:
+#   MDEV-36663: Semi-sync Replica Can't Kill Dump Thread When Using SSL
+#
+--source include/have_binlog_format_mixed.inc # format-agnostic
+--source include/have_ssl_communication.inc
+
+--echo # Skip starting the slave because we manually start with SSL later
+--let $rpl_skip_start_slave= 1
+--source include/master-slave.inc
+
+--echo #
+--echo # Setup
+--connection master
+CREATE USER replssl@localhost;
+GRANT REPLICATION SLAVE on *.* to replssl@localhost REQUIRE SSL;
+
+set @orig_master_enabled= @@GLOBAL.rpl_semi_sync_master_enabled;
+SET @@GLOBAL.rpl_semi_sync_master_enabled= 1;
+
+--connection slave
+--replace_result $MYSQL_TEST_DIR MYSQL_TEST_DIR
+eval CHANGE MASTER TO
+  master_user='replssl',
+  master_password='',
+  master_ssl=1,
+  master_ssl_ca='$MYSQL_TEST_DIR/std_data/cacert.pem',
+  master_ssl_cert='$MYSQL_TEST_DIR/std_data/client-cert.pem',
+  master_ssl_key='$MYSQL_TEST_DIR/std_data/client-key.pem';
+
+set @orig_slave_enabled= @@GLOBAL.rpl_semi_sync_slave_enabled;
+SET @@GLOBAL.rpl_semi_sync_slave_enabled= 1;
+
+--source include/start_slave.inc
+
+--connection master
+--echo # Verify Semi-Sync is active
+--let $status_var= Rpl_semi_sync_master_status
+--let $status_var_value= ON
+--source include/wait_for_status_var.inc
+SHOW STATUS LIKE 'Rpl_semi_sync_master_clients';
+
+--echo # Create some table so slave can be seen as up-to-date and working
+--connection master
+CREATE TABLE t1 (a INT);
+--sync_slave_with_master
+
+--echo # Disconnect the slave and wait until the master's dump thread is gone
+--connection slave
+STOP SLAVE;
+--connection master
+
+--echo # MDEV-36663: Verifying dump thread connection is killed..
+# Prior to MDEV-36663 fixes, this would time out and
+# Rpl_semi_sync_master_clients would remain 1.
+--let $wait_condition= SELECT COUNT(*)=0 FROM information_schema.PROCESSLIST WHERE COMMAND = 'Binlog Dump'
+--source include/wait_condition.inc
+
+--let $n_master_clients= query_get_value(SHOW STATUS LIKE 'Rpl_semi_sync_master_clients', Value, 1)
+if ($n_master_clients)
+{
+  --echo # Rpl_semi_sync_master_clients: $n_master_clients
+  --die Semi-sync dump thread connection not killed
+}
+--echo # ..done
+
+--echo # Cleanup
+--connection master
+SET @@GLOBAL.rpl_semi_sync_master_enabled= @orig_master_enabled;
+DROP USER replssl@localhost;
+DROP TABLE t1;
+
+--connection slave
+SET @@GLOBAL.rpl_semi_sync_slave_enabled= @orig_slave_enabled;
+CHANGE MASTER TO
+  master_user='root',
+  master_ssl=0,
+  master_ssl_ca='',
+  master_ssl_cert='',
+  master_ssl_key='';
+
+--connection slave
+--source include/start_slave.inc
+
+--source include/rpl_end.inc
+--echo # End of rpl_semi_sync_ssl_stop.inc

sql/rpl_mi.cc

@@ -21,6 +21,7 @@
 #include "slave.h"
 #include "strfunc.h"
 #include "sql_repl.h"
+#include <sql_common.h>
 
 #ifdef HAVE_REPLICATION
 
@@ -2068,4 +2069,52 @@ bool Master_info_index::flush_all_relay_logs()
   DBUG_RETURN(result);
 }
 
+void setup_mysql_connection_for_master(MYSQL *mysql, Master_info *mi,
+                                       uint timeout)
+{
+  DBUG_ASSERT(mi);
+  DBUG_ASSERT(mi->mysql);
+  mysql_options(mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *) &timeout);
+  mysql_options(mysql, MYSQL_OPT_READ_TIMEOUT, (char *) &timeout);
+
+#ifdef HAVE_OPENSSL
+  if (mi->ssl)
+  {
+    mysql_ssl_set(mysql,
+                  mi->ssl_key[0]?mi->ssl_key:0,
+                  mi->ssl_cert[0]?mi->ssl_cert:0,
+                  mi->ssl_ca[0]?mi->ssl_ca:0,
+                  mi->ssl_capath[0]?mi->ssl_capath:0,
+                  mi->ssl_cipher[0]?mi->ssl_cipher:0);
+    mysql_options(mysql, MYSQL_OPT_SSL_CRL,
+                  mi->ssl_crl[0] ? mi->ssl_crl : 0);
+    mysql_options(mysql, MYSQL_OPT_SSL_CRLPATH,
+                  mi->ssl_crlpath[0] ? mi->ssl_crlpath : 0);
+    mysql_options(mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
+                  &mi->ssl_verify_server_cert);
+  }
+#endif
+
+  /*
+    If server's default charset is not supported (like utf16, utf32) as client
+    charset, then set client charset to 'latin1' (default client charset).
+  */
+  if (is_supported_parser_charset(default_charset_info))
+    mysql_options(mysql, MYSQL_SET_CHARSET_NAME, default_charset_info->cs_name.str);
+  else
+  {
+    sql_print_information("'%s' can not be used as client character set. "
+                          "'%s' will be used as default client character set "
+                          "while connecting to master.",
+                          default_charset_info->cs_name.str,
+                          default_client_charset_info->cs_name.str);
+    mysql_options(mysql, MYSQL_SET_CHARSET_NAME,
+                  default_client_charset_info->cs_name.str);
+  }
+
+  /* Set MYSQL_PLUGIN_DIR in case master asks for an external authentication plugin */
+  if (opt_plugin_dir_ptr && *opt_plugin_dir_ptr)
+    mysql_options(mysql, MYSQL_PLUGIN_DIR, opt_plugin_dir_ptr);
+}
+
 #endif /* HAVE_REPLICATION */

sql/rpl_mi.h

@@ -487,5 +487,16 @@ void free_key_master_info(Master_info *mi);
 uint any_slave_sql_running(bool already_locked);
 bool give_error_if_slave_running(bool already_lock);
 
+/*
+  Sets up the basic options for a MYSQL connection, mysql, to connect to the
+  primary server described by the Master_info parameter, mi. The timeout must
+  be passed explicitly, as different types of connections created by the slave
+  will use different values.
+
+  Assumes mysql_init() has already been called on the mysql connection object.
+*/
+void setup_mysql_connection_for_master(MYSQL *mysql, Master_info *mi,
+                                       uint timeout);
+
 #endif /* HAVE_REPLICATION */
 #endif /* RPL_MI_H */

sql/semisync_slave.cc

@@ -141,7 +141,7 @@ void Repl_semi_sync_slave::slave_stop(Master_info *mi)
     DBUG_ASSERT(!debug_sync_set_action(mi->io_thd, STRING_WITH_LEN(act)));
   };);
 #endif
-    kill_connection(mi->mysql);
+    kill_connection(mi);
   }
 
   set_slave_enabled(0);
@@ -158,8 +158,9 @@ void Repl_semi_sync_slave::slave_reconnect(Master_info *mi)
 }
 
 
-void Repl_semi_sync_slave::kill_connection(MYSQL *mysql)
+void Repl_semi_sync_slave::kill_connection(Master_info *mi)
 {
+  MYSQL *mysql= mi->mysql;
   if (!mysql)
     return;
 
@@ -168,8 +169,8 @@ void Repl_semi_sync_slave::kill_connection(MYSQL *mysql)
   size_t kill_buffer_length;
 
   kill_mysql = mysql_init(kill_mysql);
-  mysql_options(kill_mysql, MYSQL_OPT_CONNECT_TIMEOUT, &m_kill_conn_timeout);
-  mysql_options(kill_mysql, MYSQL_OPT_READ_TIMEOUT, &m_kill_conn_timeout);
+
+  setup_mysql_connection_for_master(kill_mysql, mi, m_kill_conn_timeout);
   mysql_options(kill_mysql, MYSQL_OPT_WRITE_TIMEOUT, &m_kill_conn_timeout);
 
   bool ret= (!mysql_real_connect(kill_mysql, mysql->host,

sql/semisync_slave.h

@@ -92,7 +92,7 @@ public:
   void slave_stop(Master_info *mi);
   void slave_reconnect(Master_info *mi);
   int request_transmit(Master_info *mi);
-  void kill_connection(MYSQL *mysql);
+  void kill_connection(Master_info *mi);
 
 private:
   /* True when init_object has been called */

sql/slave.cc

@@ -7618,50 +7618,10 @@ static int connect_to_master(THD* thd, MYSQL* mysql, Master_info* mi,
   if (opt_slave_compressed_protocol)
     client_flag|= CLIENT_COMPRESS;                /* We will use compression */
 
-  mysql_options(mysql, MYSQL_OPT_CONNECT_TIMEOUT, (char *) &slave_net_timeout);
-  mysql_options(mysql, MYSQL_OPT_READ_TIMEOUT, (char *) &slave_net_timeout);
+  setup_mysql_connection_for_master(mi->mysql, mi, slave_net_timeout);
   mysql_options(mysql, MYSQL_OPT_USE_THREAD_SPECIFIC_MEMORY,
                 (char*) &my_true);
 
-#ifdef HAVE_OPENSSL
-  if (mi->ssl)
-  {
-    mysql_ssl_set(mysql,
-                  mi->ssl_key[0]?mi->ssl_key:0,
-                  mi->ssl_cert[0]?mi->ssl_cert:0,
-                  mi->ssl_ca[0]?mi->ssl_ca:0,
-                  mi->ssl_capath[0]?mi->ssl_capath:0,
-                  mi->ssl_cipher[0]?mi->ssl_cipher:0);
-    mysql_options(mysql, MYSQL_OPT_SSL_CRL,
-                  mi->ssl_crl[0] ? mi->ssl_crl : 0);
-    mysql_options(mysql, MYSQL_OPT_SSL_CRLPATH,
-                  mi->ssl_crlpath[0] ? mi->ssl_crlpath : 0);
-    mysql_options(mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
-                  &mi->ssl_verify_server_cert);
-  }
-#endif
-
-  /*
-    If server's default charset is not supported (like utf16, utf32) as client
-    charset, then set client charset to 'latin1' (default client charset).
-  */
-  if (is_supported_parser_charset(default_charset_info))
-    mysql_options(mysql, MYSQL_SET_CHARSET_NAME, default_charset_info->cs_name.str);
-  else
-  {
-    sql_print_information("'%s' can not be used as client character set. "
-                          "'%s' will be used as default client character set "
-                          "while connecting to master.",
-                          default_charset_info->cs_name.str,
-                          default_client_charset_info->cs_name.str);
-    mysql_options(mysql, MYSQL_SET_CHARSET_NAME,
-                  default_client_charset_info->cs_name.str);
-  }
-
-  /* Set MYSQL_PLUGIN_DIR in case master asks for an external authentication plugin */
-  if (opt_plugin_dir_ptr && *opt_plugin_dir_ptr)
-    mysql_options(mysql, MYSQL_PLUGIN_DIR, opt_plugin_dir_ptr);
-
   /* we disallow empty users */
   if (mi->user[0] == 0)
   {