MDEV-4483: CHANGE MASTER TO master_use_gtid=xxx looses old-style coordinates.

There was some old code that cleared the position in CHANGE MASTER,
it was forgotten to be removed.

In addition, add code that saves/restores the old-style position
when we nuke the old relay logs as part of GTID slave start.
Normally we will not use these, but it could be useful in case
the GTID connect fails and user wants to go back to the old-style
coordinates.

mysql-test/include/wait_for_slave_param.inc

@@ -12,7 +12,7 @@
 # [--let $slave_timeout= NUMBER]
 # [--let $slave_error_param= [Slave_SQL_Errno | Slave_IO_Errno]]
 # [--let $rpl_debug= 1]
-# --source include/slave_wait_param.inc
+# --source include/wait_for_slave_param.inc
 #
 # Parameters:
 #

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

@@ -22,6 +22,7 @@ a
 include/kill_binlog_dump_threads.inc
 INSERT INTO t1 VALUES (10);
 SET @old_dbug= @@GLOBAL.debug_dbug;
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 include/start_slave.inc
 SELECT * FROM t1 ORDER BY a;
@@ -64,6 +65,7 @@ a
 13
 include/kill_binlog_dump_threads.inc
 INSERT INTO t1 VALUES (20);
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 include/start_slave.inc
 SELECT * FROM t1 ORDER BY a;
@@ -99,6 +101,7 @@ SET gtid_domain_id= 10;
 SET gtid_seq_no= 200;
 INSERT INTO t1 VALUES (13);
 SET gtid_domain_id= 10;
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 START SLAVE UNTIL master_gtid_pos="9-1-50,10-1-200";
 include/wait_for_slave_to_stop.inc
@@ -129,6 +132,7 @@ SET GLOBAL debug_dbug= @old_debug;
 TRUNCATE t1;
 RESET MASTER;
 include/kill_binlog_dump_threads.inc
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,binlog_force_reconnect_after_22_events";
 CREATE TABLE t2 (a INT PRIMARY KEY) ENGINE=InnoDB;
 INSERT INTO t2 VALUES (1);

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

@@ -188,5 +188,41 @@ include/start_slave.inc
 SELECT '0-1-2' AS Gtid_Slave_Pos;
 Gtid_Slave_Pos
 0-1-2
+*** MDEV-4483: Slave loses traditional master coordinates immediately on CHANGE MASTER TO MASTER_USE_GTID = 1 ***
+include/stop_slave.inc
+DROP TABLE t1;
+RESET SLAVE ALL;
+RESET MASTER;
+SET GLOBAL gtid_slave_pos= "";
+CHANGE MASTER TO master_host='127.0.0.1', master_port=MASTER_PORT, master_user='root', master_use_gtid=no, master_log_file="", master_log_pos= 4;
+DROP TABLE t1;
+RESET MASTER;
+CREATE TABLE t1 (a INT PRIMARY KEY);
+include/start_slave.inc
+include/stop_slave.inc
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+START SLAVE UNTIL master_log_file='LOG_FILE1', master_log_pos=LOG_POS1;
+include/wait_for_slave_sql_to_stop.inc
+SELECT * FROM t1;
+a
+1
+include/wait_for_slave_param.inc [Read_Master_Log_Pos]
+include/stop_slave_io.inc
+CHANGE MASTER TO master_use_gtid=slave_pos;
+SET GLOBAL gtid_slave_pos="0-42-42";
+SET sql_log_bin=0;
+call mtr.add_suppression("Error: connecting slave requested to start from GTID");
+SET sql_log_bin=1;
+START SLAVE;
+include/wait_for_slave_io_error.inc [errno=1236]
+STOP SLAVE SQL_THREAD;
+include/wait_for_slave_sql_to_stop.inc
+CHANGE MASTER TO master_use_gtid=no;
+include/start_slave.inc
+SELECT * FROM t1 ORDER BY a;
+a
+1
+2
 DROP TABLE t1;
 include/rpl_end.inc

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

@@ -43,6 +43,7 @@ SELECT * FROM t1 ORDER BY a;
 --source include/kill_binlog_dump_threads.inc
 INSERT INTO t1 VALUES (10);
 SET @old_dbug= @@GLOBAL.debug_dbug;
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 --save_master_pos
 
@@ -88,6 +89,7 @@ SELECT * FROM t1 ORDER BY a;
 --connection server_1
 --source include/kill_binlog_dump_threads.inc
 INSERT INTO t1 VALUES (20);
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 --save_master_pos
 
@@ -127,6 +129,7 @@ SET gtid_seq_no= 200;
 INSERT INTO t1 VALUES (13);
 SET gtid_domain_id= 10;
 
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,gtid_force_reconnect_at_10_1_100";
 
 --connection server_2
@@ -158,6 +161,7 @@ TRUNCATE t1;
 RESET MASTER;
 
 --source include/kill_binlog_dump_threads.inc
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
 SET GLOBAL debug_dbug="+d,binlog_force_reconnect_after_22_events";
 
 # 4 events for FD, fake rotate, gtid list, binlog checkpoint.

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

@@ -277,6 +277,75 @@ SET GLOBAL gtid_slave_pos='0-1-2';
 --let $value= query_get_value(SHOW ALL SLAVES STATUS, "Gtid_Slave_Pos", 1)
 eval SELECT '$value' AS Gtid_Slave_Pos;
 
+
+--echo *** MDEV-4483: Slave loses traditional master coordinates immediately on CHANGE MASTER TO MASTER_USE_GTID = 1 ***
+--connection server_2
+--source include/stop_slave.inc
+DROP TABLE t1;
+RESET SLAVE ALL;
+RESET MASTER;
+SET GLOBAL gtid_slave_pos= "";
+
+# Set up old-style replication. The bug was that CHANGE MASTER would clear
+# out the old-style binlog/relaylog coordinates when it should not.
+
+--replace_result $MASTER_MYPORT MASTER_PORT
+eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$MASTER_MYPORT, master_user='root', master_use_gtid=no, master_log_file="", master_log_pos= 4;
+
+--connection server_1
+DROP TABLE t1;
+RESET MASTER;
+
+CREATE TABLE t1 (a INT PRIMARY KEY);
+--save_master_pos
+
+--connection server_2
+--source include/start_slave.inc
+--sync_with_master
+
+--source include/stop_slave.inc
+
+--connection server_1
+INSERT INTO t1 VALUES (1);
+--let $log_file1= query_get_value(SHOW MASTER STATUS, "File", 1)
+--let $log_pos1= query_get_value(SHOW MASTER STATUS, "Position", 1)
+INSERT INTO t1 VALUES (2);
+--let $log_file2= query_get_value(SHOW MASTER STATUS, "File", 1)
+--let $log_pos2= query_get_value(SHOW MASTER STATUS, "Position", 1)
+--save_master_pos
+
+--connection server_2
+# Get the slave to a point where the IO thread has fetched one event ahead
+# of the SQL thread (we want to test that CHANGE MASTER does not mess with
+# existing relay logs).
+--replace_result $log_file1 LOG_FILE1 $log_pos1 LOG_POS1
+eval START SLAVE UNTIL master_log_file='$log_file1', master_log_pos=$log_pos1;
+--source include/wait_for_slave_sql_to_stop.inc
+SELECT * FROM t1;
+--let $slave_param= Read_Master_Log_Pos
+--let $slave_param_value= $log_pos2
+--source include/wait_for_slave_param.inc
+--source include/stop_slave_io.inc
+
+# Test that we can change to GTID and back without loosing our
+# old-style slave position.
+CHANGE MASTER TO master_use_gtid=slave_pos;
+# Unknown GTID, so slave will fail to connect.
+SET GLOBAL gtid_slave_pos="0-42-42";
+SET sql_log_bin=0;
+call mtr.add_suppression("Error: connecting slave requested to start from GTID");
+SET sql_log_bin=1;
+START SLAVE;
+--let $slave_io_errno= 1236
+--source include/wait_for_slave_io_error.inc
+STOP SLAVE SQL_THREAD;
+--source include/wait_for_slave_sql_to_stop.inc
+CHANGE MASTER TO master_use_gtid=no;
+
+--source include/start_slave.inc
+--sync_with_master
+SELECT * FROM t1 ORDER BY a;
+
 # Clean up.
 --connection server_1
 DROP TABLE t1;

scripts/mysql_system_tables.sql

@@ -24,7 +24,7 @@ set sql_mode='';
 -- We want this to be created with the default storage engine.
 -- This way, if InnoDB is used we get crash safety, and if MyISAM is used
 -- we avoid mixed-engine transactions.
-CREATE TABLE IF NOT EXISTS gtid_slave_pos (domain_id INT UNSIGNED NOT NULL, sub_id BIGINT UNSIGNED NOT NULL, server_id INT UNSIGNED NOT NULL, seq_no BIGINT UNSIGNED NOT NULL, PRIMARY KEY (domain_id, sub_id)) comment='Replication slave GTID state';
+CREATE TABLE IF NOT EXISTS gtid_slave_pos (domain_id INT UNSIGNED NOT NULL, sub_id BIGINT UNSIGNED NOT NULL, server_id INT UNSIGNED NOT NULL, seq_no BIGINT UNSIGNED NOT NULL, PRIMARY KEY (domain_id, sub_id)) comment='Replication slave GTID position';
 
 set storage_engine=myisam;
 flush tables;

sql/slave.cc

@@ -873,9 +873,21 @@ int start_slave_threads(bool need_slave_mutex, bool wait_for_start,
   if (mi->using_gtid != Master_info::USE_GTID_NO &&
       !mi->slave_running && !mi->rli.slave_running)
   {
+    /*
+      purge_relay_logs() clears the mi->rli.group_master_log_pos.
+      So save and restore them, like we do in CHANGE MASTER.
+      (We are not going to use them for GTID, but it might be worth to
+      keep them in case connection with GTID fails and user wants to go
+      back and continue with previous old-style replication coordinates).
+    */
+    mi->master_log_pos = max(BIN_LOG_HEADER_SIZE, mi->rli.group_master_log_pos);
+    strmake(mi->master_log_name, mi->rli.group_master_log_name,
+            sizeof(mi->master_log_name)-1);
     purge_relay_logs(&mi->rli, NULL, 0, &errmsg);
-    mi->master_log_name[0]= 0;
-    mi->master_log_pos= 0;
+    mi->rli.group_master_log_pos= mi->master_log_pos;
+    strmake(mi->rli.group_master_log_name, mi->master_log_name,
+            sizeof(mi->rli.group_master_log_name)-1);
+
     error= rpl_load_gtid_state(&mi->gtid_current_pos, mi->using_gtid ==
                                              Master_info::USE_GTID_CURRENT_POS);
     mi->events_queued_since_last_gtid= 0;

sql/sql_repl.cc

@@ -3293,16 +3293,6 @@ bool change_master(THD* thd, Master_info* mi, bool *master_info_added)
       ret= TRUE;
       goto err;
     }
-
-    if (mi->using_gtid != Master_info::USE_GTID_NO)
-    {
-      /*
-        Clear the position in the master binlogs, so that we request the
-        correct GTID position.
-      */
-      mi->master_log_name[0]= 0;
-      mi->master_log_pos= 0;
-    }
   }
   else
   {