summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--mysql-test/suite/innodb/r/innodb_bug53046.result27
-rw-r--r--mysql-test/suite/innodb/t/innodb_bug53046.test48
-rw-r--r--storage/innobase/btr/btr0cur.c4
-rw-r--r--storage/innobase/dict/dict0dict.c142
-rw-r--r--storage/innobase/dict/dict0load.c6
-rw-r--r--storage/innobase/handler/ha_innodb.cc24
-rw-r--r--storage/innobase/include/dict0dict.h34
-rw-r--r--storage/innobase/row/row0mysql.c6
8 files changed, 217 insertions, 74 deletions
diff --git a/mysql-test/suite/innodb/r/innodb_bug53046.result b/mysql-test/suite/innodb/r/innodb_bug53046.result
new file mode 100644
index 00000000000..69be6c4e0a7
--- /dev/null
+++ b/mysql-test/suite/innodb/r/innodb_bug53046.result
@@ -0,0 +1,27 @@
+CREATE TABLE bug53046_1 (c1 INT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE bug53046_2 (c2 INT PRIMARY KEY,
+FOREIGN KEY (c2) REFERENCES bug53046_1(c1)
+ON UPDATE CASCADE ON DELETE CASCADE) ENGINE=INNODB;
+INSERT INTO bug53046_1 VALUES (1);
+INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+FROM bug53046_1;
+INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+FROM bug53046_1;
+INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+FROM bug53046_1;
+INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+FROM bug53046_1;
+INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+FROM bug53046_1;
+INSERT INTO bug53046_2 VALUES (1), (2);
+ANALYZE TABLE bug53046_1;
+Table Op Msg_type Msg_text
+test.bug53046_1 analyze status OK
+SHOW TABLE STATUS LIKE 'bug53046_1';
+UPDATE bug53046_1 SET c1 = c1 - 1;
+DELETE FROM bug53046_1;
+INSERT INTO bug53046_1 VALUES (1);
+INSERT INTO bug53046_2 VALUES (1);
+TRUNCATE TABLE bug53046_2;
+DROP TABLE bug53046_2;
+DROP TABLE bug53046_1;
diff --git a/mysql-test/suite/innodb/t/innodb_bug53046.test b/mysql-test/suite/innodb/t/innodb_bug53046.test
new file mode 100644
index 00000000000..77f0a638728
--- /dev/null
+++ b/mysql-test/suite/innodb/t/innodb_bug53046.test
@@ -0,0 +1,48 @@
+#
+# http://bugs.mysql.com/53046
+# dict_update_statistics_low can still be run concurrently on same table
+#
+# This is a symbolic test, it would not fail if the bug is present.
+# Rather those SQL commands have been used during manual testing under
+# UNIV_DEBUG & UNIV_SYNC_DEBUG to test all changed codepaths for locking
+# correctness.
+#
+
+-- source include/have_innodb.inc
+
+CREATE TABLE bug53046_1 (c1 INT PRIMARY KEY) ENGINE=INNODB;
+CREATE TABLE bug53046_2 (c2 INT PRIMARY KEY,
+ FOREIGN KEY (c2) REFERENCES bug53046_1(c1)
+ ON UPDATE CASCADE ON DELETE CASCADE) ENGINE=INNODB;
+
+INSERT INTO bug53046_1 VALUES (1);
+let $i = 5;
+while ($i) {
+ eval INSERT INTO bug53046_1 SELECT c1+(SELECT MAX(c1) FROM bug53046_1)
+ FROM bug53046_1;
+ dec $i;
+}
+
+INSERT INTO bug53046_2 VALUES (1), (2);
+
+# CREATE TABLE innodb_table_monitor (a int) ENGINE=INNODB;
+# wait more than 1 minute and observe the mysqld output
+# DROP TABLE innodb_table_monitor;
+
+ANALYZE TABLE bug53046_1;
+
+# this prints create time and other nondeterministic data
+-- disable_result_log
+SHOW TABLE STATUS LIKE 'bug53046_1';
+-- enable_result_log
+
+UPDATE bug53046_1 SET c1 = c1 - 1;
+
+DELETE FROM bug53046_1;
+
+INSERT INTO bug53046_1 VALUES (1);
+INSERT INTO bug53046_2 VALUES (1);
+TRUNCATE TABLE bug53046_2;
+
+DROP TABLE bug53046_2;
+DROP TABLE bug53046_1;
diff --git a/storage/innobase/btr/btr0cur.c b/storage/innobase/btr/btr0cur.c
index 378b9020c0d..b8668abf321 100644
--- a/storage/innobase/btr/btr0cur.c
+++ b/storage/innobase/btr/btr0cur.c
@@ -3633,8 +3633,6 @@ 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]
@@ -3664,8 +3662,6 @@ 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/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c
index 2512f199875..f9bf08a0675 100644
--- a/storage/innobase/dict/dict0dict.c
+++ b/storage/innobase/dict/dict0dict.c
@@ -91,9 +91,18 @@ UNIV_INTERN mysql_pfs_key_t dict_foreign_err_mutex_key;
/** 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];
+/** array of rw locks protecting
+dict_table_t::stat_initialized
+dict_table_t::stat_n_rows (*)
+dict_table_t::stat_clustered_index_size
+dict_table_t::stat_sum_of_other_index_sizes
+dict_table_t::stat_modified_counter (*)
+dict_table_t::indexes*::stat_n_diff_key_vals[]
+dict_table_t::indexes*::stat_index_size
+dict_table_t::indexes*::stat_n_leaf_pages
+(*) those are not always protected for performance reasons */
+#define DICT_TABLE_STATS_LATCHES_SIZE 64
+static rw_lock_t dict_table_stats_latches[DICT_TABLE_STATS_LATCHES_SIZE];
/*******************************************************************//**
Tries to find column names for the index and sets the col field of the
@@ -254,43 +263,65 @@ dict_mutex_exit_for_mysql(void)
mutex_exit(&(dict_sys->mutex));
}
-/** Get the mutex that protects index->stat_n_diff_key_vals[] */
-#define GET_INDEX_STAT_MUTEX(index) \
- (&dict_index_stat_mutex[ut_fold_ull(index->id) \
- % DICT_INDEX_STAT_MUTEX_SIZE])
+/** Get the latch that protects the stats of a given table */
+#define GET_TABLE_STATS_LATCH(table) \
+ (&dict_table_stats_latches[ut_fold_ull(table->id) \
+ % DICT_TABLE_STATS_LATCHES_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. */
+Lock the appropriate latch to protect a given table's statistics.
+table->id is used to pick the corresponding latch from a global array of
+latches. */
UNIV_INTERN
void
-dict_index_stat_mutex_enter(
-/*========================*/
- const dict_index_t* index) /*!< in: index */
+dict_table_stats_lock(
+/*==================*/
+ const dict_table_t* table, /*!< in: table */
+ ulint latch_mode) /*!< in: RW_S_LATCH or
+ RW_X_LATCH */
{
- ut_ad(index != NULL);
- ut_ad(index->magic_n == DICT_INDEX_MAGIC_N);
- ut_ad(index->cached);
- ut_ad(!index->to_be_dropped);
+ ut_ad(table != NULL);
+ ut_ad(table->magic_n == DICT_TABLE_MAGIC_N);
- mutex_enter(GET_INDEX_STAT_MUTEX(index));
+ switch (latch_mode) {
+ case RW_S_LATCH:
+ rw_lock_s_lock(GET_TABLE_STATS_LATCH(table));
+ break;
+ case RW_X_LATCH:
+ rw_lock_x_lock(GET_TABLE_STATS_LATCH(table));
+ break;
+ case RW_NO_LATCH:
+ /* fall through */
+ default:
+ ut_error;
+ }
}
/**********************************************************************//**
-Unlock the appropriate mutex that protects index->stat_n_diff_key_vals[]. */
+Unlock the latch that has been locked by dict_table_stats_lock() */
UNIV_INTERN
void
-dict_index_stat_mutex_exit(
-/*=======================*/
- const dict_index_t* index) /*!< in: index */
+dict_table_stats_unlock(
+/*====================*/
+ const dict_table_t* table, /*!< in: table */
+ ulint latch_mode) /*!< in: RW_S_LATCH or
+ RW_X_LATCH */
{
- ut_ad(index != NULL);
- ut_ad(index->magic_n == DICT_INDEX_MAGIC_N);
- ut_ad(index->cached);
- ut_ad(!index->to_be_dropped);
+ ut_ad(table != NULL);
+ ut_ad(table->magic_n == DICT_TABLE_MAGIC_N);
- mutex_exit(GET_INDEX_STAT_MUTEX(index));
+ switch (latch_mode) {
+ case RW_S_LATCH:
+ rw_lock_s_unlock(GET_TABLE_STATS_LATCH(table));
+ break;
+ case RW_X_LATCH:
+ rw_lock_x_unlock(GET_TABLE_STATS_LATCH(table));
+ break;
+ case RW_NO_LATCH:
+ /* fall through */
+ default:
+ ut_error;
+ }
}
/********************************************************************//**
@@ -682,9 +713,9 @@ dict_init(void)
mutex_create(dict_foreign_err_mutex_key,
&dict_foreign_err_mutex, SYNC_ANY_LATCH);
- for (i = 0; i < DICT_INDEX_STAT_MUTEX_SIZE; i++) {
- mutex_create(PFS_NOT_INSTRUMENTED,
- &dict_index_stat_mutex[i], SYNC_INDEX_TREE);
+ for (i = 0; i < DICT_TABLE_STATS_LATCHES_SIZE; i++) {
+ rw_lock_create(PFS_NOT_INSTRUMENTED,
+ &dict_table_stats_latches[i], SYNC_INDEX_TREE);
}
}
@@ -715,12 +746,11 @@ dict_table_get(
mutex_exit(&(dict_sys->mutex));
if (table != NULL) {
- if (!table->stat_initialized) {
- /* If table->ibd_file_missing == TRUE, this will
- print an error message and return without doing
- anything. */
- dict_update_statistics(table);
- }
+ /* If table->ibd_file_missing == TRUE, this will
+ print an error message and return without doing
+ anything. */
+ dict_update_statistics(table, TRUE /* only update stats
+ if they have not been initialized */);
}
return(table);
@@ -4201,6 +4231,10 @@ void
dict_update_statistics_low(
/*=======================*/
dict_table_t* table, /*!< in/out: table */
+ ibool only_calc_if_missing_stats,/*!< in: only
+ update/recalc the stats if they have
+ not been initialized yet, otherwise
+ do nothing */
ibool has_dict_mutex __attribute__((unused)))
/*!< in: TRUE if the caller has the
dictionary mutex */
@@ -4231,6 +4265,12 @@ dict_update_statistics_low(
return;
}
+ dict_table_stats_lock(table, RW_X_LATCH);
+
+ if (only_calc_if_missing_stats && table->stat_initialized) {
+ dict_table_stats_unlock(table, RW_X_LATCH);
+ return;
+ }
do {
if (UNIV_LIKELY
@@ -4276,13 +4316,9 @@ 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
@@ -4291,6 +4327,8 @@ dict_update_statistics_low(
table->stat_initialized = TRUE;
table->stat_modified_counter = 0;
+
+ dict_table_stats_unlock(table, RW_X_LATCH);
}
/*********************************************************************//**
@@ -4300,9 +4338,13 @@ UNIV_INTERN
void
dict_update_statistics(
/*===================*/
- dict_table_t* table) /*!< in/out: table */
+ dict_table_t* table, /*!< in/out: table */
+ ibool only_calc_if_missing_stats)/*!< in: only
+ update/recalc the stats if they have
+ not been initialized yet, otherwise
+ do nothing */
{
- dict_update_statistics_low(table, FALSE);
+ dict_update_statistics_low(table, only_calc_if_missing_stats, FALSE);
}
/**********************************************************************//**
@@ -4382,7 +4424,11 @@ dict_table_print_low(
ut_ad(mutex_own(&(dict_sys->mutex)));
- dict_update_statistics_low(table, TRUE);
+ dict_update_statistics_low(table,
+ FALSE /* update even if initialized */,
+ TRUE /* we have the dict mutex */);
+
+ dict_table_stats_lock(table, RW_S_LATCH);
fprintf(stderr,
"--------------------------------------\n"
@@ -4410,6 +4456,8 @@ dict_table_print_low(
index = UT_LIST_GET_NEXT(indexes, index);
}
+ dict_table_stats_unlock(table, RW_S_LATCH);
+
foreign = UT_LIST_GET_FIRST(table->foreign_list);
while (foreign != NULL) {
@@ -4458,8 +4506,6 @@ 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];
@@ -4467,8 +4513,6 @@ dict_index_print_low(
n_vals = index->stat_n_diff_key_vals[1];
}
- dict_index_stat_mutex_exit(index);
-
fprintf(stderr,
" INDEX: name %s, id %llu, fields %lu/%lu,"
" uniq %lu, type %lu\n"
@@ -4955,8 +4999,8 @@ dict_close(void)
mem_free(dict_sys);
dict_sys = NULL;
- for (i = 0; i < DICT_INDEX_STAT_MUTEX_SIZE; i++) {
- mutex_free(&dict_index_stat_mutex[i]);
+ for (i = 0; i < DICT_TABLE_STATS_LATCHES_SIZE; i++) {
+ rw_lock_free(&dict_table_stats_latches[i]);
}
}
#endif /* !UNIV_HOTBACKUP */
diff --git a/storage/innobase/dict/dict0load.c b/storage/innobase/dict/dict0load.c
index 9c6fa086d58..33259a4163d 100644
--- a/storage/innobase/dict/dict0load.c
+++ b/storage/innobase/dict/dict0load.c
@@ -346,7 +346,11 @@ dict_process_sys_tables_rec(
/* Update statistics if DICT_TABLE_UPDATE_STATS
is set */
- dict_update_statistics_low(*table, TRUE);
+ dict_update_statistics_low(*table,
+ FALSE /* update even if
+ initialized */,
+ TRUE /* we have the dict
+ mutex */);
}
return(NULL);
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 1a2e26ba1ab..0d83cd7e332 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -7534,6 +7534,7 @@ ha_innobase::estimate_rows_upper_bound(void)
dict_index_t* index;
ulonglong estimate;
ulonglong local_data_file_length;
+ ulint stat_n_leaf_pages;
DBUG_ENTER("estimate_rows_upper_bound");
@@ -7553,10 +7554,12 @@ ha_innobase::estimate_rows_upper_bound(void)
index = dict_table_get_first_index(prebuilt->table);
- ut_a(index->stat_n_leaf_pages > 0);
+ stat_n_leaf_pages = index->stat_n_leaf_pages;
+
+ ut_a(stat_n_leaf_pages > 0);
local_data_file_length =
- ((ulonglong) index->stat_n_leaf_pages) * UNIV_PAGE_SIZE;
+ ((ulonglong) stat_n_leaf_pages) * UNIV_PAGE_SIZE;
/* Calculate a minimum length for a clustered index record and from
@@ -7752,7 +7755,9 @@ ha_innobase::info_low(
prebuilt->trx->op_info = "updating table statistics";
- dict_update_statistics(ib_table);
+ dict_update_statistics(ib_table,
+ FALSE /* update even if stats
+ are initialized */);
prebuilt->trx->op_info = "returning various info to MySQL";
}
@@ -7771,6 +7776,9 @@ ha_innobase::info_low(
}
if (flag & HA_STATUS_VARIABLE) {
+
+ dict_table_stats_lock(ib_table, RW_S_LATCH);
+
n_rows = ib_table->stat_n_rows;
/* Because we do not protect stat_n_rows by any mutex in a
@@ -7820,6 +7828,8 @@ ha_innobase::info_low(
ib_table->stat_sum_of_other_index_sizes)
* UNIV_PAGE_SIZE;
+ dict_table_stats_unlock(ib_table, RW_S_LATCH);
+
/* Since fsp_get_available_space_in_free_extents() is
acquiring latches inside InnoDB, we do not call it if we
are asked by MySQL to avoid locking. Another reason to
@@ -7896,6 +7906,8 @@ ha_innobase::info_low(
table->s->keys);
}
+ dict_table_stats_lock(ib_table, RW_S_LATCH);
+
for (i = 0; i < table->s->keys; i++) {
ulong j;
/* We could get index quickly through internal
@@ -7933,8 +7945,6 @@ ha_innobase::info_low(
break;
}
- dict_index_stat_mutex_enter(index);
-
if (index->stat_n_diff_key_vals[j + 1] == 0) {
rec_per_key = stats.records;
@@ -7943,8 +7953,6 @@ ha_innobase::info_low(
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
@@ -7961,6 +7969,8 @@ ha_innobase::info_low(
(ulong) rec_per_key;
}
}
+
+ dict_table_stats_unlock(ib_table, RW_S_LATCH);
}
if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) {
diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h
index 029cf408141..f5a3e8dafc0 100644
--- a/storage/innobase/include/dict0dict.h
+++ b/storage/innobase/include/dict0dict.h
@@ -1083,6 +1083,10 @@ void
dict_update_statistics_low(
/*=======================*/
dict_table_t* table, /*!< in/out: table */
+ ibool only_calc_if_missing_stats,/*!< in: only
+ update/recalc the stats if they have
+ not been initialized yet, otherwise
+ do nothing */
ibool has_dict_mutex);/*!< in: TRUE if the caller has the
dictionary mutex */
/*********************************************************************//**
@@ -1092,7 +1096,11 @@ UNIV_INTERN
void
dict_update_statistics(
/*===================*/
- dict_table_t* table); /*!< in/out: table */
+ dict_table_t* table, /*!< in/out: table */
+ ibool only_calc_if_missing_stats);/*!< in: only
+ update/recalc the stats if they have
+ not been initialized yet, otherwise
+ do nothing */
/********************************************************************//**
Reserves the dictionary system mutex for MySQL. */
UNIV_INTERN
@@ -1106,21 +1114,25 @@ 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. */
+Lock the appropriate latch to protect a given table's statistics.
+table->id is used to pick the corresponding latch from a global array of
+latches. */
UNIV_INTERN
void
-dict_index_stat_mutex_enter(
-/*========================*/
- const dict_index_t* index); /*!< in: index */
+dict_table_stats_lock(
+/*==================*/
+ const dict_table_t* table, /*!< in: table */
+ ulint latch_mode); /*!< in: RW_S_LATCH or
+ RW_X_LATCH */
/**********************************************************************//**
-Unlock the appropriate mutex that protects index->stat_n_diff_key_vals[]. */
+Unlock the latch that has been locked by dict_table_stats_lock() */
UNIV_INTERN
void
-dict_index_stat_mutex_exit(
-/*=======================*/
- const dict_index_t* index); /*!< in: index */
+dict_table_stats_unlock(
+/*====================*/
+ const dict_table_t* table, /*!< in: table */
+ ulint latch_mode); /*!< in: RW_S_LATCH or
+ RW_X_LATCH */
/********************************************************************//**
Checks if the database name in two table names is the same.
@return TRUE if same db name */
diff --git a/storage/innobase/row/row0mysql.c b/storage/innobase/row/row0mysql.c
index bd9143125f8..709750fc4e7 100644
--- a/storage/innobase/row/row0mysql.c
+++ b/storage/innobase/row/row0mysql.c
@@ -930,7 +930,8 @@ row_update_statistics_if_needed(
if (counter > 2000000000
|| ((ib_int64_t)counter > 16 + table->stat_n_rows / 16)) {
- dict_update_statistics(table);
+ dict_update_statistics(table, FALSE /* update even if stats
+ are initialized */);
}
}
@@ -3020,7 +3021,8 @@ next_rec:
dict_table_autoinc_lock(table);
dict_table_autoinc_initialize(table, 1);
dict_table_autoinc_unlock(table);
- dict_update_statistics(table);
+ dict_update_statistics(table, FALSE /* update even if stats are
+ initialized */);
trx_commit_for_mysql(trx);
View Lorry Controller Status