Merge lp:~laurynas-biveinis/percona-server/bug1188168-5.6 into lp:percona-server/5.6

Proposed by Laurynas Biveinis
Status: Merged
Approved by: Stewart Smith
Approved revision: no longer in the source branch.
Merged at revision: 480
Proposed branch: lp:~laurynas-biveinis/percona-server/bug1188168-5.6
Merge into: lp:percona-server/5.6
Prerequisite: lp:~laurynas-biveinis/percona-server/fake-changes-from-fb-5.6
Diff against target: 123 lines (+102/-1)
3 files modified
Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_1188168.result (+71/-0)
Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_1188168.test (+30/-0)
Percona-Server/storage/innobase/row/row0ins.cc (+1/-1)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug1188168-5.6
Reviewer Review Type Date Requested Status
Stewart Smith (community) Approve
Laurynas Biveinis Pending
Sergei Glushchenko g2 Pending
Review via email: mp+176875@code.launchpad.net

This proposal supersedes a proposal from 2013-07-17.

Description of the change

No BT or ST but blocks BT 20439.

2nd MP

http://jenkins.percona.com/job/percona-server-5.6-param/212/

Review comment addressed

1st MP

http://jenkins.percona.com/job/percona-server-5.6-param/199/

Mere bug 1188168 fix from 5.5.

To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote : Posted in a previous version of this proposal

This patch is missing branch prediction. Is it intentionally?

review: Needs Information (g2)
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

Yes, because it's (big_rec && !fake_changes) -> (big_rec && likely true) -> as likely as big_rec, and big_rec is neither likely nor unlikely.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote : Posted in a previous version of this proposal

Yes, you're right. But I mean, why not

if (big_rec && UNIV_LIKELY(!thr_get_trx(thr)->fake_changes)) ?

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote : Posted in a previous version of this proposal

Hmm, I didn't think of that, thanks

review: Needs Fixing
Revision history for this message
Stewart Smith (stewart) wrote :

(and yes, i looked at the patches themselves before approving)

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/r/percona_innodb_fake_changes_bug_1188168.result'
2--- Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_1188168.result 1970-01-01 00:00:00 +0000
3+++ Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_1188168.result 2013-10-17 09:12:51 +0000
4@@ -0,0 +1,71 @@
5+SET @@GLOBAL.userstat=ON;
6+SET @@GLOBAL.innodb_stats_transient_sample_pages=30000;
7+DROP TABLE IF EXISTS t1;
8+CREATE TABLE t1(id INT NOT NULL PRIMARY KEY, data TEXT) ENGINE=InnoDB;
9+INSERT INTO t1 VALUES(1, '');
10+INSERT INTO t1 VALUES(2, '');
11+INSERT INTO t1 VALUES(3, '');
12+INSERT INTO t1 VALUES(4, '');
13+DELETE FROM t1 WHERE id = 4;
14+SELECT @@global.userstat = 1 AS should_be_1;
15+should_be_1
16+1
17+SELECT @@global.innodb_stats_transient_sample_pages = 30000 AS should_be_1;
18+should_be_1
19+1
20+ANALYZE TABLE t1;
21+Table Op Msg_type Msg_text
22+test.t1 analyze status OK
23+SELECT VARIABLE_VALUE INTO @innodb_rows_inserted_1 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Innodb_rows_inserted';
24+SELECT VARIABLE_VALUE INTO @innodb_rows_deleted_1 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Innodb_rows_deleted';
25+SELECT VARIABLE_VALUE INTO @innodb_rows_updated_1 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Innodb_rows_updated';
26+SELECT NUM_ROWS INTO @table_rows_estimate_1 FROM INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS WHERE NAME LIKE 'test/t1';
27+SELECT ROWS_CHANGED INTO @table_rows_changed_1 FROM INFORMATION_SCHEMA.TABLE_STATISTICS WHERE TABLE_SCHEMA LIKE 'test' AND TABLE_NAME LIKE 't1';
28+SELECT ROWS_CHANGED_X_INDEXES INTO @table_rows_changed_x_indexes_1 FROM INFORMATION_SCHEMA.TABLE_STATISTICS WHERE TABLE_SCHEMA LIKE 'test' AND TABLE_NAME LIKE 't1';
29+SELECT MODIFIED_COUNTER INTO @table_dml_counter_1 FROM INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS WHERE NAME LIKE 'test/t1';
30+SHOW INDEXES IN t1;
31+Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment
32+t1 0 PRIMARY 1 id A 3 NULL NULL BTREE
33+SET innodb_fake_changes=1;
34+INSERT INTO t1 VALUES (4, LPAD('a', 12000, 'b'));
35+ERROR HY000: Got error 131 during COMMIT
36+SET innodb_fake_changes=0;
37+CHECK TABLE t1;
38+Table Op Msg_type Msg_text
39+test.t1 check status OK
40+SELECT VARIABLE_VALUE INTO @innodb_rows_inserted_2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'Innodb_rows_inserted';
41+SELECT VARIABLE_VALUE INTO @innodb_rows_deleted_2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='Innodb_rows_deleted';
42+SELECT VARIABLE_VALUE INTO @innodb_rows_updated_2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='Innodb_rows_updated';
43+SELECT NUM_ROWS INTO @table_rows_estimate_2 FROM INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS WHERE NAME LIKE 'test/t1';
44+SELECT ROWS_CHANGED INTO @table_rows_changed_2 FROM INFORMATION_SCHEMA.TABLE_STATISTICS WHERE TABLE_SCHEMA LIKE 'test' AND TABLE_NAME LIKE 't1';
45+SELECT ROWS_CHANGED_X_INDEXES INTO @table_rows_changed_x_indexes_2 FROM INFORMATION_SCHEMA.TABLE_STATISTICS WHERE TABLE_SCHEMA LIKE 'test' AND TABLE_NAME LIKE 't1';
46+SELECT MODIFIED_COUNTER INTO @table_dml_counter_2 FROM INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS WHERE NAME LIKE 'test/t1';
47+should_be_1
48+1
49+SELECT @innodb_rows_inserted_2 - @innodb_rows_inserted_1 AS should_be_0;
50+should_be_0
51+0
52+SELECT @innodb_rows_deleted_2 - @innodb_rows_deleted_1 AS should_be_0;
53+should_be_0
54+0
55+SELECT @innodb_rows_updated_2 - @innodb_rows_updated_1 AS should_be_0;
56+should_be_0
57+0
58+SELECT @table_rows_estimate_2 - @table_rows_estimate_1 AS should_be_0;
59+should_be_0
60+0
61+SELECT @table_rows_changed_2 - @table_rows_changed_1 AS should_be_0;
62+should_be_0
63+0
64+SELECT @table_rows_changed_x_indexes_2 - @table_rows_changed_x_indexes_1 AS should_be_0;
65+should_be_0
66+0
67+SELECT @table_dml_counter_2 - @table_dml_counter_1 AS should_be_0;
68+should_be_0
69+0
70+SHOW INDEXES IN t1;
71+Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment
72+t1 0 PRIMARY 1 id A 3 NULL NULL BTREE
73+DROP TABLE t1;
74+SET @@GLOBAL.userstat=default;
75+SET @@GLOBAL.innodb_stats_transient_sample_pages=default;
76
77=== added file 'Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_1188168.test'
78--- Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_1188168.test 1970-01-01 00:00:00 +0000
79+++ Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_1188168.test 2013-10-17 09:12:51 +0000
80@@ -0,0 +1,30 @@
81+--source include/have_innodb.inc
82+
83+SET @@GLOBAL.userstat=ON;
84+SET @@GLOBAL.innodb_stats_transient_sample_pages=30000;
85+
86+--disable_warnings
87+DROP TABLE IF EXISTS t1;
88+--enable_warnings
89+
90+CREATE TABLE t1(id INT NOT NULL PRIMARY KEY, data TEXT) ENGINE=InnoDB;
91+
92+INSERT INTO t1 VALUES(1, '');
93+INSERT INTO t1 VALUES(2, '');
94+INSERT INTO t1 VALUES(3, '');
95+INSERT INTO t1 VALUES(4, '');
96+
97+DELETE FROM t1 WHERE id = 4;
98+
99+--let $fake_changes_table=t1
100+--source include/start_fake_changes.inc
101+
102+--error ER_ERROR_DURING_COMMIT
103+INSERT INTO t1 VALUES (4, LPAD('a', 12000, 'b'));
104+
105+--source include/stop_fake_changes.inc
106+
107+DROP TABLE t1;
108+
109+SET @@GLOBAL.userstat=default;
110+SET @@GLOBAL.innodb_stats_transient_sample_pages=default;
111
112=== modified file 'Percona-Server/storage/innobase/row/row0ins.cc'
113--- Percona-Server/storage/innobase/row/row0ins.cc 2013-10-17 08:00:06 +0000
114+++ Percona-Server/storage/innobase/row/row0ins.cc 2013-10-17 09:12:51 +0000
115@@ -2430,7 +2430,7 @@
116
117 rec_t* rec = btr_cur_get_rec(&cursor);
118
119- if (big_rec) {
120+ if (big_rec && UNIV_LIKELY(!thr_get_trx(thr)->fake_changes)) {
121 ut_a(err == DB_SUCCESS);
122 /* Write out the externally stored
123 columns while still x-latching

Subscribers

People subscribed via source and target branches