From a53f3c6d3cfa50b15b1aff26bc9479eb582d8611 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 28 Sep 2016 16:12:58 +0300 Subject: [PATCH] MDEV-10649: Optimizer sometimes use "index" instead of "range" access for UPDATE (Fixing both InnoDB and XtraDB) Re-opening a TABLE object (after e.g. FLUSH TABLES or open table cache eviction) causes ha_innobase to call dict_stats_update(DICT_STATS_FETCH_ONLY_IF_NOT_IN_MEMORY). Inside this call, the following is done: dict_stats_empty_table(table); dict_stats_copy(table, t); On the other hand, commands like UPDATE make this call to get the "rows in table" statistics in table->stats.records: ha_innobase->info(HA_STATUS_VARIABLE|HA_STATUS_NO_LOCK) note the HA_STATUS_NO_LOCK parameter. It means, no locks are taken by ::info() If the ::info() call happens between dict_stats_empty_table and dict_stats_copy calls, the UPDATE's optimizer will get an estimate of table->stats.records=1, which causes it to pick a full table scan, which in turn will take a lot of row locks and cause other bad consequences. --- storage/innobase/dict/dict0stats.cc | 29 +++++++++++++++++++---------- storage/xtradb/dict/dict0stats.cc | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/storage/innobase/dict/dict0stats.cc b/storage/innobase/dict/dict0stats.cc index b073398f8ecf7..a4aa43651f8a5 100644 --- a/storage/innobase/dict/dict0stats.cc +++ b/storage/innobase/dict/dict0stats.cc @@ -673,7 +673,10 @@ void dict_stats_copy( /*============*/ dict_table_t* dst, /*!< in/out: destination table */ - const dict_table_t* src) /*!< in: source table */ + const dict_table_t* src, /*!< in: source table */ + bool reset_ignored_indexes) /*!< in: if true, set ignored indexes + to have the same statistics as if + the table was empty */ { dst->stats_last_recalc = src->stats_last_recalc; dst->stat_n_rows = src->stat_n_rows; @@ -692,7 +695,16 @@ dict_stats_copy( && (src_idx = dict_table_get_next_index(src_idx)))) { if (dict_stats_should_ignore_index(dst_idx)) { - continue; + if (reset_ignored_indexes) { + /* Reset index statistics for all ignored indexes, + unless they are FT indexes (these have no statistics)*/ + if (dst_idx->type & DICT_FTS) { + continue; + } + dict_stats_empty_index(dst_idx); + } else { + continue; + } } ut_ad(!dict_index_is_univ(dst_idx)); @@ -782,7 +794,7 @@ dict_stats_snapshot_create( t = dict_stats_table_clone_create(table); - dict_stats_copy(t, table); + dict_stats_copy(t, table, false); t->stat_persistent = table->stat_persistent; t->stats_auto_recalc = table->stats_auto_recalc; @@ -3240,13 +3252,10 @@ dict_stats_update( dict_table_stats_lock(table, RW_X_LATCH); - /* Initialize all stats to dummy values before - copying because dict_stats_table_clone_create() does - skip corrupted indexes so our dummy object 't' may - have less indexes than the real object 'table'. */ - dict_stats_empty_table(table); - - dict_stats_copy(table, t); + /* Pass reset_ignored_indexes=true as parameter + to dict_stats_copy. This will cause statictics + for corrupted indexes to be set to empty values */ + dict_stats_copy(table, t, true); dict_stats_assert_initialized(table); diff --git a/storage/xtradb/dict/dict0stats.cc b/storage/xtradb/dict/dict0stats.cc index b073398f8ecf7..a4aa43651f8a5 100644 --- a/storage/xtradb/dict/dict0stats.cc +++ b/storage/xtradb/dict/dict0stats.cc @@ -673,7 +673,10 @@ void dict_stats_copy( /*============*/ dict_table_t* dst, /*!< in/out: destination table */ - const dict_table_t* src) /*!< in: source table */ + const dict_table_t* src, /*!< in: source table */ + bool reset_ignored_indexes) /*!< in: if true, set ignored indexes + to have the same statistics as if + the table was empty */ { dst->stats_last_recalc = src->stats_last_recalc; dst->stat_n_rows = src->stat_n_rows; @@ -692,7 +695,16 @@ dict_stats_copy( && (src_idx = dict_table_get_next_index(src_idx)))) { if (dict_stats_should_ignore_index(dst_idx)) { - continue; + if (reset_ignored_indexes) { + /* Reset index statistics for all ignored indexes, + unless they are FT indexes (these have no statistics)*/ + if (dst_idx->type & DICT_FTS) { + continue; + } + dict_stats_empty_index(dst_idx); + } else { + continue; + } } ut_ad(!dict_index_is_univ(dst_idx)); @@ -782,7 +794,7 @@ dict_stats_snapshot_create( t = dict_stats_table_clone_create(table); - dict_stats_copy(t, table); + dict_stats_copy(t, table, false); t->stat_persistent = table->stat_persistent; t->stats_auto_recalc = table->stats_auto_recalc; @@ -3240,13 +3252,10 @@ dict_stats_update( dict_table_stats_lock(table, RW_X_LATCH); - /* Initialize all stats to dummy values before - copying because dict_stats_table_clone_create() does - skip corrupted indexes so our dummy object 't' may - have less indexes than the real object 'table'. */ - dict_stats_empty_table(table); - - dict_stats_copy(table, t); + /* Pass reset_ignored_indexes=true as parameter + to dict_stats_copy. This will cause statictics + for corrupted indexes to be set to empty values */ + dict_stats_copy(table, t, true); dict_stats_assert_initialized(table);