BUG#12691 (Exec_master_log_pos corrupted with SQL_SLAVE_SKIP_COUNTER):

  
Adding code to keep skipping events while inside a transaction. Execution
will start just after the transaction has been skipped.


sql/slave.cc:
  Adding code to set the thd->options flag for the slave SQL thread
  even when BEGIN, ROLLBACK, COMMIT, and XID events are being skipped.
      
  Adding code to not decrease the slave skip counter from 1 to 0 if we
  are inside a transaction. This will keep the counter at 1, and keep
  skipping events, until a transaction terminator is read. At that point,
  the slave skip counter will be decreased to 0, and events will be read
  and executed instead of read and skipped.
mysql-test/r/rpl_slave_skip.result:
  New BitKeeper file ``mysql-test/r/rpl_slave_skip.result''
mysql-test/t/rpl_slave_skip-slave.opt:
  New BitKeeper file ``mysql-test/t/rpl_slave_skip-slave.opt''
mysql-test/t/rpl_slave_skip.test:
  New BitKeeper file ``mysql-test/t/rpl_slave_skip.test''

mysql-test/r/rpl_slave_skip.result

+stop slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+reset master;
+reset slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+start slave;
+**** On Master ****
+CREATE TABLE t1 (a INT, b SET('master','slave')) ENGINE=INNODB;
+CREATE TABLE t2 (a INT, b SET('master','slave')) ENGINE=MYISAM;
+==== Skipping normal transactions ==== 
+**** On Slave ****
+STOP SLAVE;
+**** On Master ****
+BEGIN;
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+BEGIN;
+INSERT INTO t1 VALUES (4, 'master,slave');
+INSERT INTO t1 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, 'master,slave');
+COMMIT;
+SELECT * FROM t1 ORDER BY a;
+a	b
+1	master
+2	master
+3	master
+4	master,slave
+5	master,slave
+6	master,slave
+**** On Slave ****
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+SELECT * FROM t1 ORDER BY a;
+a	b
+4	master,slave
+5	master,slave
+6	master,slave
+**** On Master ****
+DELETE FROM t1;
+==== Skipping two normal transactions ==== 
+**** On Slave ****
+STOP SLAVE;
+**** On Master ****
+BEGIN;
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+BEGIN;
+INSERT INTO t1 VALUES (4, 'master');
+INSERT INTO t1 VALUES (5, 'master');
+INSERT INTO t1 VALUES (6, 'master');
+COMMIT;
+BEGIN;
+INSERT INTO t1 VALUES (7, 'master,slave');
+INSERT INTO t1 VALUES (8, 'master,slave');
+INSERT INTO t1 VALUES (9, 'master,slave');
+COMMIT;
+SELECT * FROM t1 ORDER BY a;
+a	b
+1	master
+2	master
+3	master
+4	master
+5	master
+6	master
+7	master,slave
+8	master,slave
+9	master,slave
+**** On Slave ****
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=8;
+START SLAVE;
+SELECT * FROM t1 ORDER BY a;
+a	b
+7	master,slave
+8	master,slave
+9	master,slave
+**** On Master ****
+DELETE FROM t1;
+==== Skipping without autocommit ==== 
+**** On Slave ****
+STOP SLAVE;
+**** On Master ****
+SET AUTOCOMMIT=0;
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+INSERT INTO t1 VALUES (4, 'master,slave');
+INSERT INTO t1 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, 'master,slave');
+COMMIT;
+SELECT * FROM t1 ORDER BY a;
+a	b
+1	master
+2	master
+3	master
+4	master,slave
+5	master,slave
+6	master,slave
+**** On Slave ****
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+SELECT * FROM t1 ORDER BY a;
+a	b
+4	master,slave
+5	master,slave
+6	master,slave
+==== Rollback of transaction with non-transactional change ==== 
+**** On Master ****
+DELETE FROM t1;
+SET AUTOCOMMIT=1;
+**** On Slave ****
+STOP SLAVE;
+**** On Master ****
+BEGIN;
+INSERT INTO t1 VALUES (1, '');
+INSERT INTO t2 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, '');
+ROLLBACK;
+BEGIN;
+INSERT INTO t1 VALUES (4, '');
+INSERT INTO t2 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, '');
+ROLLBACK;
+SELECT * FROM t1 ORDER BY a;
+a	b
+SELECT * FROM t2 ORDER BY a;
+a	b
+2	master
+5	master,slave
+**** On Slave ****
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+SELECT * FROM t1 ORDER BY a;
+a	b
+SELECT * FROM t2 ORDER BY a;
+a	b
+5	master,slave
+==== Cleanup ==== 
+**** On Master ****
+DROP TABLE t1, t2;

mysql-test/t/rpl_slave_skip-slave.opt

+--innodb

mysql-test/t/rpl_slave_skip.test

+source include/have_innodb.inc;
+source include/master-slave.inc;
+
+# This test is for checking that the use of SQL_SLAVE_SKIP_COUNTER
+# behaves as expected, i.e., that it is guaranteed to skip an entire
+# group and not start executing in the middle of a transaction.
+
+# We are checking the correct behaviour when using both a
+# transactional and non-transactional table. The non-transactional
+# table comes into play when rolling back a transaction containing a
+# write to this table. In that case, the transaction should still be
+# written to the binary log, and the slave will apply it and then roll
+# it back to get the non-transactional change into the table.
+
+--echo **** On Master ****
+CREATE TABLE t1 (a INT, b SET('master','slave')) ENGINE=INNODB;
+CREATE TABLE t2 (a INT, b SET('master','slave')) ENGINE=MYISAM;
+
+--echo ==== Skipping normal transactions ==== 
+
+--echo **** On Slave ****
+sync_slave_with_master;
+STOP SLAVE;
+source include/wait_for_slave_to_stop.inc;
+
+--echo **** On Master ****
+connection master;
+
+BEGIN;
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+
+BEGIN;
+INSERT INTO t1 VALUES (4, 'master,slave');
+INSERT INTO t1 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, 'master,slave');
+COMMIT;
+
+save_master_pos;
+
+SELECT * FROM t1 ORDER BY a;
+
+# This will skip a begin event and the first INSERT of the
+# transaction, and it should keep skipping until it has reached the
+# transaction terminator.
+
+--echo **** On Slave ****
+connection slave;
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+source include/wait_for_slave_to_start.inc;
+sync_with_master;
+SELECT * FROM t1 ORDER BY a;
+
+--echo **** On Master ****
+connection master;
+DELETE FROM t1;
+sync_slave_with_master;
+
+--echo ==== Skipping two normal transactions ==== 
+
+--echo **** On Slave ****
+connection slave;
+STOP SLAVE;
+source include/wait_for_slave_to_stop.inc;
+
+--echo **** On Master ****
+connection master;
+
+BEGIN;
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+
+BEGIN;
+INSERT INTO t1 VALUES (4, 'master');
+INSERT INTO t1 VALUES (5, 'master');
+INSERT INTO t1 VALUES (6, 'master');
+COMMIT;
+
+BEGIN;
+INSERT INTO t1 VALUES (7, 'master,slave');
+INSERT INTO t1 VALUES (8, 'master,slave');
+INSERT INTO t1 VALUES (9, 'master,slave');
+COMMIT;
+
+save_master_pos;
+
+SELECT * FROM t1 ORDER BY a;
+
+# This will skip a begin event and the first INSERT of the
+# transaction, and it should keep skipping until it has reached the
+# transaction terminator.
+
+--echo **** On Slave ****
+connection slave;
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=8;
+START SLAVE;
+source include/wait_for_slave_to_start.inc;
+sync_with_master;
+SELECT * FROM t1 ORDER BY a;
+
+--echo **** On Master ****
+connection master;
+DELETE FROM t1;
+sync_slave_with_master;
+
+--echo ==== Skipping without autocommit ==== 
+
+# Testing without using autocommit instead. It should still write a
+# BEGIN event, so the behaviour should be the same
+
+--echo **** On Slave ****
+connection slave;
+STOP SLAVE;
+source include/wait_for_slave_to_stop.inc;
+
+--echo **** On Master ****
+connection master;
+SET AUTOCOMMIT=0;
+
+INSERT INTO t1 VALUES (1, 'master');
+INSERT INTO t1 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, 'master');
+COMMIT;
+
+INSERT INTO t1 VALUES (4, 'master,slave');
+INSERT INTO t1 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, 'master,slave');
+COMMIT;
+
+save_master_pos;
+
+SELECT * FROM t1 ORDER BY a;
+
+# This will skip a begin event and the first INSERT of the
+# transaction, and it should keep skipping until it has reached the
+# transaction terminator.
+
+--echo **** On Slave ****
+connection slave;
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+source include/wait_for_slave_to_start.inc;
+sync_with_master;
+SELECT * FROM t1 ORDER BY a;
+
+# Testing with a non-transactional table in the transaction. This will
+# log a ROLLBACK as a transaction terminator, which is a normal Query
+# log event.
+
+--echo ==== Rollback of transaction with non-transactional change ==== 
+
+--echo **** On Master ****
+connection master;
+DELETE FROM t1;
+SET AUTOCOMMIT=1;
+
+--echo **** On Slave ****
+sync_slave_with_master;
+STOP SLAVE;
+source include/wait_for_slave_to_stop.inc;
+
+--echo **** On Master ****
+connection master;
+disable_warnings;
+BEGIN;
+INSERT INTO t1 VALUES (1, '');
+INSERT INTO t2 VALUES (2, 'master');
+INSERT INTO t1 VALUES (3, '');
+ROLLBACK;
+
+BEGIN;
+INSERT INTO t1 VALUES (4, '');
+INSERT INTO t2 VALUES (5, 'master,slave');
+INSERT INTO t1 VALUES (6, '');
+ROLLBACK;
+enable_warnings;
+
+save_master_pos;
+
+SELECT * FROM t1 ORDER BY a;
+SELECT * FROM t2 ORDER BY a;
+
+--echo **** On Slave ****
+connection slave;
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
+START SLAVE;
+source include/wait_for_slave_to_start.inc;
+sync_with_master;
+
+SELECT * FROM t1 ORDER BY a;
+SELECT * FROM t2 ORDER BY a;
+
+--echo ==== Cleanup ==== 
+
+--echo **** On Master ****
+connection master;
+DROP TABLE t1, t2;
+sync_slave_with_master;

sql/slave.cc

@@ -3279,7 +3279,43 @@ static int exec_relay_log_event(THD* thd, RELAY_LOG_INFO* rli)
       now the relay log starts with its Format_desc, has a Rotate etc).
     */
 
-    DBUG_PRINT("info",("type_code=%d, server_id=%d",type_code,ev->server_id));
+    DBUG_PRINT("info",("type_code: %d; server_id: %d; slave_skip_counter: %d",
+                       type_code, ev->server_id, rli->slave_skip_counter));
+
+    /*
+      If the slave skip counter is positive, we still need to set the
+      OPTION_BEGIN flag correctly and not skip the log events that
+      start or end a transaction. If we do this, the slave will not
+      notice that it is inside a transaction, and happily start
+      executing from inside the transaction.
+
+      Note that the code block below is strictly 5.0.
+     */
+#if MYSQL_VERSION_ID < 50100
+    if (unlikely(rli->slave_skip_counter > 0))
+    {
+      switch (type_code)
+      {
+      case QUERY_EVENT:
+      {
+        Query_log_event* const qev= (Query_log_event*) ev;
+        DBUG_PRINT("info", ("QUERY_EVENT { query: '%s', q_len: %u }",
+                            qev->query, qev->q_len));
+        if (memcmp("BEGIN", qev->query, qev->q_len+1) == 0)
+          thd->options|= OPTION_BEGIN;
+        else if (memcmp("COMMIT", qev->query, qev->q_len+1) == 0 ||
+                 memcmp("ROLLBACK", qev->query, qev->q_len+1) == 0)
+          thd->options&= ~OPTION_BEGIN;
+      }
+      break;
+
+      case XID_EVENT:
+        DBUG_PRINT("info", ("XID_EVENT"));
+        thd->options&= ~OPTION_BEGIN;
+        break;
+      }
+    }
+#endif
 
     if ((ev->server_id == (uint32) ::server_id &&
          !replicate_same_server_id &&
@@ -3301,6 +3337,9 @@ static int exec_relay_log_event(THD* thd, RELAY_LOG_INFO* rli)
         flush_relay_log_info(rli);
       }
 
+      DBUG_PRINT("info", ("thd->options: %s",
+                          (thd->options & OPTION_BEGIN) ? "OPTION_BEGIN" : ""))
+
       /*
         Protect against common user error of setting the counter to 1
         instead of 2 while recovering from an insert which used auto_increment,
@@ -3311,6 +3350,15 @@ static int exec_relay_log_event(THD* thd, RELAY_LOG_INFO* rli)
              type_code == RAND_EVENT ||
              type_code == USER_VAR_EVENT) &&
             rli->slave_skip_counter == 1) &&
+#if MYSQL_VERSION_ID < 50100
+          /*
+            Decrease the slave skip counter only if we are not inside
+            a transaction or the slave skip counter is more than
+            1. The slave skip counter will be decreased from 1 to 0
+            when reaching the final ROLLBACK, COMMIT, or XID_EVENT.
+           */
+          (!(thd->options & OPTION_BEGIN) || rli->slave_skip_counter > 1) &&
+#endif
           /*
             The events from ourselves which have something to do with the relay
             log itself must be skipped, true, but they mustn't decrement
@@ -3321,8 +3369,10 @@ static int exec_relay_log_event(THD* thd, RELAY_LOG_INFO* rli)
             would not be skipped.
           */
           !(ev->server_id == (uint32) ::server_id &&
-            (type_code == ROTATE_EVENT || type_code == STOP_EVENT ||
-             type_code == START_EVENT_V3 || type_code == FORMAT_DESCRIPTION_EVENT)))
+            (type_code == ROTATE_EVENT ||
+             type_code == STOP_EVENT ||
+             type_code == START_EVENT_V3 ||
+             type_code == FORMAT_DESCRIPTION_EVENT)))
         --rli->slave_skip_counter;
       pthread_mutex_unlock(&rli->data_lock);
       delete ev;