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

Proposed by Laurynas Biveinis on 2013-08-23
Status: Merged
Approved by: Alexey Kopytov on 2013-08-23
Approved revision: 578
Merged at revision: 587
Proposed branch: lp:~laurynas-biveinis/percona-server/fake-changes-binlog-5.1
Merge into: lp:percona-server/5.1
Diff against target: 309 lines (+238/-2)
5 files modified
Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result (+72/-0)
Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test (+106/-0)
Percona-Server/sql/handler.cc (+20/-1)
Percona-Server/sql/handler.h (+1/-0)
Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc (+39/-1)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/fake-changes-binlog-5.1
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2013-08-23 Approve on 2013-08-23
Vlad Lesin g2 2013-08-23 Pending
Review via email: mp+181710@code.launchpad.net

This proposal supersedes a proposal from 2013-06-21.

Description of the change

2nd MP:

- Moved treating of fake changes handlertons as read-write to ha_check_and_coalesce_trx_read_only().
- Minor comment formatting fix.
- Rebased on the current GCA.

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

1st MP:

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/

To post a comment you must log in.
Vlad Lesin (vlad-lesin) : Posted in a previous version of this proposal
review: Approve (g2)
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

This is a good suggestion, I will implement this.

Alexey Kopytov (akopytov) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result'
2--- Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result 1970-01-01 00:00:00 +0000
3+++ Percona-Server/mysql-test/suite/binlog/r/binlog_percona_fake_changes.result 2013-08-23 05:49:02 +0000
4@@ -0,0 +1,72 @@
5+DROP TABLE IF EXISTS t1;
6+DROP TABLE IF EXISTS t2;
7+DROP TABLE IF EXISTS t3;
8+CREATE TABLE t1 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
9+INSERT INTO t1 VALUES (1,1);
10+CREATE TABLE t2 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
11+CREATE INDEX bx ON t2(b);
12+INSERT INTO t2 VALUES (1,1);
13+CREATE TABLE t3 (a INT PRIMARY KEY, b text) ENGINE=InnoDB;
14+INSERT INTO t3 VALUES (1,'');
15+RESET MASTER;
16+SET innodb_fake_changes=1;
17+SELECT * FROM t1;
18+ERROR HY000: Got error 131 during COMMIT
19+SELECT * FROM t2;
20+ERROR HY000: Got error 131 during COMMIT
21+SELECT * FROM t3;
22+ERROR HY000: Got error 131 during COMMIT
23+INSERT INTO t1 VALUES (2,2);
24+ERROR HY000: Got error 131 during COMMIT
25+INSERT INTO t2 VALUES (2,2);
26+ERROR HY000: Got error 131 during COMMIT
27+INSERT INTO t3 VALUES (2,lpad('a',10000, 'b'));
28+ERROR HY000: Got error 131 during COMMIT
29+UPDATE t1 SET a=0 where b=1;
30+ERROR HY000: Got error 131 during COMMIT
31+UPDATE t2 SET a=0 where b=1;
32+ERROR HY000: Got error 131 during COMMIT
33+UPDATE t3 SET a=0 where a=1;
34+ERROR HY000: Got error 131 during COMMIT
35+UPDATE t1 SET b=0 where a=1;
36+ERROR HY000: Got error 131 during COMMIT
37+UPDATE t2 SET b=0 where a=1;
38+ERROR HY000: Got error 131 during COMMIT
39+UPDATE t2 SET b=lpad('a',10000, 'z') where a=1;
40+ERROR HY000: Got error 131 during COMMIT
41+UPDATE t1 SET b=0 where a=2;
42+ERROR HY000: Got error 131 during COMMIT
43+UPDATE t2 SET b=0 where a=2;
44+ERROR HY000: Got error 131 during COMMIT
45+UPDATE t2 SET b=lpad('a',10000, 'z') where a=2;
46+ERROR HY000: Got error 131 during COMMIT
47+DELETE FROM t1 where b=2;
48+ERROR HY000: Got error 131 during COMMIT
49+DELETE FROM t2 where b=2;
50+ERROR HY000: Got error 131 during COMMIT
51+DELETE FROM t1 where a=2;
52+ERROR HY000: Got error 131 during COMMIT
53+DELETE FROM t2 where a=2;
54+ERROR HY000: Got error 131 during COMMIT
55+DELETE FROM t3 where a=2;
56+ERROR HY000: Got error 131 during COMMIT
57+REPLACE INTO t1 values (2,3);
58+ERROR HY000: Got error 131 during COMMIT
59+REPLACE INTO t2 values (2,3);
60+ERROR HY000: Got error 131 during COMMIT
61+REPLACE INTO t3 values (2,lpad('a',9000,'q'));
62+ERROR HY000: Got error 131 during COMMIT
63+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
64+ERROR HY000: Got error 131 during COMMIT
65+INSERT INTO t2 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
66+ERROR HY000: Got error 131 during COMMIT
67+INSERT INTO t3 VALUES (1,1) ON DUPLICATE KEY UPDATE b=lpad('b',11000,'c');
68+ERROR HY000: Got error 131 during COMMIT
69+show binlog events from <binlog_start>;
70+Log_name Pos Event_type Server_id End_log_pos Info
71+must_be_1
72+1
73+SET innodb_fake_changes=default;
74+DROP TABLE t1;
75+DROP TABLE t2;
76+DROP TABLE t3;
77
78=== added file 'Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test'
79--- Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test 1970-01-01 00:00:00 +0000
80+++ Percona-Server/mysql-test/suite/binlog/t/binlog_percona_fake_changes.test 2013-08-23 05:49:02 +0000
81@@ -0,0 +1,106 @@
82+#
83+# Test that fake changes are not written to binlog
84+# Based on rpl_percona_fake_changes.test in Facebook patch
85+# https://bugs.launchpad.net/percona-server/+bug/1190580
86+# https://bugs.launchpad.net/percona-server/+bug/1188162
87+#
88+
89+--source include/have_log_bin.inc
90+--source include/have_innodb_plugin.inc
91+
92+--disable_warnings
93+DROP TABLE IF EXISTS t1;
94+DROP TABLE IF EXISTS t2;
95+DROP TABLE IF EXISTS t3;
96+--enable_warnings
97+
98+CREATE TABLE t1 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
99+INSERT INTO t1 VALUES (1,1);
100+
101+CREATE TABLE t2 (a INT PRIMARY KEY, b int) ENGINE=InnoDB;
102+CREATE INDEX bx ON t2(b);
103+INSERT INTO t2 VALUES (1,1);
104+
105+CREATE TABLE t3 (a INT PRIMARY KEY, b text) ENGINE=InnoDB;
106+INSERT INTO t3 VALUES (1,'');
107+
108+RESET MASTER;
109+
110+let $binlog_pos_1= query_get_value("SHOW MASTER STATUS", Position, 1);
111+
112+SET innodb_fake_changes=1;
113+
114+--error ER_ERROR_DURING_COMMIT
115+SELECT * FROM t1;
116+--error ER_ERROR_DURING_COMMIT
117+SELECT * FROM t2;
118+--error ER_ERROR_DURING_COMMIT
119+SELECT * FROM t3;
120+
121+--error ER_ERROR_DURING_COMMIT
122+INSERT INTO t1 VALUES (2,2);
123+--error ER_ERROR_DURING_COMMIT
124+INSERT INTO t2 VALUES (2,2);
125+--error ER_ERROR_DURING_COMMIT
126+INSERT INTO t3 VALUES (2,lpad('a',10000, 'b'));
127+
128+--error ER_ERROR_DURING_COMMIT
129+UPDATE t1 SET a=0 where b=1;
130+--error ER_ERROR_DURING_COMMIT
131+UPDATE t2 SET a=0 where b=1;
132+--error ER_ERROR_DURING_COMMIT
133+UPDATE t3 SET a=0 where a=1;
134+
135+--error ER_ERROR_DURING_COMMIT
136+UPDATE t1 SET b=0 where a=1;
137+--error ER_ERROR_DURING_COMMIT
138+UPDATE t2 SET b=0 where a=1;
139+--error ER_ERROR_DURING_COMMIT
140+UPDATE t2 SET b=lpad('a',10000, 'z') where a=1;
141+
142+--error ER_ERROR_DURING_COMMIT
143+UPDATE t1 SET b=0 where a=2;
144+--error ER_ERROR_DURING_COMMIT
145+UPDATE t2 SET b=0 where a=2;
146+--error ER_ERROR_DURING_COMMIT
147+UPDATE t2 SET b=lpad('a',10000, 'z') where a=2;
148+
149+--error ER_ERROR_DURING_COMMIT
150+DELETE FROM t1 where b=2;
151+--error ER_ERROR_DURING_COMMIT
152+DELETE FROM t2 where b=2;
153+
154+--error ER_ERROR_DURING_COMMIT
155+DELETE FROM t1 where a=2;
156+--error ER_ERROR_DURING_COMMIT
157+DELETE FROM t2 where a=2;
158+--error ER_ERROR_DURING_COMMIT
159+DELETE FROM t3 where a=2;
160+
161+--error ER_ERROR_DURING_COMMIT
162+REPLACE INTO t1 values (2,3);
163+--error ER_ERROR_DURING_COMMIT
164+REPLACE INTO t2 values (2,3);
165+--error ER_ERROR_DURING_COMMIT
166+REPLACE INTO t3 values (2,lpad('a',9000,'q'));
167+
168+--error ER_ERROR_DURING_COMMIT
169+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
170+--error ER_ERROR_DURING_COMMIT
171+INSERT INTO t2 VALUES (1,1) ON DUPLICATE KEY UPDATE b=2;
172+--error ER_ERROR_DURING_COMMIT
173+INSERT INTO t3 VALUES (1,1) ON DUPLICATE KEY UPDATE b=lpad('b',11000,'c');
174+
175+--source include/show_binlog_events.inc
176+
177+let $binlog_pos_2= query_get_value("SHOW MASTER STATUS", Position, 1);
178+
179+--disable_query_log
180+eval SELECT $binlog_pos_1 = $binlog_pos_2 as must_be_1;
181+--enable_query_log
182+
183+SET innodb_fake_changes=default;
184+
185+DROP TABLE t1;
186+DROP TABLE t2;
187+DROP TABLE t3;
188
189=== modified file 'Percona-Server/sql/handler.cc'
190--- Percona-Server/sql/handler.cc 2013-06-03 03:53:55 +0000
191+++ Percona-Server/sql/handler.cc 2013-08-23 05:49:02 +0000
192@@ -1031,6 +1031,18 @@
193 {
194 if (ha_info->is_trx_read_write())
195 ++rw_ha_count;
196+ else
197+ {
198+ /*
199+ If we have any fake changes handlertons, they will not be marked as
200+ read-write, potentially skipping 2PC and causing the fake transaction
201+ to be binlogged. Force using 2PC in this case by bumping rw_ha_count
202+ for each fake changes handlerton.
203+ */
204+ handlerton *ht= ha_info->ht();
205+ if (unlikely(ht->is_fake_change && ht->is_fake_change(ht, thd)))
206+ ++rw_ha_count;
207+ }
208
209 if (! all)
210 {
211@@ -1168,7 +1180,14 @@
212 transaction is read-only. This allows for simpler
213 implementation in engines that are always read-only.
214 */
215- if (! ha_info->is_trx_read_write())
216+ /*
217+ But do call two-phase commit if the handlerton has fake changes
218+ enabled even if it's not marked as read-write. This will ensure that
219+ the fake changes handlerton prepare will fail, preventing binlogging
220+ and committing the transaction in other engines.
221+ */
222+ if (! ha_info->is_trx_read_write()
223+ && !(ht->is_fake_change && ht->is_fake_change(ht, thd)))
224 continue;
225 /*
226 Sic: we know that prepare() is not NULL since otherwise
227
228=== modified file 'Percona-Server/sql/handler.h'
229--- Percona-Server/sql/handler.h 2013-01-22 16:28:47 +0000
230+++ Percona-Server/sql/handler.h 2013-08-23 05:49:02 +0000
231@@ -695,6 +695,7 @@
232 class Item *cond);
233 my_bool (*flush_changed_page_bitmaps)(void);
234 my_bool (*purge_changed_page_bitmaps)(ulonglong lsn);
235+ my_bool (*is_fake_change)(handlerton *hton, THD *thd);
236 uint32 flags; /* global handler flags */
237 /*
238 Those handlerton functions below are properly initialized at handler
239
240=== modified file 'Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc'
241--- Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2013-08-05 13:11:52 +0000
242+++ Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2013-08-23 05:49:02 +0000
243@@ -335,6 +335,19 @@
244 /*================================*/
245 ulonglong lsn); /*!< in: LSN to purge files up to */
246
247+
248+/*****************************************************************//**
249+Check whether this is a fake change transaction.
250+@return TRUE if a fake change transaction */
251+static
252+my_bool
253+innobase_is_fake_change(
254+/*====================*/
255+ handlerton *hton, /*!< in: InnoDB handlerton */
256+ THD* thd); /*!< in: MySQL thread handle of the user for
257+ whom the transaction is being committed */
258+
259+
260 static const char innobase_hton_name[]= "InnoDB";
261
262 /*************************************************************//**
263@@ -2353,6 +2366,7 @@
264 = innobase_flush_changed_page_bitmaps;
265 innobase_hton->purge_changed_page_bitmaps
266 = innobase_purge_changed_page_bitmaps;
267+ innobase_hton->is_fake_change = innobase_is_fake_change;
268
269 ut_a(DATA_MYSQL_TRUE_VARCHAR == (ulint)MYSQL_TYPE_VARCHAR);
270
271@@ -2962,6 +2976,23 @@
272 return (my_bool)log_online_purge_changed_page_bitmaps(lsn);
273 }
274
275+/*****************************************************************//**
276+Check whether this is a fake change transaction.
277+@return TRUE if a fake change transaction */
278+static
279+my_bool
280+innobase_is_fake_change(
281+/*====================*/
282+ handlerton *hton __attribute__((unused)),
283+ /*!< in: InnoDB handlerton */
284+ THD* thd) /*!< in: MySQL thread handle of the user for
285+ whom the transaction is being committed */
286+{
287+ trx_t* trx = check_trx_exists(thd);
288+ return trx->fake_changes;
289+}
290+
291+
292 /****************************************************************//**
293 Copy the current replication position from MySQL to a transaction. */
294 static
295@@ -10867,7 +10898,14 @@
296 return(0);
297 }
298
299- if (trx->fake_changes) {
300+ if (UNIV_UNLIKELY(trx->fake_changes)) {
301+
302+ if (all || (!thd_test_options(thd, OPTION_NOT_AUTOCOMMIT
303+ | OPTION_BEGIN))) {
304+
305+ thd->main_da.reset_diagnostics_area();
306+ return(HA_ERR_WRONG_COMMAND);
307+ }
308 return(0);
309 }
310

Subscribers

People subscribed via source and target branches