Merge lp:~laurynas-biveinis/percona-server/fake-changes-binlog-5.1 into lp:percona-server/5.1

[Laurynas Biveinis]

No BT or ST, but prerequisite for BT 20439, as well as being quite critical bug for the fake changes feature.

Fix

- bug 1190580 (Fake changes transactions are binlogged) and
- bug 1188162 (Verify in MTR that fake changes transactions are not
  binlogged).

Fixed bug 1188162 by adding a testcase
binlog_percona_fake_changes.test that executes a fake changes workload
and checks that no binlog events have resulted from this. The
testcase is adopted from a similar testcase in Facebook patch
rpl_percona_fake_changes.test with the difference that we check the
binlog only instead of setting up replication.

This testcase then immediatelly fails showing that fake changes
transactions are binlogged (bug 1190580). This is fixed by the
following.
- Adjusting innobase_xa_prepare() to return early with
  HA_ERR_WRONG_COMMAND instead of 0 (success) if fake changes are
  enabled and the function was called for a COMMIT statement or a DML
  statement with autocommit=1. Also clear the diagnostic area. Keep
  on returning success if innobase_xa_prepare() is called for
  individual statements in a multi-statement tranaction. This causes
  ha_commit_trans() not to write the transaction to binlog if fake
  changes are enabled and 2PC between the storage engines and binlog
  is used.
- Instead of skipping the 2PC prepare step for handlertons that are
  not marked as read-write, skip it only for handlertons that are not
  marked as read-write and have not fake changes enabled. The fake
  changes handlertons are never marked as read-write, which causes
  prepare to be skipped for them, making it impossible for its
  non-success return to do anything.
- Related to above, 2PC is only used if the total number of read-write
  handlertons in the transaction is at least two. Since fake changes
  handlertons do not contribute to this number, add a loop in
  ha_commit_trans() to bump rw_ha_count for each fake changes
  transaction, which will then force 2PC to be used.
- To support the above, add a new function pointer is_fake_change to
  the handlerton struct. This is also done by the Facebook patch.

http://jenkins.percona.com/job/percona-server-5.1-param/558/

[Vlad Lesin (vlad-lesin)]

review: Approve (g2)

[Alexey Kopytov (akopytov)]

Laurynas,

Are there any reasons to implement a separate a loop over trans->ha_list to bump rw_ha_count instead of embedding it into the existing loop in ha_check_and_coalesce_trx_read_only()? I guess that would result in a cleaner and a bit more efficient code.

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

This is a good suggestion, I will implement this.

=== added file 'Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result'
--- Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result	2013-08-22 12:56:44 +0000
@@ -0,0 +1,72 @@
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+DROP TABLE IF EXISTS t3;
+CREATE TABLE t1 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1);
+CREATE TABLE t2 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
+CREATE INDEX bx ON t2(b);
+INSERT INTO t2 VALUES (1,1);
+CREATE TABLE t3 (a INT PRIMARY KEY, b text) ENGINE=InnoDB;
+INSERT INTO t3 VALUES (1,'');
+RESET MASTER;
+SET innodb_fake_changes=1;
+SELECT * FROM t1;
+ERROR HY000: Got error 131 during COMMIT
+SELECT * FROM t2;
+ERROR HY000: Got error 131 during COMMIT
+SELECT * FROM t3;
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t1 VALUES (2,2);
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t2 VALUES (2,2);
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t3 VALUES (2,lpad('a',10000, 'b'));
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t1 SET a=0 where b=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t2 SET a=0 where b=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t3 SET a=0 where a=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t1 SET b=0 where a=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t2 SET b=0 where a=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t2 SET b=lpad('a',10000, 'z') where a=1;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t1 SET b=0 where a=2;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t2 SET b=0 where a=2;
+ERROR HY000: Got error 131 during COMMIT
+UPDATE t2 SET b=lpad('a',10000, 'z') where a=2;
+ERROR HY000: Got error 131 during COMMIT
+DELETE FROM t1 where b=2;
+ERROR HY000: Got error 131 during COMMIT
+DELETE FROM t2 where b=2;
+ERROR HY000: Got error 131 during COMMIT
+DELETE FROM t1 where a=2;
+ERROR HY000: Got error 131 during COMMIT
+DELETE FROM t2 where a=2;
+ERROR HY000: Got error 131 during COMMIT
+DELETE FROM t3 where a=2;
+ERROR HY000: Got error 131 during COMMIT
+REPLACE INTO t1 values (2,3);
+ERROR HY000: Got error 131 during COMMIT
+REPLACE INTO t2 values (2,3);
+ERROR HY000: Got error 131 during COMMIT
+REPLACE INTO t3 values (2,lpad('a',9000,'q'));
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t2 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t3 VALUES (1,1) ON DUPLICATE KEY UPDATE b=lpad('b',11000,'c');
+ERROR HY000: Got error 131 during COMMIT
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+must_be_1
+1
+SET innodb_fake_changes=default;
+DROP TABLE t1;
+DROP TABLE t2;
+DROP TABLE t3;

=== added file 'Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test'
--- Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test	2013-08-22 12:56:44 +0000
@@ -0,0 +1,106 @@
+#
+# Test that fake changes are not written to binlog
+# Based on rpl_percona_fake_changes.test in Facebook patch
+# https://bugs.launchpad.net/percona-server/+bug/1190580
+# https://bugs.launchpad.net/percona-server/+bug/1188162
+#
+
+--source include/have_log_bin.inc
+--source include/have_innodb_plugin.inc
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+DROP TABLE IF EXISTS t3;
+--enable_warnings
+
+CREATE TABLE t1 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1);
+
+CREATE TABLE t2 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
+CREATE INDEX bx ON t2(b);
+INSERT INTO t2 VALUES (1,1);
+
+CREATE TABLE t3 (a INT PRIMARY KEY, b text) ENGINE=InnoDB;
+INSERT INTO t3 VALUES (1,'');
+
+RESET MASTER;
+
+let $binlog_pos_1= query_get_value("SHOW MASTER STATUS", Position, 1);
+
+SET innodb_fake_changes=1;
+
+--error ER_ERROR_DURING_COMMIT
+SELECT * FROM t1;
+--error ER_ERROR_DURING_COMMIT
+SELECT * FROM t2;
+--error ER_ERROR_DURING_COMMIT
+SELECT * FROM t3;
+
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t1 VALUES (2,2);
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t2 VALUES (2,2);
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t3 VALUES (2,lpad('a',10000, 'b'));
+
+--error ER_ERROR_DURING_COMMIT
+UPDATE t1 SET a=0 where b=1;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t2 SET a=0 where b=1;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t3 SET a=0 where a=1;
+
+--error ER_ERROR_DURING_COMMIT
+UPDATE t1 SET b=0 where a=1;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t2 SET b=0 where a=1;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t2 SET b=lpad('a',10000, 'z') where a=1;
+
+--error ER_ERROR_DURING_COMMIT
+UPDATE t1 SET b=0 where a=2;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t2 SET b=0 where a=2;
+--error ER_ERROR_DURING_COMMIT
+UPDATE t2 SET b=lpad('a',10000, 'z') where a=2;
+
+--error ER_ERROR_DURING_COMMIT
+DELETE FROM t1 where b=2;
+--error ER_ERROR_DURING_COMMIT
+DELETE FROM t2 where b=2;
+
+--error ER_ERROR_DURING_COMMIT
+DELETE FROM t1 where a=2;
+--error ER_ERROR_DURING_COMMIT
+DELETE FROM t2 where a=2;
+--error ER_ERROR_DURING_COMMIT
+DELETE FROM t3 where a=2;
+
+--error ER_ERROR_DURING_COMMIT
+REPLACE INTO t1 values (2,3);
+--error ER_ERROR_DURING_COMMIT
+REPLACE INTO t2 values (2,3);
+--error ER_ERROR_DURING_COMMIT
+REPLACE INTO t3 values (2,lpad('a',9000,'q'));
+
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t2 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t3 VALUES (1,1) ON DUPLICATE KEY UPDATE b=lpad('b',11000,'c');
+
+--source include/show_binlog_events.inc
+
+let $binlog_pos_2= query_get_value("SHOW MASTER STATUS", Position, 1);
+
+--disable_query_log
+eval SELECT $binlog_pos_1 = $binlog_pos_2 as must_be_1;
+--enable_query_log
+
+SET innodb_fake_changes=default;
+
+DROP TABLE t1;
+DROP TABLE t2;
+DROP TABLE t3;

=== modified file 'Percona-Server/sql/handler.cc'
--- Percona-Server/sql/handler.cc	2013-06-03 03:53:55 +0000
+++ Percona-Server/sql/handler.cc	2013-08-22 12:56:44 +0000
@@ -1031,6 +1031,18 @@
   {
     if (ha_info->is_trx_read_write())
       ++rw_ha_count;
+    else
+    {
+      /*
+        If we have any fake changes handlertons, they will not be marked as
+        read-write, potentially skipping 2PC and causing the fake transaction
+        to be binlogged.  Force using 2PC in this case by bumping rw_ha_count
+        for each fake changes handlerton.
+      */
+      handlerton *ht= ha_info->ht();
+      if (unlikely(ht->is_fake_change && ht->is_fake_change(ht, thd)))
+        ++rw_ha_count;
+    }
 
     if (! all)
     {
@@ -1168,7 +1180,14 @@
           transaction is read-only. This allows for simpler
           implementation in engines that are always read-only.
         */
-        if (! ha_info->is_trx_read_write())
+        /*
+          But do call two-phase commit if the handlerton has fake changes
+          enabled even if it's not marked as read-write.  This will ensure that
+          the fake changes handlerton prepare will fail, preventing binlogging
+          and committing the transaction in other engines.
+        */
+        if (! ha_info->is_trx_read_write()
+            && !(ht->is_fake_change && ht->is_fake_change(ht, thd)))
           continue;
         /*
           Sic: we know that prepare() is not NULL since otherwise

=== modified file 'Percona-Server/sql/handler.h'
--- Percona-Server/sql/handler.h	2013-01-22 16:28:47 +0000
+++ Percona-Server/sql/handler.h	2013-08-22 12:56:44 +0000
@@ -695,6 +695,7 @@
                            class Item *cond);
    my_bool (*flush_changed_page_bitmaps)(void);
    my_bool (*purge_changed_page_bitmaps)(ulonglong lsn);
+   my_bool (*is_fake_change)(handlerton *hton, THD *thd);
    uint32 flags;                                /* global handler flags */
    /*
       Those handlerton functions below are properly initialized at handler

=== modified file 'Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc'
--- Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc	2013-08-05 13:11:52 +0000
+++ Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc	2013-08-22 12:56:44 +0000
@@ -335,6 +335,19 @@
 /*================================*/
 	ulonglong lsn);	/*!< in: LSN to purge files up to */
 
+
+/*****************************************************************//**
+Check whether this is a fake change transaction.
+@return TRUE if a fake change transaction */
+static
+my_bool
+innobase_is_fake_change(
+/*====================*/
+	handlerton	*hton,  /*!< in: InnoDB handlerton */
+	THD*		thd);	/*!< in: MySQL thread handle of the user for
+				  whom the transaction is being committed */
+
+
 static const char innobase_hton_name[]= "InnoDB";
 
 /*************************************************************//**
@@ -2353,6 +2366,7 @@
 		= innobase_flush_changed_page_bitmaps;
 	innobase_hton->purge_changed_page_bitmaps
 		= innobase_purge_changed_page_bitmaps;
+	innobase_hton->is_fake_change = innobase_is_fake_change;
 
 	ut_a(DATA_MYSQL_TRUE_VARCHAR == (ulint)MYSQL_TYPE_VARCHAR);
 
@@ -2962,6 +2976,23 @@
 	return (my_bool)log_online_purge_changed_page_bitmaps(lsn);
 }
 
+/*****************************************************************//**
+Check whether this is a fake change transaction.
+@return TRUE if a fake change transaction */
+static
+my_bool
+innobase_is_fake_change(
+/*====================*/
+	handlerton	*hton __attribute__((unused)),
+				/*!< in: InnoDB handlerton */
+	THD*		thd)	/*!< in: MySQL thread handle of the user for
+				whom the transaction is being committed */
+{
+	trx_t*	trx = check_trx_exists(thd);
+	return trx->fake_changes;
+}
+
+
 /****************************************************************//**
 Copy the current replication position from MySQL to a transaction. */
 static
@@ -10867,7 +10898,14 @@
 		return(0);
 	}
 
-	if (trx->fake_changes) {
+	if (UNIV_UNLIKELY(trx->fake_changes)) {
+
+		if (all || (!thd_test_options(thd, OPTION_NOT_AUTOCOMMIT
+					      | OPTION_BEGIN))) {
+
+			thd->main_da.reset_diagnostics_area();
+			return(HA_ERR_WRONG_COMMAND);
+		}
 		return(0);
 	}