MDEV-23991 dict_table_stats_lock() has unnecessarily long scope

Patch removes dict_index_t::stats_latch. Table/index statistics now
protected with dict_sys->mutex. That way statistics computation can
happen in parallel in several threads and dict_sys->mutex will be locked
only for a short period of time.

This patch is a joint work with Marko Mäkelä

dict_index_t:🔒 make mutable which allows to pass const pointer
when only lock is touched in an object

btr_height_get()
btr_get_size(): make index argument const for better type safety

btr_estimate_number_of_different_key_vals(): now returns computed values
instead of setting fields in dict_index_t directly

remove everything related to dict_index_t::stats_latch

dict_stats_index_set_n_diff(): now returns computed values instead
of setting fields in dict_index_t directly

dict_stats_analyze_index():  now returns computed values instead
of setting fields in dict_index_t directly

Reviewed by: Marko Mäkelä

mysql-test/suite/innodb/r/stats_persistent.result

@@ -0,0 +1,18 @@
+#
+# MDEV-23991 dict_table_stats_lock() has unnecessarily long scope
+#
+CREATE TABLE t1(a INT) ENGINE=INNODB STATS_PERSISTENT=1;
+SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go';
+ANALYZE TABLE t1;
+connect con1, localhost, root;
+SET DEBUG_SYNC='now WAIT_FOR stop';
+SELECT ENGINE,SUM(DATA_LENGTH+INDEX_LENGTH),COUNT(ENGINE),SUM(DATA_LENGTH),SUM(INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB';
+ENGINE	SUM(DATA_LENGTH+INDEX_LENGTH)	COUNT(ENGINE)	SUM(DATA_LENGTH)	SUM(INDEX_LENGTH)
+InnoDB	49152	3	49152	0
+SET DEBUG_SYNC='now SIGNAL go';
+disconnect con1;
+connection default;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+SET DEBUG_SYNC= 'RESET';
+DROP TABLE t1;

mysql-test/suite/innodb/t/stats_persistent.test

@@ -0,0 +1,27 @@
+--source include/have_innodb.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+--source include/count_sessions.inc
+
+--echo #
+--echo # MDEV-23991 dict_table_stats_lock() has unnecessarily long scope
+--echo #
+CREATE TABLE t1(a INT) ENGINE=INNODB STATS_PERSISTENT=1;
+
+SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go';
+--send ANALYZE TABLE t1
+
+--connect(con1, localhost, root)
+SET DEBUG_SYNC='now WAIT_FOR stop';
+
+SELECT ENGINE,SUM(DATA_LENGTH+INDEX_LENGTH),COUNT(ENGINE),SUM(DATA_LENGTH),SUM(INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB';
+
+SET DEBUG_SYNC='now SIGNAL go';
+--disconnect con1
+
+--connection default
+--reap
+SET DEBUG_SYNC= 'RESET';
+DROP TABLE t1;
+
+--source include/wait_until_count_sessions.inc

storage/innobase/btr/btr0btr.cc

@@ -283,7 +283,7 @@ the index.
 ulint
 btr_height_get(
 /*===========*/
-	dict_index_t*	index,	/*!< in: index tree */
+	const dict_index_t*	index,	/*!< in: index tree */
 	mtr_t*		mtr)	/*!< in/out: mini-transaction */
 {
 	ulint		height=0;
@@ -592,7 +592,7 @@ Gets the number of pages in a B-tree.
 ulint
 btr_get_size(
 /*=========*/
-	dict_index_t*	index,	/*!< in: index */
+	const dict_index_t*	index,	/*!< in: index */
 	ulint		flag,	/*!< in: BTR_N_LEAF_PAGES or BTR_TOTAL_SIZE */
 	mtr_t*		mtr)	/*!< in/out: mini-transaction where index
 				is s-latched */

storage/innobase/btr/btr0cur.cc

@@ -6133,21 +6133,19 @@ btr_record_not_null_field_in_rec(
 	}
 }
 
-/*******************************************************************//**
-Estimates the number of different key values in a given index, for
+/** Estimates the number of different key values in a given index, for
 each n-column prefix of the index where 1 <= n <= dict_index_get_n_unique(index).
 The estimates are stored in the array index->stat_n_diff_key_vals[] (indexed
 0..n_uniq-1) and the number of pages that were sampled is saved in
-index->stat_n_sample_sizes[].
+result.n_sample_sizes[].
 If innodb_stats_method is nulls_ignored, we also record the number of
 non-null values for each prefix and stored the estimates in
-array index->stat_n_non_null_key_vals.
-@return true if the index is available and we get the estimated numbers,
-false if the index is unavailable. */
-bool
-btr_estimate_number_of_different_key_vals(
-/*======================================*/
-	dict_index_t*	index)	/*!< in: index */
+array result.n_non_null_key_vals.
+@param[in]	index	index
+@return vector with statistics information
+empty vector if the index is unavailable. */
+std::vector<index_field_stats_t>
+btr_estimate_number_of_different_key_vals(dict_index_t* index)
 {
 	btr_cur_t	cursor;
 	page_t*		page;
@@ -6167,11 +6165,11 @@ btr_estimate_number_of_different_key_vals(
 	rec_offs*	offsets_rec	= NULL;
 	rec_offs*	offsets_next_rec = NULL;
 
+	std::vector<index_field_stats_t> result;
+
 	/* For spatial index, there is no such stats can be
 	fetched. */
-	if (dict_index_is_spatial(index)) {
-		return(false);
-	}
+	ut_ad(!dict_index_is_spatial(index));
 
 	n_cols = dict_index_get_n_unique(index);
 
@@ -6280,7 +6278,7 @@ btr_estimate_number_of_different_key_vals(
 			mtr_commit(&mtr);
 			mem_heap_free(heap);
 
-			return(false);
+			return result;
 		}
 
 		/* Count the number of different key values for each prefix of
@@ -6386,8 +6384,12 @@ btr_estimate_number_of_different_key_vals(
 	also the pages used for external storage of fields (those pages are
 	included in index->stat_n_leaf_pages) */
 
+	result.reserve(n_cols);
+
 	for (j = 0; j < n_cols; j++) {
-		index->stat_n_diff_key_vals[j]
+		index_field_stats_t stat;
+
+		stat.n_diff_key_vals
 			= BTR_TABLE_STATS_FROM_SAMPLE(
 				n_diff[j], index, n_sample_pages,
 				total_external_size, not_empty_flag);
@@ -6408,25 +6410,23 @@ btr_estimate_number_of_different_key_vals(
 			add_on = n_sample_pages;
 		}
 
-		index->stat_n_diff_key_vals[j] += add_on;
+		stat.n_diff_key_vals += add_on;
 
-		index->stat_n_sample_sizes[j] = n_sample_pages;
+		stat.n_sample_sizes = n_sample_pages;
 
-		/* Update the stat_n_non_null_key_vals[] with our
-		sampled result. stat_n_non_null_key_vals[] is created
-		and initialized to zero in dict_index_add_to_cache(),
-		along with stat_n_diff_key_vals[] array */
 		if (n_not_null != NULL) {
-			index->stat_n_non_null_key_vals[j] =
+			stat.n_non_null_key_vals =
 				 BTR_TABLE_STATS_FROM_SAMPLE(
 					n_not_null[j], index, n_sample_pages,
 					total_external_size, not_empty_flag);
 		}
+
+		result.push_back(stat);
 	}
 
 	mem_heap_free(heap);
 
-	return(true);
+	return result;
 }
 
 /*================== EXTERNAL STORAGE OF BIG FIELDS ===================*/

storage/innobase/dict/dict0dict.cc

@@ -267,56 +267,6 @@ dict_mutex_exit_for_mysql(void)
 	mutex_exit(&dict_sys->mutex);
 }
 
-/** Lock the appropriate latch to protect a given table's statistics.
-@param[in]	table		table whose stats to lock
-@param[in]	latch_mode	RW_S_LATCH or RW_X_LATCH */
-void
-dict_table_stats_lock(
-	dict_table_t*	table,
-	ulint		latch_mode)
-{
-	ut_ad(table != NULL);
-	ut_ad(table->magic_n == DICT_TABLE_MAGIC_N);
-
-	switch (latch_mode) {
-	case RW_S_LATCH:
-		rw_lock_s_lock(&table->stats_latch);
-		break;
-	case RW_X_LATCH:
-		rw_lock_x_lock(&table->stats_latch);
-		break;
-	case RW_NO_LATCH:
-		/* fall through */
-	default:
-		ut_error;
-	}
-}
-
-/** Unlock the latch that has been locked by dict_table_stats_lock().
-@param[in]	table		table whose stats to unlock
-@param[in]	latch_mode	RW_S_LATCH or RW_X_LATCH */
-void
-dict_table_stats_unlock(
-	dict_table_t*	table,
-	ulint		latch_mode)
-{
-	ut_ad(table != NULL);
-	ut_ad(table->magic_n == DICT_TABLE_MAGIC_N);
-
-	switch (latch_mode) {
-	case RW_S_LATCH:
-		rw_lock_s_unlock(&table->stats_latch);
-		break;
-	case RW_X_LATCH:
-		rw_lock_x_unlock(&table->stats_latch);
-		break;
-	case RW_NO_LATCH:
-		/* fall through */
-	default:
-		ut_error;
-	}
-}
-
 /**********************************************************************//**
 Try to drop any indexes after an aborted index creation.
 This can also be after a server kill during DROP INDEX. */

storage/innobase/dict/dict0mem.cc

@@ -125,8 +125,7 @@ dict_mem_table_create(
 	ulint		n_cols,
 	ulint		n_v_cols,
 	ulint		flags,
-	ulint		flags2,
-	bool		init_stats_latch)
+	ulint		flags2)
 {
 	dict_table_t*	table;
 	mem_heap_t*	heap;
@@ -182,12 +181,6 @@ dict_mem_table_create(
 	new(&table->foreign_set) dict_foreign_set();
 	new(&table->referenced_set) dict_foreign_set();
 
-	if (init_stats_latch) {
-		rw_lock_create(dict_table_stats_key, &table->stats_latch,
-			       SYNC_INDEX_TREE);
-		table->stats_latch_inited = true;
-	}
-
 	return(table);
 }
 
@@ -237,10 +230,6 @@ dict_mem_table_free(
 		UT_DELETE(table->s_cols);
 	}
 
-	if (table->stats_latch_inited) {
-		rw_lock_free(&table->stats_latch);
-	}
-
 	mem_heap_free(table->heap);
 }