MDEV-4688: empty @@gtid_slave_pos during slave commit.

In record_gtid(), too many rows were deleted from the slave position
hash - we need to always keep on to the most recent committed row,
so we have a valid slave position at all times.

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

@@ -90,23 +90,28 @@ a
 2
 3
 4
-*** Test slave requesting a GTID that is not present in the master's binlog ***
+*** MDEV-4688: Empty value of @@GLOBAL.gtid_slave_pos ***
 include/stop_slave.inc
-SET GLOBAL gtid_slave_pos = "0-1-3";
-START SLAVE;
-SET sql_log_bin=0;
-CALL mtr.add_suppression("Got fatal error .* from master when reading data from binary log: 'Error: connecting slave requested to start from GTID .*, which is not in the master's binlog'");
-SET sql_log_bin=1;
-include/wait_for_slave_io_error.inc [errno=1236]
-Slave_IO_State = ''
-Last_IO_Errno = '1236'
-Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Error: connecting slave requested to start from GTID 0-1-3, which is not in the master's binlog''
-Using_Gtid = 'Current_Pos'
-include/stop_slave.inc
-SET GLOBAL gtid_slave_pos = "0-1-2";
-START SLAVE;
-include/wait_for_slave_to_start.inc
 INSERT INTO t1 VALUES (5);
+SET @old_dbug= @@GLOBAL.debug_dbug;
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
+SET GLOBAL debug_dbug="+d,gtid_fail_after_record_gtid";
+SET sql_log_bin=0;
+CALL mtr.add_suppression('Got error 131 "Command not supported by database" during COMMIT');
+SET sql_log_bin=1;
+START SLAVE;
+include/wait_for_slave_sql_error.inc [errno=1180]
+SELECT @@GLOBAL.gtid_slave_pos;
+@@GLOBAL.gtid_slave_pos
+0-1-2
+SELECT * FROM t1 ORDER BY a;
+a
+1
+2
+3
+4
+SET GLOBAL debug_dbug= @old_dbug;
+START SLAVE SQL_THREAD;
 SELECT * FROM t1 ORDER BY a;
 a
 1
@@ -114,6 +119,31 @@ a
 3
 4
 5
+*** Test slave requesting a GTID that is not present in the master's binlog ***
+include/stop_slave.inc
+SET GLOBAL gtid_slave_pos = "0-1-4";
+START SLAVE;
+SET sql_log_bin=0;
+CALL mtr.add_suppression("Got fatal error .* from master when reading data from binary log: 'Error: connecting slave requested to start from GTID .*, which is not in the master's binlog'");
+SET sql_log_bin=1;
+include/wait_for_slave_io_error.inc [errno=1236]
+Slave_IO_State = ''
+Last_IO_Errno = '1236'
+Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Error: connecting slave requested to start from GTID 0-1-4, which is not in the master's binlog''
+Using_Gtid = 'Current_Pos'
+include/stop_slave.inc
+SET GLOBAL gtid_slave_pos = "0-1-3";
+START SLAVE;
+include/wait_for_slave_to_start.inc
+INSERT INTO t1 VALUES (6);
+SELECT * FROM t1 ORDER BY a;
+a
+1
+2
+3
+4
+5
+6
 *** MDEV-4278: Slave does not detect that master is not GTID-aware ***
 include/stop_slave.inc
 SET @old_dbug= @@global.DEBUG_DBUG;
@@ -121,7 +151,7 @@ SET GLOBAL debug_dbug="+d,simulate_non_gtid_aware_master";
 START SLAVE;
 include/wait_for_slave_io_error.inc [errno=1233]
 SET GLOBAL debug_dbug= @old_dbug;
-INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
 START SLAVE;
 SET sql_log_bin=0;
 CALL mtr.add_suppression("The slave I/O thread stops because master does not support MariaDB global transaction id");

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

@@ -114,10 +114,41 @@ START SLAVE;
 --source include/wait_condition.inc
 SELECT * FROM t1 ORDER BY a;
 
+--echo *** MDEV-4688: Empty value of @@GLOBAL.gtid_slave_pos ***
+# The problem was that record_gtid() deleted too much of the in-memory state,
+# leaving the state empty until after commit when we add the newly committed
+# GTID. Test this by forcing an error after the delete of the old data but
+# before the add of new data.
+
+--source include/stop_slave.inc
+
+--connection master
+# This will be GTID 0-1-3
+INSERT INTO t1 VALUES (5);
+
+--connection slave
+SET @old_dbug= @@GLOBAL.debug_dbug;
+SET GLOBAL debug_dbug="+d,dummy_disable_default_dbug_output";
+SET GLOBAL debug_dbug="+d,gtid_fail_after_record_gtid";
+SET sql_log_bin=0;
+CALL mtr.add_suppression('Got error 131 "Command not supported by database" during COMMIT');
+SET sql_log_bin=1;
+START SLAVE;
+--let $slave_sql_errno= 1180
+--source include/wait_for_slave_sql_error.inc
+# The bug was that @@GLOBAL.gtid_slave_pos was empty here.
+SELECT @@GLOBAL.gtid_slave_pos;
+SELECT * FROM t1 ORDER BY a;
+SET GLOBAL debug_dbug= @old_dbug;
+START SLAVE SQL_THREAD;
+--let $wait_condition= SELECT COUNT(*) = 5 FROM t1
+--source include/wait_condition.inc
+SELECT * FROM t1 ORDER BY a;
+
 
 --echo *** Test slave requesting a GTID that is not present in the master's binlog ***
 --source include/stop_slave.inc
-SET GLOBAL gtid_slave_pos = "0-1-3";
+SET GLOBAL gtid_slave_pos = "0-1-4";
 START SLAVE;
 
 SET sql_log_bin=0;
@@ -130,15 +161,15 @@ SET sql_log_bin=1;
 
 --let $rpl_only_running_threads= 1
 --source include/stop_slave.inc
-SET GLOBAL gtid_slave_pos = "0-1-2";
+SET GLOBAL gtid_slave_pos = "0-1-3";
 START SLAVE;
 --source include/wait_for_slave_to_start.inc
 
 --connection master
-INSERT INTO t1 VALUES (5);
+INSERT INTO t1 VALUES (6);
 
 --connection slave
---let $wait_condition= SELECT COUNT(*) = 5 FROM t1
+--let $wait_condition= SELECT COUNT(*) = 6 FROM t1
 --source include/wait_condition.inc
 SELECT * FROM t1 ORDER BY a;
 
@@ -159,7 +190,7 @@ START SLAVE;
 
 --connection master
 SET GLOBAL debug_dbug= @old_dbug;
-INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
 --save_master_pos
 
 --connection slave

sql/log_event.cc

@@ -6927,6 +6927,12 @@ int Xid_log_event::do_apply_event(Relay_log_info const *rli)
       thd->is_slave_error= 1;
       return err;
     }
+
+    DBUG_EXECUTE_IF("gtid_fail_after_record_gtid",
+        { my_error(ER_ERROR_DURING_COMMIT, MYF(0), HA_ERR_WRONG_COMMAND);
+          thd->is_slave_error= 1;
+          return 1;
+        });
   }
 
   /* For a slave Xid_log_event is COMMIT */

sql/rpl_gtid.cc

@@ -363,6 +363,14 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
     goto end;
   }
 
+  if(opt_bin_log &&
+     (err= mysql_bin_log.bump_seq_no_counter_if_needed(gtid->domain_id,
+                                                       gtid->seq_no)))
+  {
+    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+    goto end;
+  }
+
   lock();
   if ((elem= get_element(gtid->domain_id)) == NULL)
   {
@@ -371,7 +379,30 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
     err= 1;
     goto end;
   }
-  elist= elem->grab_list();
+  if ((elist= elem->grab_list()) != NULL)
+  {
+    /* Delete any old stuff, but keep around the most recent one. */
+    list_element *cur= elist;
+    uint64 best_sub_id= cur->sub_id;
+    list_element **best_ptr_ptr= &elist;
+    while ((next= cur->next))
+    {
+      if (next->sub_id > best_sub_id)
+      {
+        best_sub_id= next->sub_id;
+        best_ptr_ptr= &cur->next;
+      }
+      cur= next;
+    }
+    /*
+      Delete the highest sub_id element from the old list, and put it back as
+      the single-element new list.
+    */
+    cur= *best_ptr_ptr;
+    *best_ptr_ptr= cur->next;
+    cur->next= NULL;
+    elem->list= cur;
+  }
   unlock();
 
   if (!elist)
@@ -393,7 +424,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
     DBUG_EXECUTE_IF("gtid_slave_pos_simulate_failed_delete",
                     { err= ENOENT;
                       table->file->print_error(err, MYF(0));
-                      /* `break' does not work in DBUG_EXECUTE_IF */
+                      /* `break' does not work inside DBUG_EXECUTE_IF */
                       goto dbug_break; });
 
     next= elist->next;
@@ -420,11 +451,6 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
 IF_DBUG(dbug_break:, )
   table->file->ha_index_end();
 
-  if(!err && opt_bin_log &&
-     (err= mysql_bin_log.bump_seq_no_counter_if_needed(gtid->domain_id,
-                                                       gtid->seq_no)))
-    my_error(ER_OUT_OF_RESOURCES, MYF(0));
-
 end:
 
   if (table_opened)