MDEV-6336: mysqldump --master-data does not work with GTID setups

MDEV-6344: mysqldump issues FLUSH TABLES, which gets written into binlog and replicated

Add a --gtid option (for compatibility, the original behaviour is preserved
when --gtid is not used).

With --gtid, --master-data and --dump-slave output the GTID position (the
old-style file/offset position is still output, but commented out). Also, a
CHANGE MASTER TO master_use_gtid=slave_pos is output to ensure a provisioned
slave is configured in GTID, as requested.

Without --gtid, the GTID position is still output, if available, but commented
out.

Also fix MDEV-6344, to avoid FLUSH TABLES getting into the binlog. Otherwise a
mysqldump on a slave server will silently inject a GTID which does not exist
on the master, which is highly undesirable.

Also fix an incorrect error handling around obtaining binlog position with
--master-data (was probably unlikely to trigger in most cases).

client/client_priv.h

@@ -92,6 +92,7 @@ enum options_client
   OPT_REPORT_PROGRESS,
   OPT_SKIP_ANNOTATE_ROWS_EVENTS,
   OPT_SSL_CRL, OPT_SSL_CRLPATH,
+  OPT_USE_GTID,
   OPT_MAX_CLIENT_OPTION /* should be always the last */
 };
 

client/mysqldump.c

@@ -87,6 +87,9 @@
 /* Chars needed to store LONGLONG, excluding trailing '\0'. */
 #define LONGLONG_LEN 20
 
+/* Max length GTID position that we will output. */
+#define MAX_GTID_LENGTH 1024
+
 static void add_load_option(DYNAMIC_STRING *str, const char *option,
                              const char *option_value);
 static ulong find_set(TYPELIB *lib, const char *x, uint length,
@@ -134,6 +137,7 @@ static ulong opt_compatible_mode= 0;
 #define MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL 2
 static uint opt_mysql_port= 0, opt_master_data;
 static uint opt_slave_data;
+static uint opt_use_gtid;
 static uint my_end_arg;
 static char * opt_mysql_unix_port=0;
 static int   first_error=0;
@@ -346,6 +350,13 @@ static struct my_option my_long_options[] =
   {"force", 'f', "Continue even if we get an SQL error.",
    &ignore_errors, &ignore_errors, 0, GET_BOOL, NO_ARG,
    0, 0, 0, 0, 0, 0},
+  {"gtid", OPT_USE_GTID, "Used together with --master-data=1 or --dump-slave=1."
+   "When enabled, the output from those options will set the GTID position "
+   "instead of the binlog file and offset; the file/offset will appear only as "
+   "a comment. When disabled, the GTID position will still appear in the "
+   "output, but only commented.",
+   &opt_use_gtid, &opt_use_gtid, 0, GET_BOOL, NO_ARG,
+   0, 0, 0, 0, 0, 0},
   {"help", '?', "Display this help message and exit.", 0, 0, 0, GET_NO_ARG,
    NO_ARG, 0, 0, 0, 0, 0, 0},
   {"hex-blob", OPT_HEXBLOB, "Dump binary strings (BINARY, "
@@ -1177,7 +1188,7 @@ check_consistent_binlog_pos(char *binlog_pos_file, char *binlog_pos_offset)
 
   if (mysql_query_with_error_report(mysql, &res,
                                     "SHOW STATUS LIKE 'binlog_snapshot_%'"))
-    return 1;
+    return 0;
 
   found= 0;
   while ((row= mysql_fetch_row(res)))
@@ -1200,6 +1211,90 @@ check_consistent_binlog_pos(char *binlog_pos_file, char *binlog_pos_offset)
   return (found == 2);
 }
 
+
+/*
+  Get the GTID position corresponding to a given old-style binlog position.
+  This uses BINLOG_GTID_POS(). The advantage is that the GTID position can
+  be obtained completely non-blocking in this way (without the need for
+  FLUSH TABLES WITH READ LOCK), as the old-style position can be obtained
+  with START TRANSACTION WITH CONSISTENT SNAPSHOT.
+
+  Returns 0 if ok, non-zero if error.
+*/
+static int
+get_binlog_gtid_pos(char *binlog_pos_file, char *binlog_pos_offset,
+                    char *out_gtid_pos)
+{
+  DYNAMIC_STRING query;
+  MYSQL_RES *res;
+  MYSQL_ROW row;
+  int err;
+  char file_buf[FN_REFLEN*2+1], offset_buf[LONGLONG_LEN*2+1];
+  size_t len_pos_file= strlen(binlog_pos_file);
+  size_t len_pos_offset= strlen(binlog_pos_offset);
+
+  if (len_pos_file >= FN_REFLEN || len_pos_offset > LONGLONG_LEN)
+    return 0;
+  mysql_real_escape_string(mysql, file_buf, binlog_pos_file, len_pos_file);
+  mysql_real_escape_string(mysql, offset_buf, binlog_pos_offset, len_pos_offset);
+  init_dynamic_string_checked(&query, "SELECT BINLOG_GTID_POS('", 256, 1024);
+  dynstr_append_checked(&query, file_buf);
+  dynstr_append_checked(&query, "', '");
+  dynstr_append_checked(&query, offset_buf);
+  dynstr_append_checked(&query, "')");
+
+  err= mysql_query_with_error_report(mysql, &res, query.str);
+  dynstr_free(&query);
+  if (err)
+    return err;
+
+  err= 1;
+  if ((row= mysql_fetch_row(res)))
+  {
+    strmake(out_gtid_pos, row[0], MAX_GTID_LENGTH-1);
+    err= 0;
+  }
+  mysql_free_result(res);
+
+  return err;
+}
+
+
+/*
+  Get the GTID position on a master or slave.
+  The parameter MASTER is non-zero to get the position on a master
+  (@@gtid_binlog_pos) or zero for a slave (@@gtid_slave_pos).
+
+  This uses the @@gtid_binlog_pos or @@gtid_slave_pos, so requires FLUSH TABLES
+  WITH READ LOCK or similar to be consistent.
+
+  Returns 0 if ok, non-zero for error.
+*/
+static int
+get_gtid_pos(char *out_gtid_pos, int master)
+{
+  MYSQL_RES *res;
+  MYSQL_ROW row;
+  int found;
+
+  if (mysql_query_with_error_report(mysql, &res,
+                                    (master ?
+                                     "SELECT @@GLOBAL.gtid_binlog_pos" :
+                                     "SELECT @@GLOBAL.gtid_slave_pos")))
+    return 1;
+
+  found= 0;
+  if ((row= mysql_fetch_row(res)))
+  {
+    strmake(out_gtid_pos, row[0], MAX_GTID_LENGTH-1);
+    found++;
+  }
+  mysql_free_result(res);
+
+  return (found != 1);
+}
+
+
 static char *my_case_str(const char *str,
                          uint str_len,
                          const char *token,
@@ -4799,12 +4894,14 @@ static int dump_selected_tables(char *db, char **table_names, int tables)
 } /* dump_selected_tables */
 
 
-static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos)
+static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
+                                 int have_mariadb_gtid, int use_gtid)
 {
   MYSQL_ROW row;
   MYSQL_RES *UNINIT_VAR(master);
   char binlog_pos_file[FN_REFLEN];
   char binlog_pos_offset[LONGLONG_LEN+1];
+  char gtid_pos[MAX_GTID_LENGTH];
   char *file, *offset;
   const char *comment_prefix=
     (opt_master_data == MYSQL_OPT_MASTER_DATA_COMMENTED_SQL) ? "-- " : "";
@@ -4815,6 +4912,9 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos)
       return 1;
     file= binlog_pos_file;
     offset= binlog_pos_offset;
+    if (have_mariadb_gtid &&
+        get_binlog_gtid_pos(binlog_pos_file, binlog_pos_offset, gtid_pos))
+      return 1;
   }
   else
   {
@@ -4844,6 +4944,9 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos)
         return 0;
       }
     }
+
+    if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 1))
+      return 1;
   }
 
   /* SHOW MASTER STATUS reports file and position */
@@ -4852,7 +4955,19 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos)
                 "recovery from\n--\n\n");
   fprintf(md_result_file,
           "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n",
-          comment_prefix, file, offset);
+          (use_gtid ? "-- " : comment_prefix), file, offset);
+  if (have_mariadb_gtid)
+  {
+    print_comment(md_result_file, 0,
+                  "\n--\n-- GTID to start replication from\n--\n\n");
+    if (use_gtid)
+      fprintf(md_result_file,
+              "%sCHANGE MASTER TO MASTER_USE_GTID=slave_pos;\n",
+              comment_prefix);
+    fprintf(md_result_file,
+            "%sSET GLOBAL gtid_slave_pos='%s';\n",
+            (!use_gtid ? "-- " : comment_prefix), gtid_pos);
+  }
   check_io(md_result_file);
 
   if (!consistent_binlog_pos)
@@ -4922,12 +5037,16 @@ static int add_slave_statements(void)
   return(0);
 }
 
-static int do_show_slave_status(MYSQL *mysql_con)
+static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
+                                int have_mariadb_gtid)
 {
   MYSQL_RES *UNINIT_VAR(slave);
   MYSQL_ROW row;
   const char *comment_prefix=
     (opt_slave_data == MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL) ? "-- " : "";
+  const char *gtid_comment_prefix= (use_gtid ? comment_prefix : "-- ");
+  const char *nogtid_comment_prefix= (!use_gtid ? comment_prefix : "-- ");
+  int set_gtid_done= 0;
 
   if (mysql_query_with_error_report(mysql_con, &slave,
                                     multi_source ?
@@ -4945,8 +5064,30 @@ static int do_show_slave_status(MYSQL *mysql_con)
 
   while ((row= mysql_fetch_row(slave)))
   {
+    if (multi_source && !set_gtid_done)
+    {
+      char gtid_pos[MAX_GTID_LENGTH];
+      if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 0))
+        return 1;
+      if (opt_comments)
+        fprintf(md_result_file, "\n--\n-- Gtid position to start replication "
+                "from\n--\n\n");
+      fprintf(md_result_file, "%sSET GLOBAL gtid_slave_pos='%s';\n",
+              gtid_comment_prefix, gtid_pos);
+      set_gtid_done= 1;
+    }
     if (row[9 + multi_source] && row[21 + multi_source])
     {
+      if (use_gtid)
+      {
+        if (multi_source)
+          fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO "
+                  "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix, row[0]);
+        else
+          fprintf(md_result_file, "%sCHANGE MASTER TO "
+                  "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix);
+      }
+
       /* SHOW MASTER STATUS reports file and position */
       if (opt_comments)
         fprintf(md_result_file,
@@ -4955,9 +5096,9 @@ static int do_show_slave_status(MYSQL *mysql_con)
 
       if (multi_source)
         fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO ",
-                comment_prefix, row[0]);
+                nogtid_comment_prefix, row[0]);
       else
-        fprintf(md_result_file, "%sCHANGE MASTER TO ", comment_prefix);
+        fprintf(md_result_file, "%sCHANGE MASTER TO ", nogtid_comment_prefix);
       
       if (opt_include_master_host_port)
       {
@@ -5030,12 +5171,13 @@ static int do_flush_tables_read_lock(MYSQL *mysql_con)
     FLUSH TABLES is to lower the probability of a stage where both mysqldump
     and most client connections are stalled. Of course, if a second long
     update starts between the two FLUSHes, we have that bad stall.
+
+    We use the LOCAL option, as we do not want the FLUSH TABLES replicated to
+    other servers.
   */
   return
     ( mysql_query_with_error_report(mysql_con, 0,
-                                    ((opt_master_data != 0) ? 
-                                        "FLUSH /*!40101 LOCAL */ TABLES" : 
-                                        "FLUSH TABLES")) ||
+                                    "FLUSH /*!40101 LOCAL */ TABLES") ||
       mysql_query_with_error_report(mysql_con, 0,
                                     "FLUSH TABLES WITH READ LOCK") );
 }
@@ -5656,6 +5798,7 @@ int main(int argc, char **argv)
   char bin_log_name[FN_REFLEN];
   int exit_code;
   int consistent_binlog_pos= 0;
+  int have_mariadb_gtid= 0;
   MY_INIT(argv[0]);
 
   sf_leaking_memory=1; /* don't report memory leaks on early exits */
@@ -5696,7 +5839,10 @@ int main(int argc, char **argv)
 
   /* Check if the server support multi source */
   if (mysql_get_server_version(mysql) >= 100000)
+  {
     multi_source= 2;
+    have_mariadb_gtid= 1;
+  }
 
   if (opt_slave_data && do_stop_slave_sql(mysql))
     goto err;
@@ -5743,9 +5889,11 @@ int main(int argc, char **argv)
   /* Add 'STOP SLAVE to beginning of dump */
   if (opt_slave_apply && add_stop_slave())
     goto err;
-  if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos))
+  if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos,
+                                               have_mariadb_gtid, opt_use_gtid))
     goto err;
-  if (opt_slave_data && do_show_slave_status(mysql))
+  if (opt_slave_data && do_show_slave_status(mysql, opt_use_gtid,
+                                             have_mariadb_gtid))
     goto err;
   if (opt_single_transaction && do_unlock_tables(mysql)) /* unlock but no commit! */
     goto err;

mysql-test/r/rpl_mysqldump_slave.result

@@ -4,18 +4,59 @@ include/master-slave.inc
 # New --dump-slave, --apply-slave-statements functionality
 #
 use test;
+-- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 STOP ALL SLAVES;
+-- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 START ALL SLAVES;
 STOP ALL SLAVES;
+-- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_MYPORT, MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 START ALL SLAVES;
 start slave;
 Warnings:
 Note	1254	Slave is already running
+-- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 start slave;
 Warnings:
 Note	1254	Slave is already running
+*** Test mysqldump --dump-slave GTID functionality.
+SET gtid_seq_no = 1000;
+CREATE TABLE t1 (a INT PRIMARY KEY);
+DROP TABLE t1;
+CREATE TABLE t2 (a INT PRIMARY KEY);
+DROP TABLE t2;
+
+1. --dump-slave=1
+
+SET GLOBAL gtid_slave_pos='0-1-1001';
+CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos;
+-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
+
+2. --dump-slave=2
+
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
+-- CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos;
+-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
+*** Test mysqldump --master-data GTID functionality.
+
+1. --master-data=1
+
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+SET GLOBAL gtid_slave_pos='0-2-1003';
+
+2. --master-data=2
+
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+-- CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
+
+3. --master-data --single-transaction
+
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+SET GLOBAL gtid_slave_pos='0-2-1003';
 include/rpl_end.inc

mysql-test/t/rpl_mysqldump_slave.test

@@ -36,4 +36,53 @@ start slave;
 --exec $MYSQL_DUMP_SLAVE --compact --dump-slave no_such_db
 start slave;
 
+
+--echo *** Test mysqldump --dump-slave GTID functionality.
+
+--connection master
+SET gtid_seq_no = 1000;
+CREATE TABLE t1 (a INT PRIMARY KEY);
+DROP TABLE t1;
+--sync_slave_with_master
+
+--connection slave
+# Inject a local transaction on the slave to check that this is not considered
+# for --dump-slave.
+CREATE TABLE t2 (a INT PRIMARY KEY);
+DROP TABLE t2;
+
+--echo
+--echo 1. --dump-slave=1
+--echo
+--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/
+--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=1 --gtid test
+
+--echo
+--echo 2. --dump-slave=2
+--echo
+--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/
+--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=2 --gtid test
+
+
+--echo *** Test mysqldump --master-data GTID functionality.
+--echo
+--echo 1. --master-data=1
+--echo
+--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/
+--exec $MYSQL_DUMP_SLAVE --compact --master-data=1 --gtid test
+
+--echo
+--echo 2. --master-data=2
+--echo
+--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/
+--exec $MYSQL_DUMP_SLAVE --compact --master-data=2 --gtid test
+
+--echo
+--echo 3. --master-data --single-transaction
+--echo
+--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/
+--exec $MYSQL_DUMP_SLAVE --compact --master-data --single-transaction --gtid test
+
+
+
 --source include/rpl_end.inc