MDEV-25223 change fil_system_t::space_list and fil_system_t::named_spaces from UT_LIST to ilist

Mostly a refactoring. Also, debug functions were added for ease of life while debugging

extra/mariabackup/xtrabackup.cc

@@ -588,36 +588,20 @@ static dberr_t enumerate_ibd_files(process_single_tablespace_func_t callback);
 
 /* ======== Datafiles iterator ======== */
 struct datafiles_iter_t {
-	fil_space_t	*space;
-	fil_node_t	*node;
-	ibool		started;
-	pthread_mutex_t	mutex;
+	space_list_t::iterator space = fil_system.space_list.end();
+	fil_node_t	*node = nullptr;
+	bool		started = false;
+	std::mutex	mutex;
 };
 
 /* ======== Datafiles iterator ======== */
-static
-datafiles_iter_t *
-datafiles_iter_new()
-{
-	datafiles_iter_t *it;
-
-	it = static_cast<datafiles_iter_t *>(malloc(sizeof(datafiles_iter_t)));
-	pthread_mutex_init(&it->mutex, NULL);
-
-	it->space = NULL;
-	it->node = NULL;
-	it->started = FALSE;
-
-	return it;
-}
-
 static
 fil_node_t *
 datafiles_iter_next(datafiles_iter_t *it)
 {
 	fil_node_t *new_node;
 
-	pthread_mutex_lock(&it->mutex);
+	std::lock_guard<std::mutex> _(it->mutex);
 
 	if (it->node == NULL) {
 		if (it->started)
@@ -629,34 +613,25 @@ datafiles_iter_next(datafiles_iter_t *it)
 			goto end;
 	}
 
-	it->space = (it->space == NULL) ?
-		UT_LIST_GET_FIRST(fil_system.space_list) :
-		UT_LIST_GET_NEXT(space_list, it->space);
+	it->space = (it->space == fil_system.space_list.end()) ?
+		fil_system.space_list.begin() :
+		std::next(it->space);
 
-	while (it->space != NULL &&
+	while (it->space != fil_system.space_list.end() &&
 	       (it->space->purpose != FIL_TYPE_TABLESPACE ||
 		UT_LIST_GET_LEN(it->space->chain) == 0))
-		it->space = UT_LIST_GET_NEXT(space_list, it->space);
-	if (it->space == NULL)
+		++it->space;
+	if (it->space == fil_system.space_list.end())
 		goto end;
 
 	it->node = UT_LIST_GET_FIRST(it->space->chain);
 
 end:
 	new_node = it->node;
-	pthread_mutex_unlock(&it->mutex);
 
 	return new_node;
 }
 
-static
-void
-datafiles_iter_free(datafiles_iter_t *it)
-{
-	pthread_mutex_destroy(&it->mutex);
-	free(it);
-}
-
 #ifndef DBUG_OFF
 struct dbug_thread_param_t
 {
@@ -750,18 +725,15 @@ static void dbug_start_query_thread(
 void mdl_lock_all()
 {
 	mdl_lock_init();
-	datafiles_iter_t *it = datafiles_iter_new();
-	if (!it)
-		return;
+	datafiles_iter_t it;
 
-	while (fil_node_t *node = datafiles_iter_next(it)){
+	while (fil_node_t *node = datafiles_iter_next(&it)) {
 		if (fil_is_user_tablespace_id(node->space->id)
 			&& check_if_skip_table(node->space->name))
 			continue;
 
 		mdl_lock_table(node->space->id);
 	}
-	datafiles_iter_free(it);
 }
 
 
@@ -4441,11 +4413,7 @@ static bool xtrabackup_backup_func()
 				"Waiting for table metadata lock", 0, 0););
 	}
 
-	datafiles_iter_t *it = datafiles_iter_new();
-	if (it == NULL) {
-		msg("mariabackup: Error: datafiles_iter_new() failed.");
-		goto fail;
-	}
+	datafiles_iter_t it;
 
 	/* Create data copying threads */
 	data_threads = (data_thread_ctxt_t *)
@@ -4454,7 +4422,7 @@ static bool xtrabackup_backup_func()
 	pthread_mutex_init(&count_mutex, NULL);
 
 	for (i = 0; i < (uint) xtrabackup_parallel; i++) {
-		data_threads[i].it = it;
+		data_threads[i].it = &it;
 		data_threads[i].num = i+1;
 		data_threads[i].count = &count;
 		data_threads[i].count_mutex = &count_mutex;
@@ -4475,7 +4443,6 @@ static bool xtrabackup_backup_func()
 
 	pthread_mutex_destroy(&count_mutex);
 	free(data_threads);
-	datafiles_iter_free(it);
 	}
 
 	bool ok = backup_start(corrupted_pages);
@@ -4629,10 +4596,8 @@ void backup_fix_ddl(CorruptedPages &corrupted_pages)
 	//  Load and copy new tables.
 	//  Close all datanodes first, reload only new tables.
 	std::vector<fil_node_t *> all_nodes;
-	datafiles_iter_t *it = datafiles_iter_new();
-	if (!it)
-		return;
-	while (fil_node_t *node = datafiles_iter_next(it)) {
+	datafiles_iter_t it;
+	while (fil_node_t *node = datafiles_iter_next(&it)) {
 		all_nodes.push_back(node);
 	}
 	for (size_t i = 0; i < all_nodes.size(); i++) {
@@ -4646,7 +4611,6 @@ void backup_fix_ddl(CorruptedPages &corrupted_pages)
 		}
 		fil_space_free(n->space->id, false);
 	}
-	datafiles_iter_free(it);
 
 	DBUG_EXECUTE_IF("check_mdl_lock_works", DBUG_ASSERT(new_tables.size() == 0););
 	for (std::set<std::string>::iterator iter = new_tables.begin();
@@ -4657,11 +4621,9 @@ void backup_fix_ddl(CorruptedPages &corrupted_pages)
 		xb_load_single_table_tablespace(*iter, false);
 	}
 
-	it = datafiles_iter_new();
-	if (!it)
-		return;
+	datafiles_iter_t it2;
 
-	while (fil_node_t *node = datafiles_iter_next(it)) {
+	while (fil_node_t *node = datafiles_iter_next(&it2)) {
 		fil_space_t * space = node->space;
 		if (!fil_is_user_tablespace_id(space->id))
 			continue;
@@ -4670,8 +4632,6 @@ void backup_fix_ddl(CorruptedPages &corrupted_pages)
 		xtrabackup_copy_datafile(node, 0, dest_name.c_str(), wf_write_through,
 			corrupted_pages);
 	}
-
-	datafiles_iter_free(it);
 }
 
 /* ================= prepare ================= */
@@ -4787,10 +4747,8 @@ xb_space_create_file(
 static fil_space_t* fil_space_get_by_name(const char* name)
 {
 	mysql_mutex_assert_owner(&fil_system.mutex);
-	for (fil_space_t* space = UT_LIST_GET_FIRST(fil_system.space_list);
-	     space != NULL;
-	     space = UT_LIST_GET_NEXT(space_list, space))
-		if (!strcmp(space->name, name)) return space;
+	for (fil_space_t& space :fil_system.space_list)
+		if (!strcmp(space.name, name)) return &space;
 	return NULL;
 }
 

include/ilist.h

@@ -75,8 +75,10 @@ template <class T, class Tag= void> class ilist
     typedef T *pointer;
     typedef T &reference;
 
-    Iterator(ListNode *node) noexcept : node_(node)
-    { DBUG_ASSERT(node_ != nullptr); }
+    explicit Iterator(ListNode *node) noexcept : node_(node)
+    {
+      DBUG_ASSERT(node_ != nullptr);
+    }
 
     Iterator &operator++() noexcept
     {
@@ -193,7 +195,7 @@ template <class T, class Tag= void> class ilist
     curr->next= nullptr;
 #endif
 
-    return next;
+    return Iterator(next);
   }
 
   void push_back(reference value) noexcept { insert(end(), value); }

storage/innobase/fts/fts0fts.cc

@@ -5735,15 +5735,13 @@ static void fil_get_fts_spaces(fts_space_set_t& fts_space_set)
 {
   mysql_mutex_lock(&fil_system.mutex);
 
-  for (fil_space_t *space= UT_LIST_GET_FIRST(fil_system.space_list);
-       space;
-       space= UT_LIST_GET_NEXT(space_list, space))
+  for (fil_space_t &space : fil_system.space_list)
   {
     index_id_t index_id= 0;
     table_id_t table_id= 0;
 
-    if (space->purpose == FIL_TYPE_TABLESPACE
-        && fts_check_aux_table(space->name, &table_id, &index_id))
+    if (space.purpose == FIL_TYPE_TABLESPACE
+        && fts_check_aux_table(space.name, &table_id, &index_id))
       fts_space_set.insert(std::make_pair(table_id, index_id));
   }
 

storage/innobase/handler/i_s.cc

@@ -6552,17 +6552,16 @@ static int i_s_sys_tablespaces_fill_table(THD *thd, TABLE_LIST *tables, Item*)
   mysql_mutex_lock(&fil_system.mutex);
   fil_system.freeze_space_list++;
 
-  for (fil_space_t *space= UT_LIST_GET_FIRST(fil_system.space_list);
-       space; space= UT_LIST_GET_NEXT(space_list, space))
+  for (fil_space_t &space : fil_system.space_list)
   {
-    if (space->purpose == FIL_TYPE_TABLESPACE && !space->is_stopping() &&
-        space->chain.start)
+    if (space.purpose == FIL_TYPE_TABLESPACE && !space.is_stopping() &&
+        space.chain.start)
     {
-      space->reacquire();
+      space.reacquire();
       mysql_mutex_unlock(&fil_system.mutex);
-      err= i_s_sys_tablespaces_fill(thd, *space, tables->table);
+      err= i_s_sys_tablespaces_fill(thd, space, tables->table);
       mysql_mutex_lock(&fil_system.mutex);
-      space->release();
+      space.release();
       if (err)
         break;
     }
@@ -6772,16 +6771,15 @@ i_s_tablespaces_encryption_fill_table(
 	mysql_mutex_lock(&fil_system.mutex);
 	fil_system.freeze_space_list++;
 
-	for (fil_space_t* space = UT_LIST_GET_FIRST(fil_system.space_list);
-	     space; space = UT_LIST_GET_NEXT(space_list, space)) {
-		if (space->purpose == FIL_TYPE_TABLESPACE
-		    && !space->is_stopping()) {
-			space->reacquire();
+	for (fil_space_t& space : fil_system.space_list) {
+		if (space.purpose == FIL_TYPE_TABLESPACE
+		    && !space.is_stopping()) {
+			space.reacquire();
 			mysql_mutex_unlock(&fil_system.mutex);
 			err = i_s_dict_fill_tablespaces_encryption(
-				thd, space, tables->table);
+				thd, &space, tables->table);
 			mysql_mutex_lock(&fil_system.mutex);
-			space->release();
+			space.release();
 			if (err) {
 				break;
 			}

storage/innobase/include/fil0crypt.h

@@ -213,12 +213,13 @@ struct fil_space_crypt_status_t {
 };
 
 /** Statistics about encryption key rotation */
-struct fil_crypt_stat_t {
-	ulint pages_read_from_cache;
-	ulint pages_read_from_disk;
-	ulint pages_modified;
-	ulint pages_flushed;
-	ulint estimated_iops;
+struct fil_crypt_stat_t
+{
+  ulint pages_read_from_cache= 0;
+  ulint pages_read_from_disk= 0;
+  ulint pages_modified= 0;
+  ulint pages_flushed= 0;
+  ulint estimated_iops= 0;
 };
 
 /*********************************************************************

storage/innobase/include/fil0fil.h

@@ -44,6 +44,10 @@ Created 10/25/1995 Heikki Tuuri
 
 struct unflushed_spaces_tag_t;
 struct rotation_list_tag_t;
+struct space_list_tag_t;
+struct named_spaces_tag_t;
+
+using space_list_t= ilist<fil_space_t, space_list_tag_t>;
 
 // Forward declaration
 extern my_bool srv_use_doublewrite_buf;
@@ -331,8 +335,10 @@ enum fil_encryption_t
   FIL_ENCRYPTION_OFF
 };
 
-struct fil_space_t final :
-  ilist_node<unflushed_spaces_tag_t>, ilist_node<rotation_list_tag_t>
+struct fil_space_t final : ilist_node<unflushed_spaces_tag_t>,
+                           ilist_node<rotation_list_tag_t>,
+                           ilist_node<space_list_tag_t>,
+                           ilist_node<named_spaces_tag_t>
 #else
 struct fil_space_t final
 #endif
@@ -377,6 +383,14 @@ struct fil_space_t final
 				/*!< number of reserved free extents for
 				ongoing operations like B-tree page split */
 private:
+#ifdef UNIV_DEBUG
+  fil_space_t *next_in_space_list();
+  fil_space_t *prev_in_space_list();
+
+  fil_space_t *next_in_unflushed_spaces();
+  fil_space_t *prev_in_unflushed_spaces();
+#endif
+
   /** the committed size of the tablespace in pages */
   Atomic_relaxed<uint32_t> committed_size;
   /** Number of pending operations on the file.
@@ -399,12 +413,6 @@ struct fil_space_t final
   os_thread_id_t latch_owner;
   ut_d(Atomic_relaxed<uint32_t> latch_count;)
 public:
-	UT_LIST_NODE_T(fil_space_t) named_spaces;
-				/*!< list of spaces for which FILE_MODIFY
-				records have been issued */
-	UT_LIST_NODE_T(fil_space_t) space_list;
-				/*!< list of all spaces */
-
 	/** MariaDB encryption data */
 	fil_space_crypt_t* crypt_data;
 
@@ -989,8 +997,8 @@ struct fil_space_t final
   @param encrypt  expected state of innodb_encrypt_tables
   @return the next tablespace
   @retval nullptr upon reaching the end of the iteration */
-  static inline fil_space_t *next(fil_space_t *space, bool recheck,
-                                  bool encrypt);
+  static space_list_t::iterator next(space_list_t::iterator space,
+                                     bool recheck, bool encrypt);
 
 #ifdef UNIV_DEBUG
   bool is_latched() const { return latch_count != 0; }
@@ -1371,11 +1379,7 @@ struct fil_system_t {
     Some members may require late initialisation, thus we just mark object as
     uninitialised. Real initialisation happens in create().
   */
-  fil_system_t(): m_initialised(false)
-  {
-    UT_LIST_INIT(space_list, &fil_space_t::space_list);
-    UT_LIST_INIT(named_spaces, &fil_space_t::named_spaces);
-  }
+  fil_system_t() : m_initialised(false) {}
 
   bool is_initialised() const { return m_initialised; }
 
@@ -1434,9 +1438,9 @@ struct fil_system_t {
   /** nonzero if fil_node_open_file_low() should avoid moving the tablespace
   to the end of space_list, for FIFO policy of try_to_close() */
   ulint freeze_space_list;
-	UT_LIST_BASE_NODE_T(fil_space_t) space_list;
+  ilist<fil_space_t, space_list_tag_t> space_list;
 					/*!< list of all file spaces */
-	UT_LIST_BASE_NODE_T(fil_space_t) named_spaces;
+  ilist<fil_space_t, named_spaces_tag_t> named_spaces;
 					/*!< list of all file spaces
 					for which a FILE_MODIFY
 					record has been written since
@@ -1458,8 +1462,7 @@ struct fil_system_t {
   @param encrypt expected state of innodb_encrypt_tables
   @return the next tablespace to process (n_pending_ops incremented)
   @retval NULL if this was the last */
-  inline fil_space_t* keyrotate_next(fil_space_t *space, bool recheck,
-                                     bool encrypt);
+  fil_space_t* keyrotate_next(fil_space_t* space, bool recheck, bool encrypt);
 
         /** Extend all open data files to the recovered size */
         ATTRIBUTE_COLD void extend_to_recv_size();