MDEV-31132 Deadlock between DDL and purge of InnoDB history

log_free_check(): Assert that the caller must not hold
exclusive lock_sys.latch. This was the case for calls from
ibuf_delete_for_discarded_space(). This caused a deadlock with
another thread that would be holding a latch on a dirty page
that would need to be written so that the checkpoint would advance
and log_free_check() could return. That other thread was waiting
for a shared lock_sys.latch.

fil_delete_tablespace(): Do not invoke ibuf_delete_for_discarded_space()
because in DDL operations, we will be holding exclusive lock_sys.latch.

trx_t::commit(std::vector<pfs_os_file_t>&), innodb_drop_database(),
row_purge_remove_clust_if_poss_low(), row_undo_ins_remove_clust_rec(),
row_discard_tablespace_for_mysql():
Invoke ibuf_delete_for_discarded_space() on the deleted tablespaces after
releasing all latches.

9 files changed, 41 insertions, 35 deletions

diff --git a/storage/innobase/dict/drop.cc b/storage/innobase/dict/drop.cc
index 9013841ba5e..45747fb3596 100644
--- a/storage/innobase/dict/drop.cc
+++ b/storage/innobase/dict/drop.cc
@@ -68,6 +68,7 @@ before transaction commit and must be rolled back explicitly are as follows:
 
 #include "dict0defrag_bg.h"
 #include "btr0defragment.h"
+#include "ibuf0ibuf.h"
 #include "lock0lock.h"
 
 #include "que0que.h"
@@ -237,6 +238,8 @@ void trx_t::commit(std::vector<pfs_os_file_t> &deleted)
   commit_persist();
   if (dict_operation)
   {
+    std::vector<uint32_t> space_ids;
+    space_ids.reserve(mod_tables.size());
     ut_ad(dict_sys.locked());
     lock_sys.wr_lock(SRW_LOCK_CALL);
     mutex_lock();
@@ -271,6 +274,7 @@ void trx_t::commit(std::vector<pfs_os_file_t> &deleted)
         dict_sys.remove(table);
         if (const auto id= space ? space->id : 0)
         {
+          space_ids.emplace_back(uint32_t(id));
           pfs_os_file_t d= fil_delete_tablespace(id);
           if (d != OS_FILE_CLOSED)
             deleted.emplace_back(d);
@@ -283,6 +287,9 @@ void trx_t::commit(std::vector<pfs_os_file_t> &deleted)
     mysql_mutex_lock(&lock_sys.wait_mutex);
     lock_sys.deadlock_check();
     mysql_mutex_unlock(&lock_sys.wait_mutex);
+
+    for (const auto id : space_ids)
+      ibuf_delete_for_discarded_space(id);
   }
   commit_cleanup();
 }
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 48d205f428a..23f0cf75f39 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -45,7 +45,6 @@ Created 10/25/1995 Heikki Tuuri
 #include "srv0start.h"
 #include "trx0purge.h"
 #include "buf0lru.h"
-#include "ibuf0ibuf.h"
 #include "buf0flu.h"
 #include "log.h"
 #ifdef __linux__
@@ -1689,7 +1688,6 @@ pfs_os_file_t fil_delete_tablespace(ulint id)
     fil_space_free_low(space);
   }
 
-  ibuf_delete_for_discarded_space(id);
   return handle;
 }
 
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index f102789d7ab..0b117c02e29 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -1542,6 +1542,7 @@ static void innodb_drop_database(handlerton*, char *path)
     dfield_set_data(&dfield, namebuf, len);
     dict_index_copy_types(&tuple, sys_index, 1);
     std::vector<pfs_os_file_t> to_close;
+    std::vector<uint32_t> space_ids;
     mtr_t mtr;
     mtr.start();
     pcur.btr_cur.page_cur.index = sys_index;
@@ -1585,6 +1586,7 @@ static void innodb_drop_database(handlerton*, char *path)
         ut_ad("corrupted SYS_TABLES.SPACE" == 0);
       else if (uint32_t space_id= mach_read_from_4(s))
       {
+        space_ids.emplace_back(space_id);
         pfs_os_file_t detached= fil_delete_tablespace(space_id);
         if (detached != OS_FILE_CLOSED)
           to_close.emplace_back(detached);
@@ -1594,6 +1596,9 @@ static void innodb_drop_database(handlerton*, char *path)
     mtr.commit();
     for (pfs_os_file_t detached : to_close)
       os_file_close(detached);
+    for (const auto id : space_ids)
+      ibuf_delete_for_discarded_space(id);
+
     /* Any changes must be persisted before we return. */
     log_write_up_to(mtr.commit_lsn(), true);
   }
diff --git a/storage/innobase/include/log0log.h b/storage/innobase/include/log0log.h
index 0f9a4da049b..0996b66ef52 100644
--- a/storage/innobase/include/log0log.h
+++ b/storage/innobase/include/log0log.h
@@ -73,15 +73,10 @@ log_reserve_and_write_fast(
 	const void*	str,
 	ulint		len,
 	lsn_t*		start_lsn);
-/***********************************************************************//**
-Checks if there is need for a log buffer flush or a new checkpoint, and does
-this if yes. Any database operation should call this when it has modified
-more than about 4 pages. NOTE that this function may only be called when the
-OS thread owns no synchronization objects except dict_sys.latch. */
-UNIV_INLINE
-void
-log_free_check(void);
-/*================*/
+/** Wait for a log checkpoint if needed.
+NOTE that this function may only be called while not holding
+any synchronization objects except dict_sys.latch. */
+void log_free_check();
 
 /** Extends the log buffer.
 @param[in]	len	requested minimum size in bytes */
diff --git a/storage/innobase/include/log0log.inl b/storage/innobase/include/log0log.inl
index c29c0bfa55f..27856fca8f9 100644
--- a/storage/innobase/include/log0log.inl
+++ b/storage/innobase/include/log0log.inl
@@ -289,23 +289,3 @@ log_reserve_and_write_fast(
 
 	return lsn;
 }
-
-/***********************************************************************//**
-Checks if there is need for a log buffer flush or a new checkpoint, and does
-this if yes. Any database operation should call this when it has modified
-more than about 4 pages. NOTE that this function may only be called when the
-OS thread owns no synchronization objects except dict_sys.latch. */
-UNIV_INLINE
-void
-log_free_check(void)
-/*================*/
-{
-	/* During row_log_table_apply(), this function will be called while we
-	are holding some latches. This is OK, as long as we are not holding
-	any latches on buffer blocks. */
-
-	if (log_sys.check_flush_or_checkpoint()) {
-
-		log_check_margins();
-	}
-}
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
index c53e2fd5074..f65e812f40f 100644
--- a/storage/innobase/log/log0log.cc
+++ b/storage/innobase/log/log0log.cc
@@ -1065,6 +1065,16 @@ ATTRIBUTE_COLD void log_check_margins()
   while (log_sys.check_flush_or_checkpoint());
 }
 
+/** Wait for a log checkpoint if needed.
+NOTE that this function may only be called while not holding
+any synchronization objects except dict_sys.latch. */
+void log_free_check()
+{
+  ut_ad(!lock_sys.is_writer());
+  if (log_sys.check_flush_or_checkpoint())
+    log_check_margins();
+}
+
 extern void buf_resize_shutdown();
 
 /** Make a checkpoint at the latest lsn on shutdown. */
diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
index 67167f19c70..d27fc964219 100644
--- a/storage/innobase/row/row0mysql.cc
+++ b/storage/innobase/row/row0mysql.cc
@@ -2492,6 +2492,7 @@ rollback:
   if (fts_exist)
     purge_sys.resume_FTS();
 
+  ibuf_delete_for_discarded_space(space_id);
   buf_flush_remove_pages(space_id);
   trx->op_info= "";
   return err;
diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc
index 753b42332fc..f4db252b069 100644
--- a/storage/innobase/row/row0purge.cc
+++ b/storage/innobase/row/row0purge.cc
@@ -162,8 +162,9 @@ close_and_exit:
 			ut_ad("corrupted SYS_INDEXES record" == 0);
 		}
 
-		if (const uint32_t space_id = dict_drop_index_tree(
-			    &node->pcur, nullptr, &mtr)) {
+		const uint32_t space_id = dict_drop_index_tree(
+			&node->pcur, nullptr, &mtr);
+		if (space_id) {
 			if (table) {
 				if (table->get_ref_count() == 0) {
 					dict_sys.remove(table);
@@ -184,6 +185,10 @@ close_and_exit:
 			table = nullptr;
 		}
 
+		if (space_id) {
+			ibuf_delete_for_discarded_space(space_id);
+		}
+
 		purge_sys.check_stop_SYS();
 		mtr.start();
 		index->set_modified(mtr);
diff --git a/storage/innobase/row/row0uins.cc b/storage/innobase/row/row0uins.cc
index 50196e78092..ff1a34b46c3 100644
--- a/storage/innobase/row/row0uins.cc
+++ b/storage/innobase/row/row0uins.cc
@@ -147,8 +147,9 @@ restart:
 
 		pfs_os_file_t d = OS_FILE_CLOSED;
 
-		if (const uint32_t space_id = dict_drop_index_tree(
-			    &node->pcur, node->trx, &mtr)) {
+		const uint32_t space_id = dict_drop_index_tree(
+			&node->pcur, node->trx, &mtr);
+		if (space_id) {
 			if (table) {
 				lock_release_on_rollback(node->trx,
 							 table);
@@ -187,6 +188,10 @@ restart:
 			os_file_close(d);
 		}
 
+		if (space_id) {
+			ibuf_delete_for_discarded_space(space_id);
+		}
+
 		mtr.start();
 		ut_a(node->pcur.restore_position(
 			BTR_MODIFY_LEAF, &mtr) == btr_pcur_t::SAME_ALL);