MDEV-15393 gtid_slave_pos duplicate key errors after mysqldump restore

When mysqldump is run to dump the `mysql` system database, it generates
INSERT statements into the table `mysql.gtid_slave_pos`.
After running the backup script
those inserts did not produce the expected gtid state on slave. In
particular the maximum of mysql.gtid_slave_pos.sub_id did not make
into
   rpl_global_gtid_slave_state.last_sub_id

an in-memory object that is supposed to match the current state of the
table. And that was regardless of whether --gtid option was specified
or not. Later when the backup recipient server starts as slave
in *non-gtid* mode this desychronization may lead to a duplicate key
error.

This effect is corrected for --gtid mode mysqldump/mariadb-dump only
as the following.  The fixes ensure the insert block of the dump
script is followed with a "summing-up" SET @global.gtid_slave_pos
assignment.

For the implemenation part, note a deferred print-out of
SET-gtid_slave_pos and associated comments is prefered over relocating
of the entire blocks if (opt_master,slave_data &&
do_show_master,slave_status) ...  because of compatiblity
concern. Namely an error inside do_show_*() is handled in the new code
the same way, as early as, as before.

A regression test can be run in how-to-reproduce mode as well.
One affected mtr test observed.
rpl_mysqldump_slave.result "mismatch" shows now the new deferring print
of SET-gtid_slave_pos policy in action.

Author: andrelkin

client/mysqldump.c

@@ -5985,8 +5985,11 @@ static int dump_selected_tables(char *db, char **table_names, int tables)
 } /* dump_selected_tables */
 
 
+const char fmt_gtid_pos[]= "%sSET GLOBAL gtid_slave_pos='%s';\n";
+
 static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
-                                 int have_mariadb_gtid, int use_gtid)
+                                 int have_mariadb_gtid, int use_gtid,
+                                 char *set_gtid_pos)
 {
   MYSQL_ROW row;
   MYSQL_RES *UNINIT_VAR(master);
@@ -6047,15 +6050,21 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
   /* gtid */
   if (have_mariadb_gtid)
   {
-    print_comment(md_result_file, 0,
-                  "\n-- Preferably use GTID to start replication from GTID "
-                  "position:\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",
+      /*
+        When --gtid is specified defer print of SET gtid_slave_pos until
+        after its placeholder table is guaranteed to have been dumped.
+      */
+      print_comment(md_result_file, 0,
+                    "\n-- A corresponding to the above master-data "
+                    "CHANGE-MASTER settings to the slave gtid state is printed "
+                    "later in the file.\n");
+    }
+    sprintf(set_gtid_pos, fmt_gtid_pos,
             (!use_gtid ? "-- " : comment_prefix), gtid_pos);
   }
 
@@ -6071,6 +6080,11 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
   fprintf(md_result_file,
           "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n",
           (use_gtid ? "-- " : comment_prefix), file, offset);
+  if (have_mariadb_gtid && !use_gtid)
+    print_comment(md_result_file, 0,
+                  "\n-- A corresponding to the above master-data CHANGE-MASTER "
+                  "settings to the slave gtid state is printed as comments "
+                  "later in the file.\n");
   check_io(md_result_file);
 
   if (!consistent_binlog_pos)
@@ -6140,8 +6154,8 @@ static int add_slave_statements(void)
   return(0);
 }
 
-static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
-                                int have_mariadb_gtid)
+static int do_show_slave_status(MYSQL *mysql_con, int have_mariadb_gtid,
+                                int use_gtid, char* set_gtid_pos)
 {
   MYSQL_RES *UNINIT_VAR(slave);
   MYSQL_ROW row;
@@ -6181,10 +6195,12 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
       mysql_free_result(slave);
       return 1;
     }
+    /* defer print similarly to do_show_master_status */
     print_comment(md_result_file, 0,
-                  "-- GTID position to start replication:\n");
-    fprintf(md_result_file, "%sSET GLOBAL gtid_slave_pos='%s';\n",
-            gtid_comment_prefix, gtid_pos);
+                  "\n-- A corresponding to the below dump-slave "
+                  "CHANGE-MASTER settings to the slave gtid state is printed "
+                  "later in the file.\n");
+    sprintf(set_gtid_pos, fmt_gtid_pos, gtid_comment_prefix, gtid_pos);
   }
   if (use_gtid)
     print_comment(md_result_file, 0,
@@ -6905,6 +6921,34 @@ static void dynstr_realloc_checked(DYNAMIC_STRING *str, ulong additional_size)
     die(EX_MYSQLERR, DYNAMIC_STR_ERROR_MSG);
 }
 
+/**
+  Print earlier prepared SET @@global.gtid_slave_pos.
+
+  @param set_gtid_pos[in]  formatted sql set statement
+**/
+static void do_print_set_gtid_slave_pos(const char *set_gtid_pos,
+                                        my_bool is_master_data)
+{
+  DBUG_ASSERT(opt_master_data || opt_slave_data);
+  if (is_master_data)
+  {
+    print_comment(md_result_file, 0,
+                  "\n-- The deferred gtid setting for slave corresponding to "
+                  "the master-data CHANGE-MASTER follows\n");
+    print_comment(md_result_file, 0,
+                  "\n-- Preferably use GTID to start replication from GTID "
+                  "position:\n\n");
+  }
+  else
+  {
+    print_comment(md_result_file, 0,
+                  "\n-- The deferred gtid setting for slave corresponding to "
+                  "the dump-slave CHANGE-MASTER follows\n");
+    print_comment(md_result_file, 0,
+                  "-- GTID position to start replication:\n");
+  }
+  fprintf(md_result_file, "%s", set_gtid_pos);
+}
 
 int main(int argc, char **argv)
 {
@@ -6913,6 +6957,12 @@ int main(int argc, char **argv)
   int exit_code;
   int consistent_binlog_pos= 0;
   int have_mariadb_gtid= 0;
+  /*
+    to hold SET @@global.gtid_slave_pos which is deferred to print
+    until the function epilogue.
+  */
+  char master_set_gtid_pos[3 + sizeof(fmt_gtid_pos) + MAX_GTID_LENGTH]= {0};
+  char slave_set_gtid_pos[3 + sizeof(fmt_gtid_pos) + MAX_GTID_LENGTH]= {0};
   MY_INIT(argv[0]);
 
   sf_leaking_memory=1; /* don't report memory leaks on early exits */
@@ -7016,10 +7066,12 @@ int main(int argc, char **argv)
     goto err;
 
   if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos,
-                                               have_mariadb_gtid, opt_use_gtid))
+                                               have_mariadb_gtid,
+                                               opt_use_gtid, master_set_gtid_pos))
     goto err;
-  if (opt_slave_data && do_show_slave_status(mysql, opt_use_gtid,
-                                             have_mariadb_gtid))
+  if (opt_slave_data && do_show_slave_status(mysql,
+                                             have_mariadb_gtid,
+                                             opt_use_gtid, slave_set_gtid_pos))
     goto err;
   if (opt_single_transaction && do_unlock_tables(mysql)) /* unlock but no commit! */
     goto err;
@@ -7087,6 +7139,11 @@ int main(int argc, char **argv)
   if (opt_system & OPT_SYSTEM_TIMEZONES)
     dump_all_timezones();
 
+  if (opt_master_data && master_set_gtid_pos[0])
+    do_print_set_gtid_slave_pos(master_set_gtid_pos, TRUE);
+  if (opt_slave_data && slave_set_gtid_pos[0])
+    do_print_set_gtid_slave_pos(slave_set_gtid_pos, FALSE);
+
   /* add 'START SLAVE' to end of dump */
   if (opt_slave_apply && add_slave_statements())
     goto err;

mysql-test/main/rpl_mysqldump_slave.result

@@ -8,26 +8,25 @@ connection master;
 use test;
 connection slave;
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+-- SET GLOBAL gtid_slave_pos='';
 /*M!999999\- enable the sandbox mode */ 
 STOP ALL SLAVES;
--- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+-- SET GLOBAL gtid_slave_pos='';
 START ALL SLAVES;
 /*M!999999\- enable the sandbox mode */ 
 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;
 
+-- SET GLOBAL gtid_slave_pos='';
 START ALL SLAVES;
 start slave;
 Warnings:
 Note	1254	Slave is already running
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 start slave;
@@ -46,72 +45,72 @@ DROP TABLE t2;
 1. --dump-slave=1 --gtid
 
 /*M!999999\- enable the sandbox mode */ 
-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;
 
+SET GLOBAL gtid_slave_pos='0-1-1001';
 
 1a. --dump-slave=1
 
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-1-1001';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
 
 2. --dump-slave=2 --gtid
 
 /*M!999999\- enable the sandbox mode */ 
--- 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;
 
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
 
 2. --dump-slave=2
 
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-1-1001';
 -- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
 *** Test mysqldump --master-data GTID/non-gtid functionality.
 
 1. --master-data=1 --gtid
 
 /*M!999999\- enable the sandbox mode */ 
 CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
-SET GLOBAL gtid_slave_pos='0-2-1003';
 -- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+SET GLOBAL gtid_slave_pos='0-2-1003';
 
 1a. --master-data=1
 
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-2-1003';
 CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
 
 2. --master-data=2 --gtid
 
 /*M!999999\- enable the sandbox mode */ 
 -- CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
--- SET GLOBAL gtid_slave_pos='0-2-1003';
 -- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
 
 2a. --master-data=2
 
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-2-1003';
 -- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
 
 3. --master-data --single-transaction --gtid
 
 /*M!999999\- enable the sandbox mode */ 
 CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
-SET GLOBAL gtid_slave_pos='0-2-1003';
 -- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+SET GLOBAL gtid_slave_pos='0-2-1003';
 
 3a. --master-data --single-transaction
 
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-2-1003';
 CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
 
 4. --master-data=2 --dump-slave=2 --single-transaction --gtid (MDEV-4827)
 
@@ -130,11 +129,9 @@ CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START
 /*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
 /*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
 /*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;
-
--- Preferably use GTID to start replication from GTID position:
-
 -- CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
--- SET GLOBAL gtid_slave_pos='0-2-1003';
+
+-- A corresponding to the above master-data CHANGE-MASTER settings to the slave gtid state is printed later in the file.
 
 --
 -- Alternately, following is the position of the binary logging from SHOW MASTER STATUS at point of backup.
@@ -149,14 +146,24 @@ CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START
 -- Use this position when creating a clone of, or replacement server, from where the backup was taken.
 -- This new server will connects to the same primary server(s).
 --
--- GTID position to start replication:
--- SET GLOBAL gtid_slave_pos='0-1-1001';
+
+-- A corresponding to the below dump-slave CHANGE-MASTER settings to the slave gtid state is printed later in the file.
 
 -- Use only the MASTER_USE_GTID=slave_pos or MASTER_LOG_FILE/MASTER_LOG_POS in the statements below.
 
 -- CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos;
 -- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+
+-- The deferred gtid setting for slave corresponding to the master-data CHANGE-MASTER follows
+
+-- Preferably use GTID to start replication from GTID position:
+
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
+
+-- The deferred gtid setting for slave corresponding to the dump-slave CHANGE-MASTER follows
+-- GTID position to start replication:
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
 /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;
 
 /*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
@@ -187,10 +194,6 @@ CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START
 /*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
 /*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;
 
--- Preferably use GTID to start replication from GTID position:
-
--- SET GLOBAL gtid_slave_pos='0-2-1003';
-
 --
 -- Alternately, following is the position of the binary logging from SHOW MASTER STATUS at point of backup.
 -- Use this when creating a replica of the primary server where the backup was made.
@@ -199,15 +202,27 @@ CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START
 
 -- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+-- A corresponding to the above master-data CHANGE-MASTER settings to the slave gtid state is printed as comments later in the file.
+
 --
 -- The following is the SQL position of the replication taken from SHOW SLAVE STATUS at the time of backup.
 -- Use this position when creating a clone of, or replacement server, from where the backup was taken.
 -- This new server will connects to the same primary server(s).
 --
--- GTID position to start replication:
--- SET GLOBAL gtid_slave_pos='0-1-1001';
+
+-- A corresponding to the below dump-slave CHANGE-MASTER settings to the slave gtid state is printed later in the file.
 -- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+
+-- The deferred gtid setting for slave corresponding to the master-data CHANGE-MASTER follows
+
+-- Preferably use GTID to start replication from GTID position:
+
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
+
+-- The deferred gtid setting for slave corresponding to the dump-slave CHANGE-MASTER follows
+-- GTID position to start replication:
+-- SET GLOBAL gtid_slave_pos='0-1-1001';
 /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;
 
 /*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
@@ -232,8 +247,8 @@ change master to master_use_gtid=slave_pos;
 connection master;
 # Ensuring the binlog dump thread is killed on primary...
 /*M!999999\- enable the sandbox mode */ 
--- SET GLOBAL gtid_slave_pos='0-1-1005';
 -- CHANGE MASTER TO MASTER_LOG_FILE='master-bin.000002', MASTER_LOG_POS=BINLOG_START;
+-- SET GLOBAL gtid_slave_pos='0-1-1005';
 connection slave;
 include/start_slave.inc
 include/rpl_end.inc