MDEV-23651 InnoDB: Failing assertion: !space->referenced()

This is the 10.5 version of the 10.4 merge
commit 1cda462f
for testing purposes.

include/my_atomic_wrapper.h

@@ -48,6 +48,8 @@ template <typename Type> class Atomic_relaxed
   { return m.fetch_add(i, o); }
   Type fetch_sub(const Type i, std::memory_order o= std::memory_order_relaxed)
   { return m.fetch_sub(i, o); }
+  Type fetch_xor(const Type i, std::memory_order o= std::memory_order_relaxed)
+  { return m.fetch_xor(i, o); }
   bool compare_exchange_strong(Type& i1, const Type i2,
                                std::memory_order o1= std::memory_order_relaxed,
                                std::memory_order o2= std::memory_order_relaxed)

storage/innobase/fil/fil0crypt.cc

@@ -1490,12 +1490,15 @@ inline fil_space_t *fil_system_t::keyrotate_next(fil_space_t *space,
     }
   }
 
-  if (it == end)
-    return NULL;
+  while (it != end)
+  {
+    space= &*it;
+    if (space->acquire())
+      return space;
+    while (++it != end && (!UT_LIST_GET_LEN(it->chain) || it->is_stopping()));
+  }
 
-  space= &*it;
-  space->acquire();
-  return space;
+  return NULL;
 }
 
 /** Return the next tablespace.
@@ -1517,12 +1520,14 @@ static fil_space_t *fil_space_next(fil_space_t *space, bool recheck,
     space= UT_LIST_GET_FIRST(fil_system.space_list);
     /* We can trust that space is not NULL because at least the
     system tablespace is always present and loaded first. */
-    space->acquire();
+    if (!space->acquire())
+      goto next;
   }
   else
   {
     /* Move on to the next fil_space_t */
     space->release();
+next:
     space= UT_LIST_GET_NEXT(space_list, space);
 
     /* Skip abnormal tablespaces or those that are being created by
@@ -1532,8 +1537,8 @@ static fil_space_t *fil_space_next(fil_space_t *space, bool recheck,
             space->is_stopping() || space->purpose != FIL_TYPE_TABLESPACE))
       space= UT_LIST_GET_NEXT(space_list, space);
 
-    if (space)
-      space->acquire();
+    if (space && !space->acquire())
+      goto next;
   }
 
   mutex_exit(&fil_system.mutex);
@@ -2246,31 +2251,27 @@ static void fil_crypt_rotation_list_fill()
 	     space = UT_LIST_GET_NEXT(space_list, space)) {
 		if (space->purpose != FIL_TYPE_TABLESPACE
 		    || space->is_in_rotation_list
-		    || space->is_stopping()
-		    || UT_LIST_GET_LEN(space->chain) == 0) {
+		    || UT_LIST_GET_LEN(space->chain) == 0
+		    || !space->acquire()) {
 			continue;
 		}
 
 		/* Ensure that crypt_data has been initialized. */
 		if (!space->size) {
-			/* Protect the tablespace while we may
-			release fil_system.mutex. */
-			ut_d(space->acquire());
 			ut_d(const fil_space_t* s=)
 			        fil_system.read_page0(space->id);
 			ut_ad(!s || s == space);
-			ut_d(space->release());
 			if (!space->size) {
 				/* Page 0 was not loaded.
 				Skip this tablespace. */
-				continue;
+				goto next;
 			}
 		}
 
 		/* Skip ENCRYPTION!=DEFAULT tablespaces. */
 		if (space->crypt_data
 		    && !space->crypt_data->is_default_encryption()) {
-			continue;
+			goto next;
 		}
 
 		if (srv_encrypt_tables) {
@@ -2278,19 +2279,21 @@ static void fil_crypt_rotation_list_fill()
 			innodb_encrypt_tables!=OFF */
 			if (space->crypt_data
 			    && space->crypt_data->min_key_version) {
-				continue;
+				goto next;
 			}
 		} else {
 			/* Skip unencrypted tablespaces if
 			innodb_encrypt_tables=OFF */
 			if (!space->crypt_data
 			    || !space->crypt_data->min_key_version) {
-				continue;
+				goto next;
 			}
 		}
 
 		fil_system.rotation_list.push_back(*space);
 		space->is_in_rotation_list = true;
+next:
+		space->release();
 	}
 }
 

storage/innobase/fil/fil0fil.cc

@@ -588,8 +588,7 @@ fil_try_to_close_file_in_LRU(
 static void fil_flush_low(fil_space_t* space, bool metadata = false)
 {
 	ut_ad(mutex_own(&fil_system.mutex));
-	ut_ad(space);
-	ut_ad(!space->stop_new_ops);
+	ut_ad(!space->is_stopping());
 
 	if (srv_file_flush_method == SRV_O_DIRECT_NO_FSYNC) {
 		/* No need to flush. User has explicitly disabled
@@ -1724,10 +1723,8 @@ fil_space_t* fil_space_acquire_low(ulint id, bool silent)
 			ib::warn() << "Trying to access missing"
 				" tablespace " << id;
 		}
-	} else if (space->is_stopping()) {
+	} else if (!space->acquire()) {
 		space = NULL;
-	} else {
-		space->acquire();
 	}
 
 	mutex_exit(&fil_system.mutex);
@@ -1955,14 +1952,14 @@ static ulint fil_check_pending_ops(const fil_space_t* space, ulint count)
 {
 	ut_ad(mutex_own(&fil_system.mutex));
 
-	if (space == NULL) {
+	if (!space) {
 		return 0;
 	}
 
-	if (ulint n_pending_ops = space->n_pending_ops) {
+	if (auto n_pending_ops = space->referenced()) {
 
-          /* Give a warning every 10 second, starting after 1 second */
-          if ((count % 500) == 50) {
+		/* Give a warning every 10 second, starting after 1 second */
+		if ((count % 500) == 50) {
 			ib::warn() << "Trying to delete"
 				" tablespace '" << space->name
 				<< "' but there are " << n_pending_ops
@@ -2033,14 +2030,13 @@ fil_check_pending_operations(
 	fil_space_t* sp = fil_space_get_by_id(id);
 
 	if (sp) {
-		sp->stop_new_ops = true;
-		if (sp->crypt_data) {
-			sp->acquire();
+		if (sp->crypt_data && sp->acquire()) {
 			mutex_exit(&fil_system.mutex);
 			fil_space_crypt_close_tablespace(sp);
 			mutex_enter(&fil_system.mutex);
 			sp->release();
 		}
+		sp->set_stopping(true);
 	}
 
 	/* Check for pending operations. */
@@ -2521,7 +2517,7 @@ fil_rename_tablespace(
 	multiple datafiles per tablespace. */
 	ut_a(UT_LIST_GET_LEN(space->chain) == 1);
 	node = UT_LIST_GET_FIRST(space->chain);
-	space->n_pending_ops++;
+	ut_a(space->acquire());
 
 	mutex_exit(&fil_system.mutex);
 
@@ -2543,8 +2539,7 @@ fil_rename_tablespace(
 	/* log_sys.mutex is above fil_system.mutex in the latching order */
 	ut_ad(log_mutex_own());
 	mutex_enter(&fil_system.mutex);
-	ut_ad(space->n_pending_ops);
-	space->n_pending_ops--;
+	space->release();
 	ut_ad(space->name == old_space_name);
 	ut_ad(node->name == old_file_name);
 	bool success;
@@ -3797,7 +3792,7 @@ fil_io(
 	if (!space
 	    || (req_type.is_read()
 		&& !sync
-		&& space->stop_new_ops
+		&& space->is_stopping()
 		&& !space->is_being_truncated)) {
 
 		mutex_exit(&fil_system.mutex);
@@ -4223,7 +4218,7 @@ fil_space_validate_for_mtr_commit(
 	mini-transaction, we should have !space->stop_new_ops. This is
 	guaranteed by meta-data locks or transactional locks, or
 	dict_sys.latch (X-lock in DROP, S-lock in purge). */
-	ut_ad(!space->stop_new_ops
+	ut_ad(!space->is_stopping()
 	      || space->is_being_truncated /* fil_truncate_prepare() */
 	      || space->referenced());
 }
@@ -4430,23 +4425,3 @@ fil_space_get_block_size(const fil_space_t* space, unsigned offset)
 
 	return block_size;
 }
-
-/*******************************************************************//**
-Returns the table space by a given id, NULL if not found. */
-fil_space_t*
-fil_space_found_by_id(
-/*==================*/
-	ulint	id)	/*!< in: space id */
-{
-	fil_space_t* space = NULL;
-	mutex_enter(&fil_system.mutex);
-	space = fil_space_get_by_id(id);
-
-	/* Not found if space is being deleted */
-	if (space && space->stop_new_ops) {
-		space = NULL;
-	}
-
-	mutex_exit(&fil_system.mutex);
-	return space;
-}

storage/innobase/handler/i_s.cc

@@ -7048,8 +7048,7 @@ i_s_tablespaces_encryption_fill_table(
 	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->acquire();
+		    && space->acquire()) {
 			mutex_exit(&fil_system.mutex);
 			if (int err = i_s_dict_fill_tablespaces_encryption(
 				    thd, space, tables->table)) {

storage/innobase/include/fil0fil.h

@@ -330,14 +330,6 @@ struct fil_space_t
 				Protected by log_sys.mutex.
 				If and only if this is nonzero, the
 				tablespace will be in named_spaces. */
-	/** set when an .ibd file is about to be deleted,
-	or an undo tablespace is about to be truncated.
-	When this is set, the following new ops are not allowed:
-	* read IO request
-	* file flush
-	Note that we can still possibly have new write operations
-	because we don't check this flag when doing flush batches. */
-	bool		stop_new_ops;
 	/** whether undo tablespace truncation is in progress */
 	bool		is_being_truncated;
 	fil_type_t	purpose;/*!< purpose */
@@ -364,14 +356,22 @@ struct fil_space_t
 	ulint		n_pending_flushes; /*!< this is positive when flushing
 				the tablespace to disk; dropping of the
 				tablespace is forbidden if this is positive */
-	/** Number of pending buffer pool operations accessing the tablespace
-	without holding a table lock or dict_sys.latch S-latch
-	that would prevent the table (and tablespace) from being
-	dropped. An example is fil_crypt_thread.
-	The tablespace cannot be dropped while this is nonzero,
-	or while fil_node_t::n_pending is nonzero.
-	Protected by fil_system.mutex and std::atomic. */
-	std::atomic<ulint>		n_pending_ops;
+private:
+  /** Number of pending buffer pool operations accessing the
+  tablespace without holding a table lock or dict_operation_lock
+  S-latch that would prevent the table (and tablespace) from being
+  dropped. An example is encryption key rotation.
+
+  The tablespace cannot be dropped while this is nonzero, or while
+  fil_node_t::n_pending is nonzero.
+
+  The most significant bit contains the STOP_NEW_OPS flag. */
+  Atomic_relaxed<size_t> n_pending_ops;
+
+  /** Flag in n_pending_ops that indicates that the tablespace is being
+  deleted, and no further operations should be performed */
+  static const size_t STOP_NEW_OPS= ~(~size_t(0) >> 1);
+public:
 	/** Number of pending block read or write operations
 	(when a write is imminent or a read has recently completed).
 	The tablespace object cannot be freed while this is nonzero,
@@ -415,9 +415,6 @@ struct fil_space_t
 
 	ulint		magic_n;/*!< FIL_SPACE_MAGIC_N */
 
-	/** @return whether the tablespace is about to be dropped */
-	bool is_stopping() const { return stop_new_ops;	}
-
   /** Clamp a page number for batched I/O, such as read-ahead.
   @param offset   page number limit
   @return offset clamped to the tablespace size */
@@ -502,20 +499,45 @@ struct fil_space_t
 	/** Close each file. Only invoked on fil_system.temp_space. */
 	void close();
 
-	/** Acquire a tablespace reference. */
-	void acquire() { n_pending_ops++; }
-	/** Release a tablespace reference.
-	@return whether this was the last reference */
-	bool release() { auto n= n_pending_ops--; ut_ad(n); return n == 1; }
-	/** @return whether references are being held */
-	bool referenced() const { return n_pending_ops; }
-
-	/** Acquire a tablespace reference for I/O. */
-	void acquire_for_io() { n_pending_ios++; }
-	/** Release a tablespace reference for I/O. */
-	void release_for_io() { ut_ad(pending_io()); n_pending_ios--; }
-	/** @return whether I/O is pending */
-	bool pending_io() const { return n_pending_ios; }
+  /** @return whether the tablespace is about to be dropped */
+  bool is_stopping() const { return n_pending_ops & STOP_NEW_OPS; }
+
+  /** @return number of references being held */
+  size_t referenced() const { return n_pending_ops & ~STOP_NEW_OPS; }
+
+  /** Note that operations on the tablespace must stop or can resume */
+  void set_stopping(bool stopping)
+  {
+    ut_d(auto n=) n_pending_ops.fetch_xor(STOP_NEW_OPS);
+    ut_ad(!(n & STOP_NEW_OPS) == stopping);
+  }
+
+  MY_ATTRIBUTE((warn_unused_result))
+  /** @return whether a tablespace reference was successfully acquired */
+  bool acquire()
+  {
+    size_t n= 0;
+    while (!n_pending_ops.compare_exchange_strong(n, n + 1,
+                                                  std::memory_order_acquire,
+                                                  std::memory_order_relaxed))
+      if (UNIV_UNLIKELY(n & STOP_NEW_OPS))
+        return false;
+    return true;
+  }
+  /** Release a tablespace reference.
+  @return whether this was the last reference */
+  bool release()
+  {
+    auto n= n_pending_ops.fetch_sub(1);
+    ut_ad(n & ~STOP_NEW_OPS);
+    return (n & ~STOP_NEW_OPS) == 1;
+  }
+  /** Acquire a tablespace reference for I/O. */
+  void acquire_for_io() { n_pending_ios++; }
+  /** Release a tablespace reference for I/O. */
+  void release_for_io() { ut_d(auto n=) n_pending_ios--; ut_ad(n); }
+  /** @return whether I/O is pending */
+  bool pending_io() const { return n_pending_ios; }
 
   /** @return whether the tablespace file can be closed and reopened */
   bool belongs_in_lru() const

storage/innobase/trx/trx0purge.cc

@@ -778,12 +778,12 @@ static void trx_purge_truncate_history()
 		/* This is only executed by srv_purge_coordinator_thread. */
 		export_vars.innodb_undo_truncations++;
 
-		/* TODO: PUNCH_HOLE the garbage (with write-ahead logging) */
+		/* In MDEV-8319 (10.5) we will PUNCH_HOLE the garbage
+		(with write-ahead logging). */
 		mutex_enter(&fil_system.mutex);
 		ut_ad(&space == purge_sys.truncate.current);
-		ut_ad(space.stop_new_ops);
 		ut_ad(space.is_being_truncated);
-		purge_sys.truncate.current->stop_new_ops = false;
+		purge_sys.truncate.current->set_stopping(false);
 		purge_sys.truncate.current->is_being_truncated = false;
 		mutex_exit(&fil_system.mutex);