MDEV-4827 mysqldump --dump-slave=2 --master-data=2 doesn't record both

Recording both is useful on a replication relay when the backup
can be used to replace the server, or ack as a new replica to the
server.

If an option=2, commented is selected, allow the alternate option
to exist.

This still disables --dump-slave=1 --master-data=1 as having the
a CHANGE MASTER TO and START SLAVE on different positions would be
confusing and dangerious to the try to execute the output. The
previous behaviour of silently disabling --master-data occurs in
this case.

The commented code related to --dump-slave/--master-data is greatly
expanded for human consumption.

A redundant opt_slave_data= 0 was removed from get_opts. If
--dump-slave=1 or 2, then the only possible value of --master-data
is a valid one.

Re-order to preference gtid based replication.

Based of code from Elena Stepanova.

Review by: Brandon Nesterenko and Anel Husakovic

client/mysqldump.c

@@ -1226,8 +1226,9 @@ static int get_options(int *argc, char ***argv)
   if (opt_slave_data)
   {
     opt_lock_all_tables= !opt_single_transaction;
-    opt_master_data= 0;
     opt_delete_master_logs= 0;
+    if (opt_slave_data != MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL)
+      opt_master_data= 0;
   }
 
   /* Ensure consistency of the set of binlog & locking options */
@@ -1240,10 +1241,7 @@ static int get_options(int *argc, char ***argv)
     return(EX_USAGE);
   }
   if (opt_master_data)
-  {
     opt_lock_all_tables= !opt_single_transaction;
-    opt_slave_data= 0;
-  }
   if (opt_single_transaction || opt_lock_all_tables)
     lock_tables= 0;
   if (enclosed && opt_enclosed)
@@ -6043,17 +6041,12 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
 
   }
 
-  /* SHOW MASTER STATUS reports file and position */
-  print_comment(md_result_file, 0,
-                "\n--\n-- Position to start replication or point-in-time "
-                "recovery from\n--\n\n");
-  fprintf(md_result_file,
-          "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n",
-          (use_gtid ? "-- " : comment_prefix), file, offset);
+  /* gtid */
   if (have_mariadb_gtid)
   {
     print_comment(md_result_file, 0,
-                  "\n--\n-- GTID to start replication from\n--\n\n");
+                  "\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",
@@ -6062,6 +6055,19 @@ static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos,
             "%sSET GLOBAL gtid_slave_pos='%s';\n",
             (!use_gtid ? "-- " : comment_prefix), gtid_pos);
   }
+
+  /* SHOW MASTER STATUS reports file and position */
+  print_comment(md_result_file, 0,
+                "\n--\n-- Alternately, following is the position of the binary "
+                "logging from SHOW MASTER STATUS at point of backup."
+                "\n-- Use this when creating a replica of the primary server "
+                "where the backup was made."
+                "\n-- The new server will be connecting to the primary server "
+                "where the backup was taken."
+                "\n--\n\n");
+  fprintf(md_result_file,
+          "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n",
+          (use_gtid ? "-- " : comment_prefix), file, offset);
   check_io(md_result_file);
 
   if (!consistent_binlog_pos)
@@ -6140,7 +6146,6 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
     (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 ?
@@ -6156,9 +6161,16 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
     return 1;
   }
 
-  while ((row= mysql_fetch_row(slave)))
-  {
-    if (multi_source && !set_gtid_done)
+  print_comment(md_result_file, 0,
+                "\n--\n-- The following is the SQL position of the replication "
+                "taken from SHOW SLAVE STATUS at the time of backup.\n"
+                "-- Use this position when creating a clone of, or replacement "
+                "server, from where the backup was taken."
+                "\n-- This new server will connects to the same primary "
+                "server%s.\n--\n",
+                multi_source ? "(s)" : "");
+
+  if (multi_source)
   {
     char gtid_pos[MAX_GTID_LENGTH];
     if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 0))
@@ -6166,13 +6178,19 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
       mysql_free_result(slave);
       return 1;
     }
-      if (opt_comments)
-        fprintf(md_result_file, "\n--\n-- Gtid position to start replication "
-                "from\n--\n\n");
+    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);
-      set_gtid_done= 1;
   }
+  if (use_gtid)
+    print_comment(md_result_file, 0,
+                  "\n-- Use only the MASTER_USE_GTID=slave_pos or "
+                  "MASTER_LOG_FILE/MASTER_LOG_POS in the statements below."
+                  "\n\n");
+
+  while ((row= mysql_fetch_row(slave)))
+  {
     if (row[9 + multi_source] && row[21 + multi_source])
     {
       if (use_gtid)
@@ -6186,11 +6204,6 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
       }
 
       /* SHOW MASTER STATUS reports file and position */
-      if (opt_comments)
-        fprintf(md_result_file,
-                "\n--\n-- Position to start replication or point-in-time "
-                "recovery from (the master of this slave)\n--\n\n");
-
       if (multi_source)
         fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO ",
                 nogtid_comment_prefix, row[0]);
@@ -6211,6 +6224,7 @@ static int do_show_slave_status(MYSQL *mysql_con, int use_gtid,
       check_io(md_result_file);
     }
   }
+  fprintf(md_result_file, "\n");
   mysql_free_result(slave);
   return 0;
 }

mysql-test/main/rpl_mysqldump_slave.result

@@ -9,19 +9,23 @@ use test;
 connection slave;
 -- 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
@@ -41,55 +45,59 @@ 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;
 
+
 1a. --dump-slave=1
 
 -- SET GLOBAL gtid_slave_pos='0-1-1001';
 CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
+
 2. --dump-slave=2 --gtid
 
 -- 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_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
+
 *** Test mysqldump --master-data GTID/non-gtid functionality.
 
 1. --master-data=1 --gtid
 
--- 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';
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 1a. --master-data=1
 
-CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 -- SET GLOBAL gtid_slave_pos='0-2-1003';
+CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 2. --master-data=2 --gtid
 
--- 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';
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 2a. --master-data=2
 
--- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 -- SET GLOBAL gtid_slave_pos='0-2-1003';
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 3. --master-data --single-transaction --gtid
 
--- 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';
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 3a. --master-data --single-transaction
 
-CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 -- SET GLOBAL gtid_slave_pos='0-2-1003';
+CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 4. --master-data=2 --dump-slave=2 --single-transaction --gtid (MDEV-4827)
 
@@ -108,18 +116,32 @@ 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:
+
+-- CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+-- SET GLOBAL gtid_slave_pos='0-2-1003';
+
 --
--- Gtid position to start replication from
+-- 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.
+-- The new server will be connecting to the primary server where the backup was taken.
 --
 
--- SET GLOBAL gtid_slave_pos='0-1-1001';
--- CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos;
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 --
--- Position to start replication or point-in-time recovery from (the master of this slave)
+-- 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';
+
+-- 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;
+
 /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;
 
 /*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
@@ -149,17 +171,27 @@ 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';
+
 --
--- Gtid position to start replication from
+-- 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.
+-- The new server will be connecting to the primary server where the backup was taken.
 --
 
--- SET GLOBAL gtid_slave_pos='0-1-1001';
+-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START;
 
 --
--- Position to start replication or point-in-time recovery from (the master of this slave)
+-- 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';
 -- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START;
+
 /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;
 
 /*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
@@ -183,8 +215,8 @@ include/stop_slave.inc
 change master to master_use_gtid=slave_pos;
 connection master;
 # Ensuring the binlog dump thread is killed on primary...
--- CHANGE MASTER TO MASTER_LOG_FILE='master-bin.000002', MASTER_LOG_POS=BINLOG_START;
 -- SET GLOBAL gtid_slave_pos='0-1-1005';
+-- CHANGE MASTER TO MASTER_LOG_FILE='master-bin.000002', MASTER_LOG_POS=BINLOG_START;
 connection slave;
 include/start_slave.inc
 include/rpl_end.inc