MDEV-25029: Reduce dict_sys mutex contention for read-only workload

In theory, read-only workload shouldn't have anything to do with dict_sys mutex. dict_sys mutex is meant to protect database metadata. But then why does dict_sys mutex shows up as part of a read-only workload? This workload needs to fetch stats for query processing and while reading these stats dict_sys mutex is taken. Is this really needed? No. For the traditional reasons, it was the default global mutex used. Based on 10.6 changes, flow can now use table->lock_mutex to protect update/access of these stats. table mutexes being table specific global contention arising out of dict_sys is reduced. Thanks to Marko Makela for his early suggestion around proposed alternative and review of the draft patch.

Author: mysqlonarm

storage/innobase/dict/dict0dict.cc

@@ -334,7 +334,9 @@ dict_table_close(
 if (last_handle && strchr(table->name.m_name, '/') != NULL
  && dict_stats_is_persistent_enabled(table)) {
 
+table->stats_mutex_lock();
 dict_stats_deinit(table);
+table->stats_mutex_unlock();
 }
 
 MONITOR_DEC(MONITOR_TABLE_REFERENCE);

storage/innobase/dict/dict0stats.cc

@@ -406,7 +406,9 @@ dict_stats_table_clone_create(
 
 dict_table_t*	t;
 
-t = (dict_table_t*) mem_heap_alloc(heap, sizeof(*t));
+t = (dict_table_t*) mem_heap_zalloc(heap, sizeof(*t));
+
+t->stats_mutex_init();
 
 MEM_CHECK_DEFINED(&table->id, sizeof(table->id));
 t->id = table->id;
@@ -434,7 +436,7 @@ dict_stats_table_clone_create(
 
 dict_index_t*	idx;
 
-idx = (dict_index_t*) mem_heap_alloc(heap, sizeof(*idx));
+idx = (dict_index_t*) mem_heap_zalloc(heap, sizeof(*idx));
 
 MEM_CHECK_DEFINED(&index->id, sizeof(index->id));
 idx->id = index->id;
@@ -452,7 +454,7 @@ dict_stats_table_clone_create(
 
 idx->n_uniq = index->n_uniq;
 
-idx->fields = (dict_field_t*) mem_heap_alloc(
+idx->fields = (dict_field_t*) mem_heap_zalloc(
 heap, idx->n_uniq * sizeof(idx->fields[0]));
 
 for (ulint i = 0; i < idx->n_uniq; i++) {
@@ -463,15 +465,15 @@ dict_stats_table_clone_create(
 /* hook idx into t->indexes */
 UT_LIST_ADD_LAST(t->indexes, idx);
 
-idx->stat_n_diff_key_vals = (ib_uint64_t*) mem_heap_alloc(
+idx->stat_n_diff_key_vals = (ib_uint64_t*) mem_heap_zalloc(
 heap,
 idx->n_uniq * sizeof(idx->stat_n_diff_key_vals[0]));
 
-idx->stat_n_sample_sizes = (ib_uint64_t*) mem_heap_alloc(
+idx->stat_n_sample_sizes = (ib_uint64_t*) mem_heap_zalloc(
 heap,
 idx->n_uniq * sizeof(idx->stat_n_sample_sizes[0]));
 
-idx->stat_n_non_null_key_vals = (ib_uint64_t*) mem_heap_alloc(
+idx->stat_n_non_null_key_vals = (ib_uint64_t*) mem_heap_zalloc(
 heap,
 idx->n_uniq * sizeof(idx->stat_n_non_null_key_vals[0]));
 ut_d(idx->magic_n = DICT_INDEX_MAGIC_N);
@@ -494,6 +496,7 @@ dict_stats_table_clone_free(
 /*========================*/
 dict_table_t*	t)/*!< in: dummy table object to free */
 {
+t->stats_mutex_destroy();
 mem_heap_free(t->heap);
 }
 
@@ -510,7 +513,7 @@ dict_stats_empty_index(
 {
 ut_ad(!(index->type & DICT_FTS));
 ut_ad(!dict_index_is_ibuf(index));
-dict_sys.assert_locked();
+ut_ad(index->table->stats_mutex_is_owner());
 
 ulint	n_uniq = index->n_uniq;
 
@@ -540,7 +543,9 @@ dict_stats_empty_table(
 boolempty_defrag_stats)
 /*!< in: whether to empty defrag stats */
 {
-dict_sys.mutex_lock();
+/* Initialize table/index level stats is now protected by
+table level lock_mutex.*/
+table->stats_mutex_lock();
 
 /* Zero the stats members */
 table->stat_n_rows = 0;
@@ -566,7 +571,7 @@ dict_stats_empty_table(
 }
 
 table->stat_initialized = TRUE;
-dict_sys.mutex_unlock();
+table->stats_mutex_unlock();
 }
 
 /*********************************************************************//**
@@ -665,7 +670,8 @@ dict_stats_copy(
  to have the same statistics as if
  the table was empty */
 {
-dict_sys.assert_locked();
+ut_ad(src->stats_mutex_is_owner());
+ut_ad(dst->stats_mutex_is_owner());
 
 dst->stats_last_recalc = src->stats_last_recalc;
 dst->stat_n_rows = src->stat_n_rows;
@@ -790,8 +796,14 @@ dict_stats_snapshot_create(
 
 t = dict_stats_table_clone_create(table);
 
+table->stats_mutex_lock();
+ut_d(t->stats_mutex_lock());
+
 dict_stats_copy(t, table, false);
 
+ut_d(t->stats_mutex_unlock());
+table->stats_mutex_unlock();
+
 t->stat_persistent = table->stat_persistent;
 t->stats_auto_recalc = table->stats_auto_recalc;
 t->stats_sample_pages = table->stats_sample_pages;
@@ -836,14 +848,14 @@ dict_stats_update_transient_for_index(
 Initialize some bogus index cardinality
 statistics, so that the data can be queried in
 various means, also via secondary indexes. */
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 dict_stats_empty_index(index, false);
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 #if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
 } else if (ibuf_debug && !dict_index_is_clust(index)) {
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 dict_stats_empty_index(index, false);
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 #endif /* UNIV_DEBUG || UNIV_IBUF_DEBUG */
 } else {
 mtr_tmtr;
@@ -864,9 +876,9 @@ dict_stats_update_transient_for_index(
 
 switch (size) {
 case ULINT_UNDEFINED:
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 dict_stats_empty_index(index, false);
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 return;
 case 0:
 /* The root node of the tree is a leaf */
@@ -883,7 +895,7 @@ dict_stats_update_transient_for_index(
 index);
 
 if (!stats.empty()) {
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 for (size_t i = 0; i < stats.size(); ++i) {
 index->stat_n_diff_key_vals[i]
 = stats[i].n_diff_key_vals;
@@ -892,7 +904,7 @@ dict_stats_update_transient_for_index(
 index->stat_n_non_null_key_vals[i]
 = stats[i].n_non_null_key_vals;
 }
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 }
 }
 }
@@ -910,7 +922,7 @@ dict_stats_update_transient(
 /*========================*/
 dict_table_t*	table)/*!< in/out: table */
 {
-dict_sys.assert_not_locked();
+ut_ad(!table->stats_mutex_is_owner());
 
 dict_index_t*	index;
 ulint	sum_of_index_sizes	= 0;
@@ -943,9 +955,9 @@ dict_stats_update_transient(
 
 if (dict_stats_should_ignore_index(index)
  || !index->is_readable()) {
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 dict_stats_empty_index(index, false);
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 continue;
 }
 
@@ -954,7 +966,7 @@ dict_stats_update_transient(
 sum_of_index_sizes += index->stat_index_size;
 }
 
-dict_sys.mutex_lock();
+table->stats_mutex_lock();
 
 index = dict_table_get_first_index(table);
 
@@ -972,7 +984,7 @@ dict_stats_update_transient(
 
 table->stat_initialized = TRUE;
 
-dict_sys.mutex_unlock();
+table->stats_mutex_unlock();
 }
 
 /* @{ Pseudo code about the relation between the following functions
@@ -1930,7 +1942,7 @@ static index_stats_t dict_stats_analyze_index(dict_index_t* index)
 DBUG_PRINT("info", ("index: %s, online status: %d", index->name(),
  dict_index_get_online_status(index)));
 
-dict_sys.assert_not_locked(); // this function is slow
+ut_ad(!index->table->stats_mutex_is_owner());
 ut_ad(index->table->get_ref_count());
 
 /* Disable update statistic for Rtree */
@@ -2002,14 +2014,14 @@ static index_stats_t dict_stats_analyze_index(dict_index_t* index)
 
 mtr.commit();
 
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 for (ulint i = 0; i < n_uniq; i++) {
 result.stats[i].n_diff_key_vals = index->stat_n_diff_key_vals[i];
 result.stats[i].n_sample_sizes = total_pages;
 result.stats[i].n_non_null_key_vals = index->stat_n_non_null_key_vals[i];
 }
 result.n_leaf_pages = index->stat_n_leaf_pages;
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 
 DBUG_RETURN(result);
 }
@@ -2243,13 +2255,13 @@ dict_stats_update_persistent(
 }
 
 ut_ad(!dict_index_is_ibuf(index));
-dict_sys.mutex_lock();
+table->stats_mutex_lock();
 dict_stats_empty_index(index, false);
-dict_sys.mutex_unlock();
+table->stats_mutex_unlock();
 
 index_stats_t stats = dict_stats_analyze_index(index);
 
-dict_sys.mutex_lock();
+table->stats_mutex_lock();
 index->stat_index_size = stats.index_size;
 index->stat_n_leaf_pages = stats.n_leaf_pages;
 for (size_t i = 0; i < stats.stats.size(); ++i) {
@@ -2285,9 +2297,9 @@ dict_stats_update_persistent(
 }
 
 if (!(table->stats_bg_flag & BG_STAT_SHOULD_QUIT)) {
-dict_sys.mutex_unlock();
+table->stats_mutex_unlock();
 stats = dict_stats_analyze_index(index);
-dict_sys.mutex_lock();
+table->stats_mutex_lock();
 
 index->stat_index_size = stats.index_size;
 index->stat_n_leaf_pages = stats.n_leaf_pages;
@@ -2313,7 +2325,7 @@ dict_stats_update_persistent(
 
 dict_stats_assert_initialized(table);
 
-dict_sys.mutex_unlock();
+table->stats_mutex_unlock();
 
 return(DB_SUCCESS);
 }
@@ -3123,13 +3135,11 @@ dict_stats_update_for_index(
 {
 DBUG_ENTER("dict_stats_update_for_index");
 
-dict_sys.assert_not_locked();
-
 if (dict_stats_is_persistent_enabled(index->table)) {
 
 if (dict_stats_persistent_storage_check(false)) {
 index_stats_t stats = dict_stats_analyze_index(index);
-dict_sys.mutex_lock();
+index->table->stats_mutex_lock();
 index->stat_index_size = stats.index_size;
 index->stat_n_leaf_pages = stats.n_leaf_pages;
 for (size_t i = 0; i < stats.stats.size(); ++i) {
@@ -3142,7 +3152,7 @@ dict_stats_update_for_index(
 }
 index->table->stat_sum_of_other_index_sizes
 += index->stat_index_size;
-dict_sys.mutex_unlock();
+index->table->stats_mutex_unlock();
 
 dict_stats_save(index->table, &index->id);
 DBUG_VOID_RETURN;
@@ -3183,7 +3193,7 @@ dict_stats_update(
 the persistent statistics
 storage */
 {
-dict_sys.assert_not_locked();
+ut_ad(!table->stats_mutex_is_owner());
 
 if (!table->is_readable()) {
 return (dict_stats_report_error(table));
@@ -3318,7 +3328,10 @@ dict_stats_update(
 switch (err) {
 case DB_SUCCESS:
 
-dict_sys.mutex_lock();
+table->stats_mutex_lock();
+/* t is localized to this thread so no need to
+take stats mutex lock (limiting it to debug only) */
+ut_d(t->stats_mutex_lock());
 
 /* Pass reset_ignored_indexes=true as parameter
 to dict_stats_copy. This will cause statictics
@@ -3327,7 +3340,8 @@ dict_stats_update(
 
 dict_stats_assert_initialized(table);
 
-dict_sys.mutex_unlock();
+ut_d(t->stats_mutex_unlock());
+table->stats_mutex_unlock();
 
 dict_stats_table_clone_free(t);
 

storage/innobase/handler/ha_innodb.cc

@@ -14044,7 +14044,7 @@ ha_innobase::info_low(
 ulint	stat_clustered_index_size;
 ulint	stat_sum_of_other_index_sizes;
 
-dict_sys.mutex_lock();
+ib_table->stats_mutex_lock();
 
 ut_a(ib_table->stat_initialized);
 
@@ -14056,7 +14056,7 @@ ha_innobase::info_low(
 stat_sum_of_other_index_sizes
 = ib_table->stat_sum_of_other_index_sizes;
 
-dict_sys.mutex_unlock();
+ib_table->stats_mutex_unlock();
 
 /*
 The MySQL optimizer seems to assume in a left join that n_rows
@@ -14172,10 +14172,9 @@ ha_innobase::info_low(
 stats.create_time = (ulong) stat_info.ctime;
 }
 
-struct Locking {
-Locking() { dict_sys.mutex_lock(); }
-~Locking() { dict_sys.mutex_unlock(); }
-} locking;
+ib_table->stats_mutex_lock();
+auto _ = make_scope_exit([ib_table]() {
+ib_table->stats_mutex_unlock(); });
 
 ut_a(ib_table->stat_initialized);
 

storage/innobase/handler/i_s.cc

@@ -56,6 +56,7 @@ Created July 18, 2007 Vasil Dimov
 #include "fil0fil.h"
 #include "fil0crypt.h"
 #include "dict0crea.h"
+#include "scope.h"
 
 /** The latest successfully looked up innodb_fts_aux_table */
 UNIV_INTERN table_id_t innodb_ft_aux_table_id;
@@ -5021,11 +5022,9 @@ i_s_dict_fill_sys_tablestats(
  table->name.m_name));
 
 {
-struct Locking
-{
-Locking() { dict_sys.mutex_lock(); }
-~Locking() { dict_sys.mutex_unlock(); }
-} locking;
+table->stats_mutex_lock();
+auto _ = make_scope_exit([table]() {
+table->stats_mutex_unlock(); });
 
 OK(fields[SYS_TABLESTATS_INIT]->store(table->stat_initialized,
  true));