storage/innobase/dict/dict0dict: add a RAII class to freeze a dict_sys_t

This replaces a lot of manual freeze() and unfreeze() call.  Doing it
with RAII is safer and easier.

I did not replace all freeze()/unfreeze() pairs because some callers
unfreeze and re-freeze in the middle of a scope.  Sometimes, adding a
new scope can be added just for such a RAII object.  Refactoring that
can be done later.

Notes:

- Instead of using the global variable `dict_sys`, I decided to pass a
  `dict_sys_t` reference parameter, because I believe it will be
  necessary to eliminate that global variable eventually (in order to
  have a per-catalog instance).  Hard-coding this global variable here
  would generate identical (not better) machine code and would be a
  step in the wrong direction.

- The new macros `SRW_LOCK_ARGS2` and `SRW_LOCK_CALL2` were necessary
  because this is the first time those debug-only parameters are
  forwarded from a function that has more parameters (i.e. the
  dict_sys_t reference).

Author: MaxKellermann

storage/innobase/dict/dict0dict.cc

@@ -808,9 +808,8 @@ dict_acquire_mdl_shared(dict_table_t *table,
 
   if (trylock)
   {
-    dict_sys.freeze(SRW_LOCK_CALL);
+    const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
     db_len= dict_get_db_name_len(table->name.m_name);
-    dict_sys.unfreeze();
   }
   else
   {
@@ -871,9 +870,8 @@ dict_table_t *dict_table_open_on_id(table_id_t table_id, bool dict_locked,
       dict_sys.unlock();
       if (table && thd)
       {
-        dict_sys.freeze(SRW_LOCK_CALL);
+        const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
         table= dict_acquire_mdl_shared<false>(table, thd, mdl, table_op);
-        dict_sys.unfreeze();
       }
       return table;
     }
@@ -3651,19 +3649,13 @@ dict_index_get_if_in_cache(
 /*=======================*/
 	index_id_t	index_id)	/*!< in: index id */
 {
-	dict_index_t*	index;
-
 	if (!dict_sys.is_initialised()) {
 		return(NULL);
 	}
 
-	dict_sys.freeze(SRW_LOCK_CALL);
-
-	index = dict_index_get_if_in_cache_low(index_id);
+	const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 
-	dict_sys.unfreeze();
-
-	return(index);
+	return dict_index_get_if_in_cache_low(index_id);
 }
 
 /**********************************************************************//**
@@ -3930,7 +3922,7 @@ dict_print_info_on_foreign_keys(
 	dict_foreign_t*	foreign;
 	std::string 	str;
 
-	dict_sys.freeze(SRW_LOCK_CALL);
+	const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 
 	for (dict_foreign_set::iterator it = table->foreign_set.begin();
 	     it != table->foreign_set.end();
@@ -3997,7 +3989,6 @@ dict_print_info_on_foreign_keys(
 		}
 	}
 
-	dict_sys.unfreeze();
 	return str;
 }
 
@@ -4191,14 +4182,12 @@ void
 dict_set_merge_threshold_all_debug(
 	uint	merge_threshold_all)
 {
-	dict_sys.freeze(SRW_LOCK_CALL);
+	const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 
 	dict_set_merge_threshold_list_debug(
 		&dict_sys.table_LRU, merge_threshold_all);
 	dict_set_merge_threshold_list_debug(
 		&dict_sys.table_non_LRU, merge_threshold_all);
-
-	dict_sys.unfreeze();
 }
 
 #endif /* UNIV_DEBUG */

storage/innobase/dict/dict0stats.cc

@@ -3209,10 +3209,9 @@ dict_stats_save(
 	dict_table_t* table_stats = dict_table_open_on_name(
 		TABLE_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 	if (table_stats) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		table_stats = dict_acquire_mdl_shared<false>(table_stats, thd,
 							     &mdl_table);
-		dict_sys.unfreeze();
 	}
 	if (!table_stats
 	    || strcmp(table_stats->name.m_name, TABLE_STATS_NAME)) {
@@ -3226,10 +3225,9 @@ dict_stats_save(
 	dict_table_t* index_stats = dict_table_open_on_name(
 		INDEX_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 	if (index_stats) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		index_stats = dict_acquire_mdl_shared<false>(index_stats, thd,
 							     &mdl_index);
-		dict_sys.unfreeze();
 	}
 	if (!index_stats) {
 		goto release_and_exit;
@@ -3788,10 +3786,9 @@ dict_stats_fetch_from_ps(
 	dict_table_t* table_stats = dict_table_open_on_name(
 		TABLE_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 	if (table_stats) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		table_stats = dict_acquire_mdl_shared<false>(table_stats, thd,
 							     &mdl_table);
-		dict_sys.unfreeze();
 	}
 	if (!table_stats
 	    || strcmp(table_stats->name.m_name, TABLE_STATS_NAME)) {
@@ -3805,10 +3802,9 @@ dict_stats_fetch_from_ps(
 	dict_table_t* index_stats = dict_table_open_on_name(
 		INDEX_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 	if (index_stats) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		index_stats = dict_acquire_mdl_shared<false>(index_stats, thd,
 							     &mdl_index);
-		dict_sys.unfreeze();
 	}
 	if (!index_stats) {
 		goto release_and_exit;

storage/innobase/handler/ha_innodb.cc

@@ -1367,19 +1367,17 @@ static void innodb_drop_database(handlerton*, char *path)
                                        DICT_ERR_IGNORE_NONE);
   if (table_stats)
   {
-    dict_sys.freeze(SRW_LOCK_CALL);
+    const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
     table_stats= dict_acquire_mdl_shared<false>(table_stats,
                                                 thd, &mdl_table);
-    dict_sys.unfreeze();
   }
   index_stats= dict_table_open_on_name(INDEX_STATS_NAME, false,
                                        DICT_ERR_IGNORE_NONE);
   if (index_stats)
   {
-    dict_sys.freeze(SRW_LOCK_CALL);
+    const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
     index_stats= dict_acquire_mdl_shared<false>(index_stats,
                                                 thd, &mdl_index);
-    dict_sys.unfreeze();
   }
 
   trx_start_for_ddl(trx);
@@ -13553,20 +13551,18 @@ int ha_innobase::delete_table(const char *name)
                                          DICT_ERR_IGNORE_NONE);
     if (table_stats)
     {
-      dict_sys.freeze(SRW_LOCK_CALL);
+      const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
       table_stats= dict_acquire_mdl_shared<false>(table_stats,
                                                   thd, &mdl_table);
-      dict_sys.unfreeze();
     }
 
     index_stats= dict_table_open_on_name(INDEX_STATS_NAME, false,
                                          DICT_ERR_IGNORE_NONE);
     if (index_stats)
     {
-      dict_sys.freeze(SRW_LOCK_CALL);
+      const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
       index_stats= dict_acquire_mdl_shared<false>(index_stats,
                                                   thd, &mdl_index);
-      dict_sys.unfreeze();
     }
 
     const bool skip_wait{table->name.is_temporary()};
@@ -13805,10 +13801,9 @@ int ha_innobase::truncate()
     /* fk_truncate_illegal_if_parent() should have failed in
     Sql_cmd_truncate_table::handler_truncate() if foreign_key_checks=ON
     and child tables exist. */
-    dict_sys.freeze(SRW_LOCK_CALL);
+    const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
     for (const auto foreign : m_prebuilt->table->referenced_set)
       ut_ad(foreign->foreign_table == m_prebuilt->table);
-    dict_sys.unfreeze();
   }
 #endif
 
@@ -13927,19 +13922,17 @@ int ha_innobase::truncate()
                                          DICT_ERR_IGNORE_NONE);
     if (table_stats)
     {
-      dict_sys.freeze(SRW_LOCK_CALL);
+      const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
       table_stats= dict_acquire_mdl_shared<false>(table_stats, m_user_thd,
                                                   &mdl_table);
-      dict_sys.unfreeze();
     }
     index_stats= dict_table_open_on_name(INDEX_STATS_NAME, false,
                                          DICT_ERR_IGNORE_NONE);
     if (index_stats)
     {
-      dict_sys.freeze(SRW_LOCK_CALL);
+      const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
       index_stats= dict_acquire_mdl_shared<false>(index_stats, m_user_thd,
                                                   &mdl_index);
-      dict_sys.unfreeze();
     }
 
     if (table_stats && index_stats &&
@@ -14099,18 +14092,16 @@ ha_innobase::rename_table(
 		table_stats = dict_table_open_on_name(TABLE_STATS_NAME, false,
 						      DICT_ERR_IGNORE_NONE);
 		if (table_stats) {
-			dict_sys.freeze(SRW_LOCK_CALL);
+			const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 			table_stats = dict_acquire_mdl_shared<false>(
 				table_stats, thd, &mdl_table);
-			dict_sys.unfreeze();
 		}
 		index_stats = dict_table_open_on_name(INDEX_STATS_NAME, false,
 						      DICT_ERR_IGNORE_NONE);
 		if (index_stats) {
-			dict_sys.freeze(SRW_LOCK_CALL);
+			const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 			index_stats = dict_acquire_mdl_shared<false>(
 				index_stats, thd, &mdl_index);
-			dict_sys.unfreeze();
 		}
 
 		if (error == DB_SUCCESS && table_stats && index_stats
@@ -15642,10 +15633,8 @@ REPLACE, not an update.
 
 uint ha_innobase::referenced_by_foreign_key()
 {
-  dict_sys.freeze(SRW_LOCK_CALL);
-  const bool empty= m_prebuilt->table->referenced_set.empty();
-  dict_sys.unfreeze();
-  return !empty;
+  const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
+  return !m_prebuilt->table->referenced_set.empty();
 }
 
 /*******************************************************************//**

storage/innobase/handler/handler0alter.cc

@@ -11404,18 +11404,16 @@ ha_innobase::commit_inplace_alter_table(
 		table_stats = dict_table_open_on_name(
 			TABLE_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 		if (table_stats) {
-			dict_sys.freeze(SRW_LOCK_CALL);
+			const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 			table_stats = dict_acquire_mdl_shared<false>(
 				table_stats, m_user_thd, &mdl_table);
-			dict_sys.unfreeze();
 		}
 		index_stats = dict_table_open_on_name(
 			INDEX_STATS_NAME, false, DICT_ERR_IGNORE_NONE);
 		if (index_stats) {
-			dict_sys.freeze(SRW_LOCK_CALL);
+			const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 			index_stats = dict_acquire_mdl_shared<false>(
 				index_stats, m_user_thd, &mdl_index);
-			dict_sys.unfreeze();
 		}
 
 		if (table_stats && index_stats

storage/innobase/include/dict0dict.h

@@ -1570,6 +1570,28 @@ class dict_sys_t
   dberr_t create_or_check_sys_tables();
 };
 
+/*********************************************************************//**
+Freeze a dict_sys_t and automatically unfreeze it when the scope exits. */
+class Scope_freeze_dict_sys {
+  dict_sys_t &the_dict_sys;
+
+public:
+  explicit Scope_freeze_dict_sys(dict_sys_t &_the_dict_sys SRW_LOCK_ARGS2(const char *file, unsigned line)) noexcept
+    :the_dict_sys(_the_dict_sys)
+  {
+    the_dict_sys.freeze(SRW_LOCK_ARGS(file, line));
+  }
+
+  ~Scope_freeze_dict_sys() noexcept {
+    the_dict_sys.unfreeze();
+  }
+
+private:
+  // copying/moving not allowed
+  Scope_freeze_dict_sys(const Scope_freeze_dict_sys &) = delete;
+  Scope_freeze_dict_sys &operator=(const Scope_freeze_dict_sys &) = delete;
+};
+
 /** the data dictionary cache */
 extern dict_sys_t	dict_sys;
 

storage/innobase/lock/lock0lock.cc

@@ -4121,7 +4121,7 @@ dberr_t lock_table_children(dict_table_t *table, trx_t *trx)
         if ((err= lock_table_for_trx(child.table, trx, LOCK_X)) != DB_SUCCESS)
           break;
 
-  dict_sys.freeze(SRW_LOCK_CALL);
+  const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
   for (table_mdl &child : children)
   {
     if (child.mdl)
@@ -4130,7 +4130,6 @@ dberr_t lock_table_children(dict_table_t *table, trx_t *trx)
       mdl_context->release_lock(child.mdl);
     }
   }
-  dict_sys.unfreeze();
 
   return err;
 }

storage/innobase/row/row0uins.cc

@@ -383,9 +383,8 @@ static bool row_undo_ins_parse_undo_rec(undo_node_t* node, bool dict_locked)
 		node->table = dict_table_open_on_id(table_id, dict_locked,
 						    DICT_TABLE_OP_NORMAL);
 	} else if (!dict_locked) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		node->table = dict_sys.acquire_temporary_table(table_id);
-		dict_sys.unfreeze();
 	} else {
 		node->table = dict_sys.acquire_temporary_table(table_id);
 	}

storage/innobase/row/row0umod.cc

@@ -1210,9 +1210,8 @@ static bool row_undo_mod_parse_undo_rec(undo_node_t* node, bool dict_locked)
 		node->table = dict_table_open_on_id(table_id, dict_locked,
 						    DICT_TABLE_OP_NORMAL);
 	} else if (!dict_locked) {
-		dict_sys.freeze(SRW_LOCK_CALL);
+		const Scope_freeze_dict_sys freeze_dict_sys{dict_sys SRW_LOCK_CALL2};
 		node->table = dict_sys.acquire_temporary_table(table_id);
-		dict_sys.unfreeze();
 	} else {
 		node->table = dict_sys.acquire_temporary_table(table_id);
 	}