- Addressed marko's review comments.bb-10.2-MDEV-19509

3 files changed, 50 insertions, 57 deletions

diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index 470c5062724..ee02ec98ab6 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -943,14 +943,10 @@ fil_crypt_read_crypt_data(fil_space_t* space)
 	mtr.commit();
 }
 
-/***********************************************************************
-Start encrypting a space
+/** Start encrypting a space
 @param[in,out]		space		Tablespace
-@return true if a recheck is needed */
-static
-bool
-fil_crypt_start_encrypting_space(
-	fil_space_t*	space)
+@return true if a recheck of tablespace is needed by encryption thread. */
+static bool fil_crypt_start_encrypting_space(fil_space_t* space)
 {
 	bool recheck = false;
 
@@ -1408,46 +1404,12 @@ fil_crypt_return_iops(
 	fil_crypt_update_total_stat(state);
 }
 
-/** Fetch the space id from key rotation list. This function depends
-whether InnoDB have to remove the tablespace from rotation list.
-@param[in]	prev_space	previous tablespace in rotation list or NULL
-@param[in]	recheck		recheck of the tablespace is needed or
-				still encryption thread does write page0 for it
-@param[in]	key_version	key version of the key state thread.
-@return next space from the rotation list. */
-static fil_space_t*
-fil_crypt_fetch_keyrotate_next(
-	fil_space_t*	prev_space,
-	bool		recheck,
-	uint		key_version)
-{
-	mutex_enter(&fil_system->mutex);
-
-	fil_space_t*	space;
-
-	/* If one of the encryption threads already started the encryption
-	of the tables then don't remove the unencrypted tablespace from
-	rotation list.
-
-	If there is a change in innod_encrypt_tables variable then don't
-	remove the last processed tablespace from rotation list. */
-
-	const bool remove = !((recheck && !prev_space->crypt_data)
-			      || (!key_version != !srv_encrypt_tables));
-
-	space = fil_space_keyrotate_next(prev_space, remove);
-	mutex_exit(&fil_system->mutex);
-	return space;
-}
-
-/***********************************************************************
-Search for a space needing rotation
-@param[in,out]		key_state		Key state
-@param[in,out]		state			Rotation state
-@param[in,out]		recheck			recheck ? */
-static
-bool
-fil_crypt_find_space_to_rotate(
+/** Search for a space needing rotation
+@param[in,out]	key_state	Key state
+@param[in,out]	state		Rotation state
+@param[in,out	recheck		recheck of the tablespace is needed or
+				still encryption thread does write page 0 */
+static bool fil_crypt_find_space_to_rotate(
 	key_state_t*		key_state,
 	rotate_thread_t*	state,
 	bool*			recheck)
@@ -1480,7 +1442,7 @@ fil_crypt_find_space_to_rotate(
 	if (srv_fil_crypt_rotate_key_age) {
 		state->space = fil_space_next(state->space);
 	} else {
-		state->space = fil_crypt_fetch_keyrotate_next(
+		state->space = fil_system->keyrotate_next(
 					state->space, *recheck,
 					key_state->key_version);
 	}
@@ -1504,7 +1466,7 @@ fil_crypt_find_space_to_rotate(
 		if (srv_fil_crypt_rotate_key_age) {
 			state->space = fil_space_next(state->space);
 		} else {
-			state->space = fil_crypt_fetch_keyrotate_next(
+			state->space = fil_system->keyrotate_next(
 					state->space, *recheck,
 					key_state->key_version);
 		}
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index d24992c1cbd..08c3a699a04 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -5985,18 +5985,30 @@ fil_space_acquire() and fil_space_release() are invoked here which
 blocks a concurrent operation from dropping the tablespace.
 @param[in]	prev_space	Previous tablespace or NULL to start
 				from beginning of fil_system->rotation list
-@param[in]	remove		Whether to remove the previous tablespace from
-				the rotation list
+@param[in]	recheck		recheck of the tablespace is needed or
+				still encryption thread does write page0 for it
+@param[in]	key_version	key version of the key state thread
 If NULL, use the first fil_space_t on fil_system->space_list.
 @return pointer to the next fil_space_t.
-@retval NULL if this was the last*/
+@retval NULL if this was the last */
 fil_space_t*
-fil_space_keyrotate_next(fil_space_t* prev_space, bool remove)
+fil_system_t::keyrotate_next(
+	fil_space_t*	prev_space,
+	bool		recheck,
+	uint		key_version)
 {
-	ut_ad(mutex_own(&fil_system->mutex));
+	mutex_enter(&fil_system->mutex);
+
+	/* If one of the encryption threads already started the encryption
+	of the table then don't remove the unencrypted spaces from
+	rotation list
+
+	If there is a change in innodb_encrypt_tables variables value then
+	don't remove the last processed tablespace from the rotation list. */
+	const bool remove = ((!recheck || prev_space->crypt_data)
+			     && (!key_version == !srv_encrypt_tables));
 
 	fil_space_t* space = prev_space;
-	fil_space_t* old   = NULL;
 
 	if (prev_space == NULL) {
 		space = UT_LIST_GET_FIRST(fil_system->rotation_list);
@@ -6009,7 +6021,6 @@ fil_space_keyrotate_next(fil_space_t* prev_space, bool remove)
 		/* Move on to the next fil_space_t */
 		space->n_pending_ops--;
 
-		old = space;
 		space = UT_LIST_GET_NEXT(rotation_list, space);
 
 		while (space != NULL
@@ -6019,7 +6030,7 @@ fil_space_keyrotate_next(fil_space_t* prev_space, bool remove)
 		}
 
 		if (remove) {
-			fil_space_remove_from_keyrotation(old);
+			fil_space_remove_from_keyrotation(prev_space);
 		}
 	}
 
@@ -6027,6 +6038,7 @@ fil_space_keyrotate_next(fil_space_t* prev_space, bool remove)
 		space->n_pending_ops++;
 	}
 
+	mutex_exit(&fil_system->mutex);
 	return(space);
 }
 
diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
index 2c6e525066d..386f574a59f 100644
--- a/storage/innobase/include/fil0fil.h
+++ b/storage/innobase/include/fil0fil.h
@@ -538,6 +538,25 @@ struct fil_system_t {
 	@return	tablespace
 	@retval	NULL	if the tablespace does not exist or cannot be read */
 	fil_space_t* read_page0(ulint id);
+
+	/** Return the next fil_space_t from key rotation list.
+	Once started, the caller must keep calling this until it returns NULL.
+	fil_space_acquire() and fil_space_release() are invoked here which
+	blocks a concurrent operation from dropping the tablespace.
+	@param[in]      prev_space      Previous tablespace or NULL to start
+					from beginning of fil_system->rotation
+					list
+	@param[in]      recheck         recheck of the tablespace is needed or
+					still encryption thread does write page0
+					for it
+	@param[in]	key_version	key version of the key state thread
+	If NULL, use the first fil_space_t on fil_system->space_list.
+	@return pointer to the next fil_space_t.
+	@retval NULL if this was the last */
+	fil_space_t* keyrotate_next(
+		fil_space_t*	prev_space,
+		bool		remove,
+		uint		key_version);
 };
 
 /** The tablespace memory cache. This variable is NULL before the module is