Skip to content

Commit

Permalink
MDEV-33431 Latching order violation reported fil_system.sys_space.lat…
Browse files Browse the repository at this point in the history
…ch 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.
  • Loading branch information
mariadb-DebarunBanerjee committed Apr 17, 2024
1 parent f6e9600 commit 040069f
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 1 deletion.
32 changes: 32 additions & 0 deletions mysql-test/suite/innodb/r/change_buffer_free.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
SET @saved_change_buffering = @@GLOBAL.innodb_change_buffering;
SET @saved_file_per_table = @@GLOBAL.innodb_file_per_table;
SET @saved_change_buffering_debug = @@GLOBAL.innodb_change_buffering_debug;
SET GLOBAL innodb_change_buffering = NONE;
SET GLOBAL innodb_file_per_table = OFF;
CREATE TABLE t2(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
CREATE INDEX i1 ON t2 (c2);
INSERT INTO t2(c2) VALUES('mariadb');
INSERT INTO t2(c2) SELECT c2 FROM t2;
INSERT INTO t2(c2) SELECT c2 FROM t2;
INSERT INTO t2(c2) SELECT c2 FROM t2;
INSERT INTO t2(c2) SELECT c2 FROM t2;
INSERT INTO t2(c2) SELECT c2 FROM t2;
CREATE TABLE t1(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
CREATE INDEX i1 ON t1 (c2);
INSERT INTO t1(c2) VALUES('mariadb');
INSERT INTO t1(c2) SELECT c2 FROM t1;
INSERT INTO t1(c2) SELECT c2 FROM t1;
INSERT INTO t1(c2) SELECT c2 FROM t1;
INSERT INTO t1(c2) SELECT c2 FROM t1;
INSERT INTO t1(c2) SELECT c2 FROM t1;
SET GLOBAL innodb_change_buffering = all;
SET GLOBAL innodb_change_buffering_debug = 1;
SET DEBUG_DBUG='+d,ibuf_force_remove_free_page';
INSERT INTO t2(c2) SELECT c2 FROM t2 IGNORE INDEX (i1) LIMIT 4;
INSERT INTO t1(c2) SELECT c2 FROM t1 IGNORE INDEX (i1) LIMIT 4;
SET DEBUG_DBUG='-d,ibuf_force_remove_free_page';
SET GLOBAL innodb_change_buffering_debug = @saved_change_buffering_debug;
SET GLOBAL innodb_change_buffering = @saved_change_buffering;
DROP TABLE t2;
DROP TABLE t1;
SET GLOBAL innodb_file_per_table = @saved_file_per_table;
2 changes: 2 additions & 0 deletions mysql-test/suite/innodb/t/change_buffer_free.opt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--innodb-page-size=4k
--innodb_sync_debug=on
54 changes: 54 additions & 0 deletions mysql-test/suite/innodb/t/change_buffer_free.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#
# MDEV 33431: Latching order violation reported fil_system.sys_space.latch and ibuf_pessimistic_insert_mutex
#
--source include/have_innodb.inc
--source include/have_debug.inc
--source include/have_debug_sync.inc

SET @saved_change_buffering = @@GLOBAL.innodb_change_buffering;
SET @saved_file_per_table = @@GLOBAL.innodb_file_per_table;
SET @saved_change_buffering_debug = @@GLOBAL.innodb_change_buffering_debug;

SET GLOBAL innodb_change_buffering = NONE;
SET GLOBAL innodb_file_per_table = OFF;

let $loop=2;
while ($loop)
{
eval CREATE TABLE t$loop(c1 INT AUTO_INCREMENT PRIMARY KEY,c2 CHAR(100))ENGINE=InnoDB;
eval CREATE INDEX i1 ON t$loop (c2);

eval INSERT INTO t$loop(c2) VALUES('mariadb');
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop;

dec $loop;
}

SET GLOBAL innodb_change_buffering = all;
SET GLOBAL innodb_change_buffering_debug = 1;

SET DEBUG_DBUG='+d,ibuf_force_remove_free_page';
let $loop=2;

while ($loop)
{
eval INSERT INTO t$loop(c2) SELECT c2 FROM t$loop IGNORE INDEX (i1) LIMIT 4;
dec $loop;
}
SET DEBUG_DBUG='-d,ibuf_force_remove_free_page';

SET GLOBAL innodb_change_buffering_debug = @saved_change_buffering_debug;
SET GLOBAL innodb_change_buffering = @saved_change_buffering;

let $loop=2;
while ($loop)
{
eval DROP TABLE t$loop;
dec $loop;
}

SET GLOBAL innodb_file_per_table = @saved_file_per_table;
5 changes: 5 additions & 0 deletions storage/innobase/ibuf/ibuf0ibuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,11 @@ ibuf_free_excess_pages(void)
/* Free at most a few pages at a time, so that we do not delay the
requested service too much */

DBUG_EXECUTE_IF("ibuf_force_remove_free_page",
ibuf_remove_free_page();
return;
);

for (ulint i = 0; i < 4; i++) {

ibool too_much_free;
Expand Down
19 changes: 18 additions & 1 deletion storage/innobase/sync/sync0debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,24 @@ sync_latch_meta_init()

LATCH_ADD_MUTEX(IBUF, SYNC_IBUF_MUTEX, ibuf_mutex_key);

LATCH_ADD_MUTEX(IBUF_PESSIMISTIC_INSERT, SYNC_IBUF_PESS_INSERT_MUTEX,
/* 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
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. */
LATCH_ADD_MUTEX(IBUF_PESSIMISTIC_INSERT, SYNC_NO_ORDER_CHECK,
ibuf_pessimistic_insert_mutex_key);

LATCH_ADD_MUTEX(PURGE_SYS_PQ, SYNC_PURGE_QUEUE,
Expand Down

0 comments on commit 040069f

Please sign in to comment.