Skip to content

Commit 040069f

Browse files
mariadb-DebarunBanerjeemariadb-DebarunBanerjee
committed
MDEV-33431 Latching order violation reported fil_system.sys_space.latch and ibuf_pessimistic_insert_mutex
Issue: ------ The actual order of acquisition of the IBUF pessimistic insert mutex (SYNC_IBUF_PESS_INSERT_MUTEX) and IBUF header page latch (SYNC_IBUF_HEADER) w.r.t space latch (SYNC_FSP) differs from the order defined in sync0types.h. It was not discovered earlier as the path to ibuf_remove_free_page was not covered by the mtr test. Ideal order and one defined in sync0types.h is as follows. SYNC_IBUF_HEADER -> SYNC_IBUF_PESS_INSERT_MUTEX -> SYNC_FSP In ibuf_remove_free_page, we acquire space latch earlier and we have the order as follows resulting in the assert with innodb_sync_debug=on. SYNC_FSP -> SYNC_IBUF_HEADER -> SYNC_IBUF_PESS_INSERT_MUTEX Fix: --- We do maintain this order in other places and there doesn't seem to be any real issue here. To reduce impact in GA versions, we avoid doing extensive changes in mutex ordering to match the current SYNC_IBUF_PESS_INSERT_MUTEX order. Instead we relax the ordering check for IBUF pessimistic insert mutex using SYNC_NO_ORDER_CHECK.
1 parent f6e9600 commit 040069f

File tree

5 files changed

+111
-1
lines changed

5 files changed

+111
-1
lines changed

‎mysql-test/suite/innodb/r/change_buffer_free.result‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
SET @saved_change_buffering = @@GLOBAL.innodb_change_buffering;
2+
SET @saved_file_per_table = @@GLOBAL.innodb_file_per_table;
3+
SET @saved_change_buffering_debug = @@GLOBAL.innodb_change_buffering_debug;
4+
SET GLOBAL innodb_change_buffering = NONE;
5+
SET GLOBAL innodb_file_per_table = OFF;
6+
CREATE TABLE t2(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
7+
CREATE INDEX i1 ON t2 (c2);
8+
INSERT INTO t2(c2) VALUES('mariadb');
9+
INSERT INTO t2(c2) SELECT c2 FROM t2;
10+
INSERT INTO t2(c2) SELECT c2 FROM t2;
11+
INSERT INTO t2(c2) SELECT c2 FROM t2;
12+
INSERT INTO t2(c2) SELECT c2 FROM t2;
13+
INSERT INTO t2(c2) SELECT c2 FROM t2;
14+
CREATE TABLE t1(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
15+
CREATE INDEX i1 ON t1 (c2);
16+
INSERT INTO t1(c2) VALUES('mariadb');
17+
INSERT INTO t1(c2) SELECT c2 FROM t1;
18+
INSERT INTO t1(c2) SELECT c2 FROM t1;
19+
INSERT INTO t1(c2) SELECT c2 FROM t1;
20+
INSERT INTO t1(c2) SELECT c2 FROM t1;
21+
INSERT INTO t1(c2) SELECT c2 FROM t1;
22+
SET GLOBAL innodb_change_buffering = all;
23+
SET GLOBAL innodb_change_buffering_debug = 1;
24+
SET DEBUG_DBUG='+d,ibuf_force_remove_free_page';
25+
INSERT INTO t2(c2) SELECT c2 FROM t2 IGNORE INDEX (i1) LIMIT 4;
26+
INSERT INTO t1(c2) SELECT c2 FROM t1 IGNORE INDEX (i1) LIMIT 4;
27+
SET DEBUG_DBUG='-d,ibuf_force_remove_free_page';
28+
SET GLOBAL innodb_change_buffering_debug = @saved_change_buffering_debug;
29+
SET GLOBAL innodb_change_buffering = @saved_change_buffering;
30+
DROP TABLE t2;
31+
DROP TABLE t1;
32+
SET GLOBAL innodb_file_per_table = @saved_file_per_table;

‎mysql-test/suite/innodb/t/change_buffer_free.opt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
--innodb-page-size=4k
2+
--innodb_sync_debug=on

‎mysql-test/suite/innodb/t/change_buffer_free.test‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#
2+
# MDEV 33431: Latching order violation reported fil_system.sys_space.latch and ibuf_pessimistic_insert_mutex
3+
#
4+
--source include/have_innodb.inc
5+
--source include/have_debug.inc
6+
--source include/have_debug_sync.inc
7+
8+
SET @saved_change_buffering = @@GLOBAL.innodb_change_buffering;
9+
SET @saved_file_per_table = @@GLOBAL.innodb_file_per_table;
10+
SET @saved_change_buffering_debug = @@GLOBAL.innodb_change_buffering_debug;
11+
12+
SET GLOBAL innodb_change_buffering = NONE;
13+
SET GLOBAL innodb_file_per_table = OFF;
14+
15+
let $loop=2;
16+
while ($loop)
17+
{
18+
eval CREATE TABLE t$loop(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
19+
eval CREATE INDEX i1 ON t$loop (c2);
20+
21+
eval INSERT INTO t$loop(c2) VALUES('mariadb');
22+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
23+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
24+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
25+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
26+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
27+
28+
dec $loop;
29+
}
30+
31+
SET GLOBAL innodb_change_buffering = all;
32+
SET GLOBAL innodb_change_buffering_debug = 1;
33+
34+
SET DEBUG_DBUG='+d,ibuf_force_remove_free_page';
35+
let $loop=2;
36+
37+
while ($loop)
38+
{
39+
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop IGNORE INDEX (i1) LIMIT 4;
40+
dec $loop;
41+
}
42+
SET DEBUG_DBUG='-d,ibuf_force_remove_free_page';
43+
44+
SET GLOBAL innodb_change_buffering_debug = @saved_change_buffering_debug;
45+
SET GLOBAL innodb_change_buffering = @saved_change_buffering;
46+
47+
let $loop=2;
48+
while ($loop)
49+
{
50+
eval DROP TABLE t$loop;
51+
dec $loop;
52+
}
53+
54+
SET GLOBAL innodb_file_per_table = @saved_file_per_table;

‎storage/innobase/ibuf/ibuf0ibuf.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,6 +2011,11 @@ ibuf_free_excess_pages(void)
20112011
/* Free at most a few pages at a time, so that we do not delay the
20122012
requested service too much */
20132013

2014+
DBUG_EXECUTE_IF("ibuf_force_remove_free_page",
2015+
ibuf_remove_free_page();
2016+
return;
2017+
);
2018+
20142019
for (ulint i = 0; i < 4; i++) {
20152020

20162021
ibool too_much_free;

‎storage/innobase/sync/sync0debug.cc‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,24 @@ sync_latch_meta_init()
12081208

12091209
LATCH_ADD_MUTEX(IBUF, SYNC_IBUF_MUTEX, ibuf_mutex_key);
12101210

1211-
LATCH_ADD_MUTEX(IBUF_PESSIMISTIC_INSERT, SYNC_IBUF_PESS_INSERT_MUTEX,
1211+
/* The actual order of acquisition of the IBUF pessimistic insert mutex
1212+
(SYNC_IBUF_PESS_INSERT_MUTEX) and IBUF header page latch
1213+
(SYNC_IBUF_HEADER) w.r.t space latch (SYNC_FSP) differs from the order
1214+
defined in sync0types.h. It was not discovered earlier as the path to
1215+
ibuf_remove_free_page was not covered by the mtr test. Ideal order and
1216+
one defined in sync0types.h is as follows.
1217+
SYNC_IBUF_HEADER -> SYNC_IBUF_PESS_INSERT_MUTEX -> SYNC_FSP
1218+
1219+
In ibuf_remove_free_page, we acquire space latch earlier and we have
1220+
the order as follows resulting in the assert with innodb_sync_debug=on.
1221+
SYNC_FSP -> SYNC_IBUF_HEADER -> SYNC_IBUF_PESS_INSERT_MUTEX
1222+
1223+
We do maintain this order in other places and there doesn't seem to be
1224+
any real issue here. To reduce impact in GA versions, we avoid doing
1225+
extensive changes in mutex ordering to match the current
1226+
SYNC_IBUF_PESS_INSERT_MUTEX order. Instead we relax the ordering check
1227+
for IBUF pessimistic insert mutex using SYNC_NO_ORDER_CHECK. */
1228+
LATCH_ADD_MUTEX(IBUF_PESSIMISTIC_INSERT, SYNC_NO_ORDER_CHECK,
12121229
ibuf_pessimistic_insert_mutex_key);
12131230

12141231
LATCH_ADD_MUTEX(PURGE_SYS_PQ, SYNC_PURGE_QUEUE,

0 commit comments

Comments
 (0)