Skip to content

Commit 7543684

Browse files
mariadb-DenisProtivenskymariadb-DenisProtivensky
committed
MDEV-37229: Set proper trx isolation level for Wsrep system threads
Every Wsrep system thread should run with READ_COMMITTED transaction isolation level to prevent issues caused by InnoDB gap locks. The exception is statement-based replication for appliers, where REPEATABLE_READ is required by the server code. To account for that, set the isolation level before every applied event. It won't affect an already running transaction, but allows to handle both statement- and row-based replications accordingly. However, the problem might arise with the mixed replication format. Apart from that, there was a separate issue with applier thread vars: wsrep_plugins_post_init() would overwrite thd->variables for every applier thread and forget to restore proper default isolation level. Then, upon every server transaction termination, trans_reset_one_shot_statistics() would set thread's isolation level to the one stored in thd->variables, thus spoiling the isolation level value for appliers.
1 parent fb49d8a commit 7543684

File tree

9 files changed

+167
-1
lines changed

9 files changed

+167
-1
lines changed

‎mysql-test/suite/galera/r/MDEV-37229.result‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
connection node_2;
2+
connection node_1;
3+
connection node_2;
4+
connection node_1;
5+
CREATE TABLE t1 (i INT NOT NULL PRIMARY KEY AUTO_INCREMENT) ENGINE=InnoDB;
6+
SET GLOBAL wsrep_forced_binlog_format='STATEMENT';
7+
START TRANSACTION;
8+
INSERT INTO t1(i) VALUES(NULL);
9+
Warnings:
10+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
11+
INSERT INTO t1(i) VALUES(NULL);
12+
Warnings:
13+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
14+
SET GLOBAL wsrep_forced_binlog_format='ROW';
15+
INSERT INTO t1(i) VALUES(NULL);
16+
INSERT INTO t1(i) VALUES(NULL);
17+
COMMIT;
18+
SET GLOBAL wsrep_forced_binlog_format='ROW';
19+
START TRANSACTION;
20+
INSERT INTO t1(i) VALUES(NULL);
21+
INSERT INTO t1(i) VALUES(NULL);
22+
SET GLOBAL wsrep_forced_binlog_format='STATEMENT';
23+
INSERT INTO t1(i) VALUES(NULL);
24+
Warnings:
25+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
26+
INSERT INTO t1(i) VALUES(NULL);
27+
Warnings:
28+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
29+
COMMIT;
30+
SELECT * FROM t1 ORDER BY i;
31+
i
32+
1
33+
3
34+
5
35+
7
36+
9
37+
11
38+
13
39+
15
40+
connection node_2;
41+
SELECT * FROM t1 ORDER BY i;
42+
i
43+
1
44+
3
45+
5
46+
7
47+
9
48+
11
49+
13
50+
15
51+
connection node_1;
52+
SET GLOBAL wsrep_forced_binlog_format='NONE';
53+
DROP TABLE t1;

‎mysql-test/suite/galera/r/galera_binlog_stmt_autoinc.result‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,28 @@ i c
3333
3 dummy_text
3434
5 dummy_text
3535
7 dummy_text
36+
insert into t1(i) values(null);
37+
Warnings:
38+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
39+
insert into t1(i) values(null);
40+
Warnings:
41+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
42+
insert into t1(i) values(null);
43+
Warnings:
44+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
45+
insert into t1(i) values(null);
46+
Warnings:
47+
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
48+
select * from t1 order by i;
49+
i c
50+
1 dummy_text
51+
3 dummy_text
52+
5 dummy_text
53+
7 dummy_text
54+
9 dummy_text
55+
11 dummy_text
56+
13 dummy_text
57+
15 dummy_text
3658
connection node_2;
3759
show variables like 'auto_increment%';
3860
Variable_name Value
@@ -44,6 +66,10 @@ i c
4466
3 dummy_text
4567
5 dummy_text
4668
7 dummy_text
69+
9 dummy_text
70+
11 dummy_text
71+
13 dummy_text
72+
15 dummy_text
4773
SET GLOBAL wsrep_forced_binlog_format='none';
4874
connection node_1;
4975
SET GLOBAL wsrep_forced_binlog_format='none';

‎mysql-test/suite/galera/t/MDEV-37229.test‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
##
2+
## Test that mixed binlog replication works at the applier side.
3+
##
4+
5+
--source include/galera_cluster.inc
6+
--source include/have_innodb.inc
7+
8+
--connection node_2
9+
--disable_query_log
10+
call mtr.add_suppression("Unsafe statement written to the binary log");
11+
--enable_query_log
12+
13+
--connection node_1
14+
--disable_query_log
15+
call mtr.add_suppression("Unsafe statement written to the binary log");
16+
--enable_query_log
17+
18+
CREATE TABLE t1 (i INT NOT NULL PRIMARY KEY AUTO_INCREMENT) ENGINE=InnoDB;
19+
20+
# Try mixed replication: statement then row.
21+
SET GLOBAL wsrep_forced_binlog_format='STATEMENT';
22+
START TRANSACTION;
23+
INSERT INTO t1(i) VALUES(NULL);
24+
INSERT INTO t1(i) VALUES(NULL);
25+
SET GLOBAL wsrep_forced_binlog_format='ROW';
26+
INSERT INTO t1(i) VALUES(NULL);
27+
INSERT INTO t1(i) VALUES(NULL);
28+
COMMIT;
29+
30+
# Try mixed replication: row then statement.
31+
SET GLOBAL wsrep_forced_binlog_format='ROW';
32+
START TRANSACTION;
33+
INSERT INTO t1(i) VALUES(NULL);
34+
INSERT INTO t1(i) VALUES(NULL);
35+
SET GLOBAL wsrep_forced_binlog_format='STATEMENT';
36+
INSERT INTO t1(i) VALUES(NULL);
37+
INSERT INTO t1(i) VALUES(NULL);
38+
COMMIT;
39+
40+
SELECT * FROM t1 ORDER BY i;
41+
42+
--connection node_2
43+
--let $wait_condition = SELECT COUNT(*) = 8 FROM t1;
44+
--source include/wait_condition.inc
45+
SELECT * FROM t1 ORDER BY i;
46+
47+
--connection node_1
48+
SET GLOBAL wsrep_forced_binlog_format='NONE';
49+
DROP TABLE t1;

‎mysql-test/suite/galera/t/galera_binlog_stmt_autoinc.cnf‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@ auto_increment_offset=1
55
auto_increment_increment=2
66

77
[mysqld.2]
8+
wsrep_slave_threads=2
89
auto_increment_offset=2
910
auto_increment_increment=2

‎mysql-test/suite/galera/t/galera_binlog_stmt_autoinc.test‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,17 @@ insert into t1(i) values(null), (null), (null);
4848

4949
select * from t1 order by i;
5050

51+
# Ensure both appliers have some work to perform in parallel.
52+
# This way we can see they both have proper isolation level set.
53+
insert into t1(i) values(null);
54+
insert into t1(i) values(null);
55+
insert into t1(i) values(null);
56+
insert into t1(i) values(null);
57+
58+
select * from t1 order by i;
59+
5160
--connection node_2
52-
--let $wait_condition = SELECT COUNT(*) = 4 FROM t1;
61+
--let $wait_condition = SELECT COUNT(*) = 8 FROM t1;
5362
--source include/wait_condition.inc
5463
show variables like 'auto_increment%';
5564
select * from t1 order by i;

‎sql/sql_plugin.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4545,6 +4545,8 @@ my_bool post_init_callback(THD *thd, void *)
45454545

45464546
// Restore option_bits
45474547
thd->variables.option_bits= option_bits_saved;
4548+
// Restore proper default isolation level for appliers
4549+
thd->variables.tx_isolation= ISO_READ_COMMITTED;
45484550
}
45494551
set_current_thd(0);
45504552
return 0;

‎sql/wsrep_applier.cc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,30 @@ int wsrep_apply_events(THD* thd,
279279
}
280280
}
281281

282+
/* Statement-based replication requires InnoDB repeatable read
283+
transaction isolation level or higher.
284+
The isolation level will be reset to the default upon
285+
transaction termination.
286+
Although the isolation level can only be set on a newly
287+
started server transaction, it cannot be determined until
288+
we see the first applied binlog event.
289+
290+
Even though the isolation level is changed for every next
291+
applied event, it won't affect the overall setting for a
292+
running transaction.
293+
This should create an issue with the mixed replication mode
294+
as applying query and row events should be performed with
295+
different isolation levels, but now it doesn't surface.
296+
*/
297+
if (LOG_EVENT_IS_QUERY(typ))
298+
{
299+
thd->tx_isolation= ISO_REPEATABLE_READ;
300+
}
301+
else
302+
{
303+
thd->tx_isolation= ISO_READ_COMMITTED;
304+
}
305+
282306
if (LOG_EVENT_IS_WRITE_ROW(typ) ||
283307
LOG_EVENT_IS_UPDATE_ROW(typ) ||
284308
LOG_EVENT_IS_DELETE_ROW(typ))

‎sql/wsrep_schema.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,7 @@ static void wsrep_init_thd_for_schema(THD *thd)
732732
/* No general log */
733733
thd->variables.option_bits|= OPTION_LOG_OFF;
734734
/* Read committed isolation to avoid gap locking */
735+
thd->tx_isolation= ISO_READ_COMMITTED;
735736
thd->variables.tx_isolation= ISO_READ_COMMITTED;
736737
wsrep_assign_from_threadvars(thd);
737738
wsrep_store_threadvars(thd);

‎sql/wsrep_storage_service.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ Wsrep_storage_service::Wsrep_storage_service(THD* thd)
5959
thd->variables.option_bits |= OPTION_LOG_OFF;
6060

6161
/* Read committed isolation to avoid gap locking */
62+
thd->tx_isolation = ISO_READ_COMMITTED;
6263
thd->variables.tx_isolation = ISO_READ_COMMITTED;
6364

6465
/* Keep wsrep on to enter commit ordering hooks */

0 commit comments

Comments
 (0)