From 2b57b0765db00497a6c02c14a69f8535c9b986d9 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 28 Apr 2010 11:46:27 +0300 Subject: [PATCH] Fix Bug#53046 dict_update_statistics_low can still be run concurrently on same table Protect dict_index_t::stat_n_diff_key_vals[] with an array of mutexes. Testing: tested all code paths under UNIV_SYNC_DEBUG for the one in dict_print() one has to enable the InnoDB table monitor: CREATE TABLE innodb_table_monitor (a int) ENGINE=INNODB; --- storage/innodb_plugin/btr/btr0cur.c | 4 ++ storage/innodb_plugin/dict/dict0dict.c | 46 ++++++++++++++++++++++ storage/innodb_plugin/handler/ha_innodb.cc | 4 ++ storage/innodb_plugin/include/dict0dict.h | 16 ++++++++ 4 files changed, 70 insertions(+) diff --git a/storage/innodb_plugin/btr/btr0cur.c b/storage/innodb_plugin/btr/btr0cur.c index aa97869fe6f..0e603fdca8f 100644 --- a/storage/innodb_plugin/btr/btr0cur.c +++ b/storage/innodb_plugin/btr/btr0cur.c @@ -3356,6 +3356,8 @@ 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) */ + dict_index_stat_mutex_enter(index); + for (j = 0; j <= n_cols; j++) { index->stat_n_diff_key_vals[j] = ((n_diff[j] @@ -3385,6 +3387,8 @@ btr_estimate_number_of_different_key_vals( index->stat_n_diff_key_vals[j] += add_on; } + dict_index_stat_mutex_exit(index); + mem_free(n_diff); if (UNIV_LIKELY_NULL(heap)) { mem_heap_free(heap); diff --git a/storage/innodb_plugin/dict/dict0dict.c b/storage/innodb_plugin/dict/dict0dict.c index 95f3ad9134c..88742cddef8 100644 --- a/storage/innodb_plugin/dict/dict0dict.c +++ b/storage/innodb_plugin/dict/dict0dict.c @@ -80,6 +80,10 @@ UNIV_INTERN rw_lock_t dict_operation_lock; /** Identifies generated InnoDB foreign key names */ static char dict_ibfk[] = "_ibfk_"; +/* array of mutexes protecting dict_index_t::stat_n_diff_key_vals[] */ +#define DICT_INDEX_STAT_MUTEX_SIZE 32 +mutex_t dict_index_stat_mutex[DICT_INDEX_STAT_MUTEX_SIZE]; + /*******************************************************************//** Tries to find column names for the index and sets the col field of the index. @@ -239,6 +243,33 @@ dict_mutex_exit_for_mysql(void) mutex_exit(&(dict_sys->mutex)); } +#define FOLD_INDEX_FOR_STAT_MUTEX(index) \ + (ut_fold_dulint(index->id) % DICT_INDEX_STAT_MUTEX_SIZE) + +/**********************************************************************//** +Lock the appropriate mutex to protect index->stat_n_diff_key_vals[]. +index->id is used to pick the right mutex and it should not change +before dict_index_stat_mutex_exit() is called on this index. */ +UNIV_INTERN +void +dict_index_stat_mutex_enter( +/*========================*/ + const dict_index_t* index) /*!< in: index */ +{ + mutex_enter(&dict_index_stat_mutex[FOLD_INDEX_FOR_STAT_MUTEX(index)]); +} + +/**********************************************************************//** +Unlock the appropriate mutex that protects index->stat_n_diff_key_vals[]. */ +UNIV_INTERN +void +dict_index_stat_mutex_exit( +/*=======================*/ + const dict_index_t* index) /*!< in: index */ +{ + mutex_exit(&dict_index_stat_mutex[FOLD_INDEX_FOR_STAT_MUTEX(index)]); +} + /********************************************************************//** Decrements the count of open MySQL handles to a table. */ UNIV_INTERN @@ -605,6 +636,8 @@ void dict_init(void) /*===========*/ { + int i; + dict_sys = mem_alloc(sizeof(dict_sys_t)); mutex_create(&dict_sys->mutex, SYNC_DICT); @@ -625,6 +658,11 @@ dict_init(void) ut_a(dict_foreign_err_file); mutex_create(&dict_foreign_err_mutex, SYNC_ANY_LATCH); + + for (i = 0; i < DICT_INDEX_STAT_MUTEX_SIZE; i++) { + mutex_create(&dict_index_stat_mutex[i], + /* XXX */ SYNC_INDEX_TREE); + } } /**********************************************************************//** @@ -4171,9 +4209,13 @@ dict_update_statistics_low( index = dict_table_get_first_index(table); + dict_index_stat_mutex_enter(index); + table->stat_n_rows = index->stat_n_diff_key_vals[ dict_index_get_n_unique(index)]; + dict_index_stat_mutex_exit(index); + table->stat_clustered_index_size = index->stat_index_size; table->stat_sum_of_other_index_sizes = sum_of_index_sizes @@ -4351,6 +4393,8 @@ dict_index_print_low( ut_ad(mutex_own(&(dict_sys->mutex))); + dict_index_stat_mutex_enter(index); + if (index->n_user_defined_cols > 0) { n_vals = index->stat_n_diff_key_vals[ index->n_user_defined_cols]; @@ -4358,6 +4402,8 @@ dict_index_print_low( n_vals = index->stat_n_diff_key_vals[1]; } + dict_index_stat_mutex_exit(index); + if (dict_index_is_clust(index)) { type_string = "clustered index"; } else if (dict_index_is_unique(index)) { diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index dcb44b11ad6..766ce42ec34 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -7597,6 +7597,8 @@ ha_innobase::info( break; } + dict_index_stat_mutex_enter(index); + if (index->stat_n_diff_key_vals[j + 1] == 0) { rec_per_key = stats.records; @@ -7605,6 +7607,8 @@ ha_innobase::info( index->stat_n_diff_key_vals[j + 1]); } + dict_index_stat_mutex_exit(index); + /* Since MySQL seems to favor table scans too much over index searches, we pretend index selectivity is 2 times better than diff --git a/storage/innodb_plugin/include/dict0dict.h b/storage/innodb_plugin/include/dict0dict.h index 788616d682a..79dcbb30de2 100644 --- a/storage/innodb_plugin/include/dict0dict.h +++ b/storage/innodb_plugin/include/dict0dict.h @@ -1061,6 +1061,22 @@ UNIV_INTERN void dict_mutex_exit_for_mysql(void); /*===========================*/ +/**********************************************************************//** +Lock the appropriate mutex to protect index->stat_n_diff_key_vals[]. +index->id is used to pick the right mutex and it should not change +before dict_index_stat_mutex_exit() is called on this index. */ +UNIV_INTERN +void +dict_index_stat_mutex_enter( +/*========================*/ + const dict_index_t* index); /*!< in: index */ +/**********************************************************************//** +Unlock the appropriate mutex that protects index->stat_n_diff_key_vals[]. */ +UNIV_INTERN +void +dict_index_stat_mutex_exit( +/*=======================*/ + const dict_index_t* index); /*!< in: index */ /********************************************************************//** Checks if the database name in two table names is the same. @return TRUE if same db name */