[Commits] a53f3c6: MDEV-10649: Optimizer sometimes use "index" instead of "range" access for UPDATE

Sergei Petrunia psergey at askmonty.org
Wed Sep 28 16:12:59 EEST 2016


revision-id: a53f3c6d3cfa50b15b1aff26bc9479eb582d8611
parent(s): 5bbe929d706e26cb3f9b291da6009526a17b1545
committer: Sergei Petrunia
branch nick: 10.0
timestamp: 2016-09-28 16:12:58 +0300
message:

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 b073398..a4aa436 100644
--- a/storage/innobase/dict/dict0stats.cc
+++ b/storage/innobase/dict/dict0stats.cc
@@ -673,7 +673,10 @@ Write all zeros (or 1 where it makes sense) into a table and its indexes'
 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 @@ Write all zeros (or 1 where it makes sense) into a table and its indexes'
 	      && (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 @@ The returned object should be freed with dict_stats_snapshot_free()
 
 	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 @@ N*AVG(Ui). In each call it searches for the currently fetched index into
 
 			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 b073398..a4aa436 100644
--- a/storage/xtradb/dict/dict0stats.cc
+++ b/storage/xtradb/dict/dict0stats.cc
@@ -673,7 +673,10 @@ Write all zeros (or 1 where it makes sense) into a table and its indexes'
 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 @@ Write all zeros (or 1 where it makes sense) into a table and its indexes'
 	      && (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 @@ The returned object should be freed with dict_stats_snapshot_free()
 
 	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 @@ N*AVG(Ui). In each call it searches for the currently fetched index into
 
 			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);
 


More information about the commits mailing list