Merge lp:~gl-az/percona-server/BT-26980-5.5-bug1068210 into lp:percona-server/5.5

[George Ormond Lorch III]

Fix for lp1068210 which is also reported as upstream 67504.

If when processing slave replication events, a trigger is fired, any slave 'deferred' events such as LAST_INSERT_ID are replayed within the trigger sub statement. Simple test added to check to see if execution is within a sub statement before attempting to execute any deferred events.

Since only test case was modified, re-ran jenkins for just that case.

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/680/

[George Ormond Lorch III (gl-az)]

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/666/

[Laurynas Biveinis (laurynas-biveinis)]

    - The code looks OK. I am not sure why there is no
      deferred_events::rewind() call immediatelly after or in
      d_e::execute() method, which would seem to resolve this issue,
      but oh well.
    - The testcase:
    - Any reason to mark this test for SBR/MBR and not RBR? If I
      understand correctly, it is only the SBR that triggers the
      bug. MBR would use RBR for all t1 events. So, let's just test
      SBR.
    - Please replace connection slave; with sync_slave_with_master at
      diff line 72, so that the presence of t1 is guaranteed at slave
      at the trigger create time.
    - Redundant "conection slave;" at line 85.
    - Lines 89 and 90 can be replaced by include/stop_slave.inc, but
      this for future reference, as in this instance the testcase
      cleanup should be done in another way:
    - Tables t1 and t2 would remain on the slave at the end of the
      testcase end, because their DROPs are not replicated. Thus,
      omit the slave stop, perform the slave-only cleanup, perform the
      (replicated) cleanup on master, and then include rpl_end.inc,
      which will sync, stop slave, and cleanup.

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

On the second thought, let's test all three SBR/RBR/MBR replication formats in the testcase. Even though the bug concerns SBR only, the testcase has triggers and slave-only tables, that's interesting for RBR too.

[George Ormond Lorch III (gl-az)]

> - Any reason to mark this test for SBR/MBR and not RBR? If I
> understand correctly, it is only the SBR that triggers the
> bug. MBR would use RBR for all t1 events. So, let's just test
> SBR.

This test will not work with RBR. It seems that if RBR is used, the slave side trigger is not fired and thus the rows replicated into t1 never create the corresponding rows in slave_only, causing the test case to fail on SELECT id FROM slave_only; IT _does_ seem to work fine for SBR and MBR though.

> - Please replace connection slave; with sync_slave_with_master at
> diff line 72, so that the presence of t1 is guaranteed at slave
> at the trigger create time.

Good catch.

> - Redundant "conection slave;" at line 85.

Another good catch.

> - Lines 89 and 90 can be replaced by include/stop_slave.inc, but
> this for future reference, as in this instance the testcase
> cleanup should be done in another way:
> - Tables t1 and t2 would remain on the slave at the end of the
> testcase end, because their DROPs are not replicated. Thus,
> omit the slave stop, perform the slave-only cleanup, perform the
> (replicated) cleanup on master, and then include rpl_end.inc,
> which will sync, stop slave, and cleanup.

Ahh, very cool. I was a bit uncertain as to the 'correct' way to clean up the slave in this situation and didn't know about rpl_end.inc. Much cleaner now, Thanks!

[Laurynas Biveinis (laurynas-biveinis)]

review: Approve

=== added file 'Percona-Server/mysql-test/suite/rpl/r/rpl_trigger_bug67504.result'
--- Percona-Server/mysql-test/suite/rpl/r/rpl_trigger_bug67504.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/rpl/r/rpl_trigger_bug67504.result	2013-02-18 21:23:23 +0000
@@ -0,0 +1,28 @@
+include/master-slave.inc
+[connection master]
+DROP TRIGGER IF EXISTS t1_insert;
+DROP TABLE IF EXISTS slave_only;
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+CREATE TABLE t1(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE t2(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE slave_only(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+CREATE TRIGGER t1_insert AFTER INSERT ON t1 FOR EACH ROW INSERT INTO slave_only VALUES(NULL);
+INSERT INTO slave_only VALUES(NULL);
+INSERT INTO slave_only VALUES(NULL);
+INSERT INTO t2 VALUES(NULL);
+INSERT INTO t2 VALUES(NULL);
+INSERT INTO t1 VALUES(NULL);
+SELECT id FROM t1;
+id
+1
+SELECT id FROM slave_only;
+id
+1
+2
+3
+DROP TRIGGER t1_insert;
+DROP TABLE slave_only;
+DROP TABLE t1;
+DROP TABLE t2;
+include/rpl_end.inc

=== added file 'Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504-slave.opt'
--- Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504-slave.opt	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504-slave.opt	2013-02-18 21:23:23 +0000
@@ -0,0 +1,1 @@
+--replicate_ignore_table=test.t2

=== added file 'Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504.test'
--- Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/rpl/t/rpl_trigger_bug67504.test	2013-02-18 21:23:23 +0000
@@ -0,0 +1,49 @@
+########################################################################
+# Bug 67504: Duplicate error in replication with slave triggers and
+#            auto increment
+########################################################################
+--source include/have_innodb.inc
+--source include/have_binlog_format_mixed_or_statement.inc 
+--source include/master-slave.inc
+
+disable_query_log;
+call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
+enable_query_log;
+
+--disable_warnings
+connection slave;
+DROP TRIGGER IF EXISTS t1_insert;
+DROP TABLE IF EXISTS slave_only;
+connection master;
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+sync_slave_with_master;
+--enable_warnings
+
+connection master;
+CREATE TABLE t1(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE t2(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+
+sync_slave_with_master;
+CREATE TABLE slave_only(id INT AUTO_INCREMENT PRIMARY KEY) ENGINE=INNODB;
+CREATE TRIGGER t1_insert AFTER INSERT ON t1 FOR EACH ROW INSERT INTO slave_only VALUES(NULL);
+INSERT INTO slave_only VALUES(NULL);
+INSERT INTO slave_only VALUES(NULL);
+
+connection master;
+INSERT INTO t2 VALUES(NULL);
+INSERT INTO t2 VALUES(NULL);
+INSERT INTO t1 VALUES(NULL);
+
+sync_slave_with_master;
+SELECT id FROM t1;
+SELECT id FROM slave_only;
+
+DROP TRIGGER t1_insert;
+DROP TABLE slave_only;
+
+connection master;
+DROP TABLE t1;
+DROP TABLE t2;
+
+--source include/rpl_end.inc 

=== modified file 'Percona-Server/sql/sql_parse.cc'
--- Percona-Server/sql/sql_parse.cc	2013-01-30 09:55:26 +0000
+++ Percona-Server/sql/sql_parse.cc	2013-02-18 21:23:23 +0000
@@ -2171,9 +2171,14 @@
     }
     /* 
        Execute deferred events first
+       Bug lp1068210 or upstream 67504: Test first to see if we are executing
+       within a sub statement. If so, DO NOT execute any deferred events, they
+       have already been executed by the parent statement and have no bearing
+       on the sub statement and can cause faulty behavior.
     */
-    if (slave_execute_deferred_events(thd))
-      DBUG_RETURN(-1);
+    if (thd->in_sub_stmt == 0)
+      if (slave_execute_deferred_events(thd))
+          DBUG_RETURN(-1);
   }
   else
   {