misc. sql/slave.cc & co. refactor

* `get_master_version_and_clock()` de-duplicate label using fall-through
* `io_slave_killed()` & `check_io_slave_killed()`:
  * reüse the result from the level lower
  * add distinguishing docs
* `try_to_reconnect()`: extract `'` from `if`-`else`
* `handle_slave_io()`: Both `while`s have the same condition;
  looks like the outer `while` can simply be an `if`.
* `connect_to_master()`:
  * assume `mysql_errno()` is not 0 on connection error
  * utilize 0’s falsiness in the loop
  * extend docs
* `sql/sql_show.cc`: refactor SHOW ALL REPLICAS filter’s condition
* `sql/mysqld.cc`: init `master-retry-count` with `master_retry_count`

Reviewed-by: Kristian Nielsen <knielsen@knielsen-hq.org>

mysql-test/include/check-testcase.inc

@@ -163,4 +163,3 @@ if (`SELECT COUNT(*)=1 FROM INFORMATION_SCHEMA.PLUGINS WHERE PLUGIN_NAME = 'wsre
 }
 
 --enable_query_log
-

sql/mysqld.cc

@@ -778,9 +778,9 @@ mysql_cond_t COND_server_started;
 int mysqld_server_started=0, mysqld_server_initialized= 0;
 File_parser_dummy_hook file_parser_dummy_hook;
 
-/* replication parameters, if master_host is not NULL, we are a slave */
+/* replication parameters */
 uint report_port= 0;
-ulong master_retry_count=0;
+ulong master_retry_count=100000;
 char *master_info_file;
 char *relay_log_info_file, *report_user, *report_password, *report_host;
 char *opt_relay_logname = 0, *opt_relaylog_index_name=0;
@@ -6817,7 +6817,7 @@ struct my_option my_long_options[]=
   {"master-retry-count", 0,
    "The number of tries the slave will make to connect to the master before giving up",
    &master_retry_count, &master_retry_count, 0, GET_ULONG,
-   REQUIRED_ARG, 100000, 0, 0, 0, 0, 0},
+   REQUIRED_ARG, static_cast<longlong>(master_retry_count), 0, 0, 0, 0, 0},
 #ifdef HAVE_REPLICATION
   {"init-rpl-role", 0, "Set the replication role",
    &rpl_status, &rpl_status, &rpl_role_typelib,

sql/slave.cc

@@ -1308,14 +1308,15 @@ void end_slave()
   DBUG_VOID_RETURN;
 }
 
+/** @return whether the slave's Master_info is marked as killed */
 static bool io_slave_killed(Master_info* mi)
 {
   DBUG_ENTER("io_slave_killed");
-
   DBUG_ASSERT(mi->slave_running); // tracking buffer overrun
-  if (mi->abort_slave || mi->io_thd->killed)
+  bool is_io_slave_killed= mi->abort_slave || mi->io_thd->killed;
+  if (is_io_slave_killed)
     DBUG_PRINT("info", ("killed"));
-  DBUG_RETURN(mi->abort_slave || mi->io_thd->killed);
+  DBUG_RETURN(is_io_slave_killed);
 }
 
 /**
@@ -2600,10 +2601,6 @@ err:
   DBUG_RETURN(0);
 
 network_err:
-  if (master_res)
-    mysql_free_result(master_res);
-  DBUG_RETURN(2);
-
 slave_killed_err:
   if (master_res)
     mysql_free_result(master_res);
@@ -4377,15 +4374,13 @@ on this slave.\
 }
 
 
+/** Return io_slave_killed(); if it's `true`, also log the given `info`. */
 static bool check_io_slave_killed(Master_info *mi, const char *info)
 {
-  if (io_slave_killed(mi))
+  bool is_io_slave_killed= io_slave_killed(mi);
-  {
+  if (is_io_slave_killed && info && global_system_variables.log_warnings)
-    if (info && global_system_variables.log_warnings)
+    sql_print_information("%s", info);
-      sql_print_information("%s", info);
+  return is_io_slave_killed;
-    return TRUE;
-  }
-  return FALSE;
 }
 
 /**
@@ -4430,11 +4425,10 @@ static int try_to_reconnect(THD *thd, MYSQL *mysql, Master_info *mi,
     {
       tmp.append(STRING_WITH_LEN("; GTID position '"));
       mi->gtid_current_pos.append_to_string(&tmp);
-      if (mi->events_queued_since_last_gtid == 0)
+      tmp.append('\'');
-        tmp.append(STRING_WITH_LEN("'"));
+      if (mi->events_queued_since_last_gtid)
-      else
       {
-        tmp.append(STRING_WITH_LEN("', GTID event skip "));
+        tmp.append(STRING_WITH_LEN(", GTID event skip "));
         tmp.append_ulonglong((ulonglong)mi->events_queued_since_last_gtid);
       }
     }
@@ -4678,7 +4672,7 @@ connected:
 
   DBUG_PRINT("info",("Starting reading binary log from master"));
   thd->set_command(COM_SLAVE_IO);
-  while (!io_slave_killed(mi))
+  if (!io_slave_killed(mi))
   {
     const uchar *event_buf;
 
@@ -6931,20 +6925,21 @@ static int safe_connect(THD* thd, MYSQL* mysql, Master_info* mi)
 }
 
 
-/*
+/**
-  SYNPOSIS
+  @brief Re/connect to the master `mi`
-    connect_to_master()
+  @details
-
+    After preparations and config validations,
-  IMPLEMENTATION
+    this repeatedly calls the low-level connection function (and logs statuses)
-    Try to connect until successful or slave killed or we have retried
+    up to mi->retry_count times until success or when io_slave_killed().
-    mi->retry_count times
+  @param reconnect
+    whether this connection is a new first-time or reconnects an existing one
+  @return errno: 1 if error or 0 if successful
 */
-
 static int connect_to_master(THD* thd, MYSQL* mysql, Master_info* mi,
                              bool reconnect, bool suppress_warnings)
 {
   int slave_was_killed;
-  int last_errno= -2;                           // impossible error
+  unsigned int last_errno= 0; // initialize with not-error
   mi->connects_tried= 0; // reset retry counter
   my_bool my_true= 1;
   DBUG_ENTER("connect_to_master");
@@ -7005,13 +7000,13 @@ static int connect_to_master(THD* thd, MYSQL* mysql, Master_info* mi,
                "terminated.");
     DBUG_RETURN(1);
   }
-  while (!(slave_was_killed = io_slave_killed(mi)) &&
+  while (!(slave_was_killed= io_slave_killed(mi)) &&
-         (reconnect ? mysql_reconnect(mysql) != 0 :
+         (reconnect ? mysql_reconnect(mysql) :
-          mysql_real_connect(mysql, mi->host, mi->user, mi->password, 0,
+          !mysql_real_connect(mysql, mi->host, mi->user, mi->password, 0,
-                             mi->port, 0, client_flag) == 0))
+                              mi->port, 0, client_flag)))
   {
-    /* Don't repeat last error */
+    /* Don't repeat last error and don't report killed error */
-    if ((int)mysql_errno(mysql) != last_errno && !io_slave_killed(mi))
+    if (mysql_errno(mysql) != last_errno && !io_slave_killed(mi))
     {
       last_errno=mysql_errno(mysql);
       suppress_warnings= 0;

sql/sql_show.cc

@@ -9226,18 +9226,12 @@ static int make_slave_status_old_format(THD *thd, ST_SCHEMA_TABLE *schema_table)
   for (uint i=0; !field_info->end_marker(); field_info++, i++)
   {
     if (all_slaves ||
-        // not SLAVE_STATUS_COL_CONNECTION_NAME,
+        (i > SLAVE_STATUS_COL_SLAVE_SQL_STATE &&
-        // SLAVE_STATUS_COL_SLAVE_SQL_STATE
+         i < SLAVE_STATUS_COL_RETRIED_TRANSACTIONS))
-        // and less
-        // SLAVE_STATUS_COL_RETRIED_TRANSACTIONS
-        !(i <= SLAVE_STATUS_COL_SLAVE_SQL_STATE ||
-          i >= SLAVE_STATUS_COL_RETRIED_TRANSACTIONS))
     {
       LEX_CSTRING field_name= field_info->name();
       Item_field *field= new (thd->mem_root)
         Item_field(thd, context, field_name);
-      DBUG_ASSERT(all_slaves || (i > SLAVE_STATUS_COL_SLAVE_SQL_STATE &&
-                                 i < SLAVE_STATUS_COL_RETRIED_TRANSACTIONS));
       if (!field || add_item_to_list(thd, field))
         return 1;
     }