MDEV-13637 InnoDB change buffer housekeeping can cause redo log overrun and possibly deadlocks

The function ibuf_remove_free_page() may be called while the caller
is holding several mutexes or rw-locks. Because of this, this
housekeeping loop may cause performance glitches for operations that
involve tables that are stored in the InnoDB system tablespace.
Also deadlocks might be possible.

The worst impact of all is that due to the mutexes being held, calls to
log_free_check() had to be skipped during this housekeeping.
This means that the cyclic InnoDB redo log may be overwritten.
If the system crashes during this, it would be unable to recover.

The entry point to the problematic code is ibuf_free_excess_pages().
It would make sense to call it before acquiring any mutexes or rw-locks,
in any 'pessimistic' operation that involves the system tablespace.

fseg_create_general(), fseg_alloc_free_page_general(): Do not call
ibuf_free_excess_pages() while potentially holding some latches.

ibuf_remove_free_page(): Do call log_free_check(), like every operation
that is about to generate redo log should do.

ibuf_free_excess_pages(): Remove some assertions that are replaced
by stricter assertions in the log_free_check() that is now called by
ibuf_remove_free_page().

row_ins_sec_index_entry(), row_undo_ins_remove_sec_low(),
row_undo_mod_del_mark_or_remove_sec_low(),
row_undo_mod_del_unmark_sec_and_undo_update(): Call
ibuf_free_excess_pages() if the operation may involve allocating pages
and change buffering in the system tablespace.

10 files changed, 54 insertions, 68 deletions

diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/fsp0fsp.cc
index d04cd7168ed..66f4ada8485 100644
--- a/storage/innobase/fsp/fsp0fsp.cc
+++ b/storage/innobase/fsp/fsp0fsp.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -2032,15 +2033,6 @@ fseg_create_general(
 
 	mtr_x_lock(latch, mtr);
 
-	if (rw_lock_get_x_lock_count(latch) == 1) {
-		/* This thread did not own the latch before this call: free
-		excess pages from the insert buffer free list */
-
-		if (space == IBUF_SPACE_ID) {
-			ibuf_free_excess_pages();
-		}
-	}
-
 	if (!has_done_reservation) {
 		success = fsp_reserve_free_extents(&n_reserved, space, 2,
 						   FSP_NORMAL, mtr);
@@ -2612,15 +2604,6 @@ fseg_alloc_free_page_general(
 
 	mtr_x_lock(latch, mtr);
 
-	if (rw_lock_get_x_lock_count(latch) == 1) {
-		/* This thread did not own the latch before this call: free
-		excess pages from the insert buffer free list */
-
-		if (space == IBUF_SPACE_ID) {
-			ibuf_free_excess_pages();
-		}
-	}
-
 	inode = fseg_inode_get(seg_header, space, zip_size, mtr);
 
 	if (!has_done_reservation
diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc
index 49c1d1a4227..99be552c967 100644
--- a/storage/innobase/ibuf/ibuf0ibuf.cc
+++ b/storage/innobase/ibuf/ibuf0ibuf.cc
@@ -2147,6 +2147,8 @@ ibuf_remove_free_page(void)
 	page_t*	root;
 	page_t*	bitmap_page;
 
+	log_free_check();
+
 	mtr_start(&mtr);
 
 	/* Acquire the fsp latch before the ibuf header, obeying the latching
@@ -2258,22 +2260,7 @@ ibuf_free_excess_pages(void)
 {
 	ulint		i;
 
-#ifdef UNIV_SYNC_DEBUG
-	ut_ad(rw_lock_own(fil_space_get_latch(IBUF_SPACE_ID, NULL),
-			  RW_LOCK_EX));
-#endif /* UNIV_SYNC_DEBUG */
-
-	ut_ad(rw_lock_get_x_lock_count(
-		fil_space_get_latch(IBUF_SPACE_ID, NULL)) == 1);
-
-	/* NOTE: We require that the thread did not own the latch before,
-	because then we know that we can obey the correct latching order
-	for ibuf latches */
-
-	if (!ibuf) {
-		/* Not yet initialized; not sure if this is possible, but
-		does no harm to check for it. */
-
+	if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) {
 		return;
 	}
 
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
index 5cfa21c1fa2..d2e9e8f1940 100644
--- a/storage/innobase/row/row0ins.cc
+++ b/storage/innobase/row/row0ins.cc
@@ -38,6 +38,7 @@ Created 4/20/1996 Heikki Tuuri
 #include "btr0btr.h"
 #include "btr0cur.h"
 #include "mach0data.h"
+#include "ibuf0ibuf.h"
 #include "que0que.h"
 #include "row0upd.h"
 #include "row0sel.h"
@@ -2940,6 +2941,11 @@ row_ins_sec_index_entry(
 	if (err == DB_FAIL) {
 		mem_heap_empty(heap);
 
+		if (index->space == IBUF_SPACE_ID
+		    && !dict_index_is_unique(index)) {
+			ibuf_free_excess_pages();
+		}
+
 		/* Try then pessimistic descent to the B-tree */
 
 		log_free_check();
diff --git a/storage/innobase/row/row0uins.cc b/storage/innobase/row/row0uins.cc
index 651042fb820..a9733f435b7 100644
--- a/storage/innobase/row/row0uins.cc
+++ b/storage/innobase/row/row0uins.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -201,6 +202,10 @@ row_undo_ins_remove_sec_low(
 		mtr_s_lock(dict_index_get_lock(index), &mtr);
 	} else {
 		ut_ad(mode == BTR_MODIFY_TREE);
+		if (index->space == IBUF_SPACE_ID
+		    && !dict_index_is_unique(index)) {
+			ibuf_free_excess_pages();
+		}
 		mtr_x_lock(dict_index_get_lock(index), &mtr);
 	}
 
diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc
index 2e66bc494d4..77fa3cada2b 100644
--- a/storage/innobase/row/row0umod.cc
+++ b/storage/innobase/row/row0umod.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -35,6 +36,7 @@ Created 2/27/1997 Heikki Tuuri
 #include "trx0roll.h"
 #include "btr0btr.h"
 #include "mach0data.h"
+#include "ibuf0ibuf.h"
 #include "row0undo.h"
 #include "row0vers.h"
 #include "row0log.h"
@@ -432,6 +434,11 @@ row_undo_mod_del_mark_or_remove_sec_low(
 
 	log_free_check();
 	mtr_start_trx(&mtr, thr_get_trx(thr));
+	if (mode == BTR_MODIFY_TREE
+	    && index->space == IBUF_SPACE_ID
+	    && !dict_index_is_unique(index)) {
+		ibuf_free_excess_pages();
+	}
 
 	if (*index->name == TEMP_INDEX_PREFIX) {
 		/* The index->online_status may change if the
@@ -604,6 +611,11 @@ row_undo_mod_del_unmark_sec_and_undo_update(
 
 	log_free_check();
 	mtr_start_trx(&mtr, thr_get_trx(thr));
+	if (mode == BTR_MODIFY_TREE
+	    && index->space == IBUF_SPACE_ID
+	    && !dict_index_is_unique(index)) {
+		ibuf_free_excess_pages();
+	}
 
 	if (*index->name == TEMP_INDEX_PREFIX) {
 		/* The index->online_status may change if the
diff --git a/storage/xtradb/fsp/fsp0fsp.cc b/storage/xtradb/fsp/fsp0fsp.cc
index cf6fa4211ca..d441d710671 100644
--- a/storage/xtradb/fsp/fsp0fsp.cc
+++ b/storage/xtradb/fsp/fsp0fsp.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -2041,15 +2042,6 @@ fseg_create_general(
 
 	mtr_x_lock(latch, mtr);
 
-	if (rw_lock_get_x_lock_count(latch) == 1) {
-		/* This thread did not own the latch before this call: free
-		excess pages from the insert buffer free list */
-
-		if (space == IBUF_SPACE_ID) {
-			ibuf_free_excess_pages();
-		}
-	}
-
 	if (!has_done_reservation) {
 		success = fsp_reserve_free_extents(&n_reserved, space, 2,
 						   FSP_NORMAL, mtr);
@@ -2621,15 +2613,6 @@ fseg_alloc_free_page_general(
 
 	mtr_x_lock(latch, mtr);
 
-	if (rw_lock_get_x_lock_count(latch) == 1) {
-		/* This thread did not own the latch before this call: free
-		excess pages from the insert buffer free list */
-
-		if (space == IBUF_SPACE_ID) {
-			ibuf_free_excess_pages();
-		}
-	}
-
 	inode = fseg_inode_get(seg_header, space, zip_size, mtr);
 
 	if (!has_done_reservation
diff --git a/storage/xtradb/ibuf/ibuf0ibuf.cc b/storage/xtradb/ibuf/ibuf0ibuf.cc
index 414d7fc39be..d542e488dba 100644
--- a/storage/xtradb/ibuf/ibuf0ibuf.cc
+++ b/storage/xtradb/ibuf/ibuf0ibuf.cc
@@ -2188,6 +2188,8 @@ ibuf_remove_free_page(void)
 	page_t*	root;
 	page_t*	bitmap_page;
 
+	log_free_check();
+
 	mtr_start(&mtr);
 
 	/* Acquire the fsp latch before the ibuf header, obeying the latching
@@ -2299,22 +2301,7 @@ ibuf_free_excess_pages(void)
 {
 	ulint		i;
 
-#ifdef UNIV_SYNC_DEBUG
-	ut_ad(rw_lock_own(fil_space_get_latch(IBUF_SPACE_ID, NULL),
-			  RW_LOCK_EX));
-#endif /* UNIV_SYNC_DEBUG */
-
-	ut_ad(rw_lock_get_x_lock_count(
-		fil_space_get_latch(IBUF_SPACE_ID, NULL)) == 1);
-
-	/* NOTE: We require that the thread did not own the latch before,
-	because then we know that we can obey the correct latching order
-	for ibuf latches */
-
-	if (!ibuf) {
-		/* Not yet initialized; not sure if this is possible, but
-		does no harm to check for it. */
-
+	if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) {
 		return;
 	}
 
diff --git a/storage/xtradb/row/row0ins.cc b/storage/xtradb/row/row0ins.cc
index ec9f74cd4ad..20a68e7eca2 100644
--- a/storage/xtradb/row/row0ins.cc
+++ b/storage/xtradb/row/row0ins.cc
@@ -38,6 +38,7 @@ Created 4/20/1996 Heikki Tuuri
 #include "btr0btr.h"
 #include "btr0cur.h"
 #include "mach0data.h"
+#include "ibuf0ibuf.h"
 #include "que0que.h"
 #include "row0upd.h"
 #include "row0sel.h"
@@ -3013,6 +3014,11 @@ row_ins_sec_index_entry(
 	if (err == DB_FAIL) {
 		mem_heap_empty(heap);
 
+		if (index->space == IBUF_SPACE_ID
+		    && !dict_index_is_unique(index)) {
+			ibuf_free_excess_pages();
+		}
+
 		/* Try then pessimistic descent to the B-tree */
 
 		log_free_check();
diff --git a/storage/xtradb/row/row0uins.cc b/storage/xtradb/row/row0uins.cc
index 651042fb820..a9733f435b7 100644
--- a/storage/xtradb/row/row0uins.cc
+++ b/storage/xtradb/row/row0uins.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -201,6 +202,10 @@ row_undo_ins_remove_sec_low(
 		mtr_s_lock(dict_index_get_lock(index), &mtr);
 	} else {
 		ut_ad(mode == BTR_MODIFY_TREE);
+		if (index->space == IBUF_SPACE_ID
+		    && !dict_index_is_unique(index)) {
+			ibuf_free_excess_pages();
+		}
 		mtr_x_lock(dict_index_get_lock(index), &mtr);
 	}
 
diff --git a/storage/xtradb/row/row0umod.cc b/storage/xtradb/row/row0umod.cc
index c8a229ea640..bd0a36e2240 100644
--- a/storage/xtradb/row/row0umod.cc
+++ b/storage/xtradb/row/row0umod.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -35,6 +36,7 @@ Created 2/27/1997 Heikki Tuuri
 #include "trx0roll.h"
 #include "btr0btr.h"
 #include "mach0data.h"
+#include "ibuf0ibuf.h"
 #include "row0undo.h"
 #include "row0vers.h"
 #include "row0log.h"
@@ -402,6 +404,11 @@ row_undo_mod_del_mark_or_remove_sec_low(
 
 	log_free_check();
 	mtr_start_trx(&mtr, thr_get_trx(thr));
+	if (mode == BTR_MODIFY_TREE
+	    && index->space == IBUF_SPACE_ID
+	    && !dict_index_is_unique(index)) {
+		ibuf_free_excess_pages();
+	}
 
 	if (*index->name == TEMP_INDEX_PREFIX) {
 		/* The index->online_status may change if the
@@ -574,6 +581,11 @@ row_undo_mod_del_unmark_sec_and_undo_update(
 
 	log_free_check();
 	mtr_start_trx(&mtr, thr_get_trx(thr));
+	if (mode == BTR_MODIFY_TREE
+	    && index->space == IBUF_SPACE_ID
+	    && !dict_index_is_unique(index)) {
+		ibuf_free_excess_pages();
+	}
 
 	if (*index->name == TEMP_INDEX_PREFIX) {
 		/* The index->online_status may change if the