Commit 2b57b076 authored by Vasil Dimov's avatar Vasil Dimov

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;
parent 91702fd3
...@@ -3356,6 +3356,8 @@ btr_estimate_number_of_different_key_vals( ...@@ -3356,6 +3356,8 @@ btr_estimate_number_of_different_key_vals(
also the pages used for external storage of fields (those pages are also the pages used for external storage of fields (those pages are
included in index->stat_n_leaf_pages) */ included in index->stat_n_leaf_pages) */
dict_index_stat_mutex_enter(index);
for (j = 0; j <= n_cols; j++) { for (j = 0; j <= n_cols; j++) {
index->stat_n_diff_key_vals[j] index->stat_n_diff_key_vals[j]
= ((n_diff[j] = ((n_diff[j]
...@@ -3385,6 +3387,8 @@ btr_estimate_number_of_different_key_vals( ...@@ -3385,6 +3387,8 @@ btr_estimate_number_of_different_key_vals(
index->stat_n_diff_key_vals[j] += add_on; index->stat_n_diff_key_vals[j] += add_on;
} }
dict_index_stat_mutex_exit(index);
mem_free(n_diff); mem_free(n_diff);
if (UNIV_LIKELY_NULL(heap)) { if (UNIV_LIKELY_NULL(heap)) {
mem_heap_free(heap); mem_heap_free(heap);
......
...@@ -80,6 +80,10 @@ UNIV_INTERN rw_lock_t dict_operation_lock; ...@@ -80,6 +80,10 @@ UNIV_INTERN rw_lock_t dict_operation_lock;
/** Identifies generated InnoDB foreign key names */ /** Identifies generated InnoDB foreign key names */
static char dict_ibfk[] = "_ibfk_"; 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 Tries to find column names for the index and sets the col field of the
index. index.
...@@ -239,6 +243,33 @@ dict_mutex_exit_for_mysql(void) ...@@ -239,6 +243,33 @@ dict_mutex_exit_for_mysql(void)
mutex_exit(&(dict_sys->mutex)); 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. */ Decrements the count of open MySQL handles to a table. */
UNIV_INTERN UNIV_INTERN
...@@ -605,6 +636,8 @@ void ...@@ -605,6 +636,8 @@ void
dict_init(void) dict_init(void)
/*===========*/ /*===========*/
{ {
int i;
dict_sys = mem_alloc(sizeof(dict_sys_t)); dict_sys = mem_alloc(sizeof(dict_sys_t));
mutex_create(&dict_sys->mutex, SYNC_DICT); mutex_create(&dict_sys->mutex, SYNC_DICT);
...@@ -625,6 +658,11 @@ dict_init(void) ...@@ -625,6 +658,11 @@ dict_init(void)
ut_a(dict_foreign_err_file); ut_a(dict_foreign_err_file);
mutex_create(&dict_foreign_err_mutex, SYNC_ANY_LATCH); 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( ...@@ -4171,9 +4209,13 @@ dict_update_statistics_low(
index = dict_table_get_first_index(table); index = dict_table_get_first_index(table);
dict_index_stat_mutex_enter(index);
table->stat_n_rows = index->stat_n_diff_key_vals[ table->stat_n_rows = index->stat_n_diff_key_vals[
dict_index_get_n_unique(index)]; dict_index_get_n_unique(index)];
dict_index_stat_mutex_exit(index);
table->stat_clustered_index_size = index->stat_index_size; table->stat_clustered_index_size = index->stat_index_size;
table->stat_sum_of_other_index_sizes = sum_of_index_sizes table->stat_sum_of_other_index_sizes = sum_of_index_sizes
...@@ -4351,6 +4393,8 @@ dict_index_print_low( ...@@ -4351,6 +4393,8 @@ dict_index_print_low(
ut_ad(mutex_own(&(dict_sys->mutex))); ut_ad(mutex_own(&(dict_sys->mutex)));
dict_index_stat_mutex_enter(index);
if (index->n_user_defined_cols > 0) { if (index->n_user_defined_cols > 0) {
n_vals = index->stat_n_diff_key_vals[ n_vals = index->stat_n_diff_key_vals[
index->n_user_defined_cols]; index->n_user_defined_cols];
...@@ -4358,6 +4402,8 @@ dict_index_print_low( ...@@ -4358,6 +4402,8 @@ dict_index_print_low(
n_vals = index->stat_n_diff_key_vals[1]; n_vals = index->stat_n_diff_key_vals[1];
} }
dict_index_stat_mutex_exit(index);
if (dict_index_is_clust(index)) { if (dict_index_is_clust(index)) {
type_string = "clustered index"; type_string = "clustered index";
} else if (dict_index_is_unique(index)) { } else if (dict_index_is_unique(index)) {
......
...@@ -7597,6 +7597,8 @@ ha_innobase::info( ...@@ -7597,6 +7597,8 @@ ha_innobase::info(
break; break;
} }
dict_index_stat_mutex_enter(index);
if (index->stat_n_diff_key_vals[j + 1] == 0) { if (index->stat_n_diff_key_vals[j + 1] == 0) {
rec_per_key = stats.records; rec_per_key = stats.records;
...@@ -7605,6 +7607,8 @@ ha_innobase::info( ...@@ -7605,6 +7607,8 @@ ha_innobase::info(
index->stat_n_diff_key_vals[j + 1]); index->stat_n_diff_key_vals[j + 1]);
} }
dict_index_stat_mutex_exit(index);
/* Since MySQL seems to favor table scans /* Since MySQL seems to favor table scans
too much over index searches, we pretend too much over index searches, we pretend
index selectivity is 2 times better than index selectivity is 2 times better than
......
...@@ -1061,6 +1061,22 @@ UNIV_INTERN ...@@ -1061,6 +1061,22 @@ UNIV_INTERN
void void
dict_mutex_exit_for_mysql(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. Checks if the database name in two table names is the same.
@return TRUE if same db name */ @return TRUE if same db name */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment