Add locking when reading index cardinality in InnoDB

Summary:
When updating index cardinality in InnoDB, the stats are first zero'd out via `dict_stats_empty_index` and then populated. Although there is locking on the write path, when updating index stats, it looks like we usually do not have locking on the read path in innodb. This can lead to bad plans, as we would return bad index stats.

The fix is to take a shared lock on the read paths.

https://bugs.mysql.com/bug.php?id=103210

Reviewed By: luqun, yizhang82

Differential Revision: D27545419

mysql-test/suite/innodb/r/innodb_stats_race.result

@@ -0,0 +1,44 @@
+set global innodb_stats_locked_reads = on;
+CREATE TABLE t (a int,
+b int,
+c int,
+d int,
+e int,
+PRIMARY KEY(a),
+KEY innodb_index_lock_test (b, c, d)) ENGINE=InnoDB;
+insert into t values (1, 1, 1, 1, 1), (2, 2, 2, 2, 2), (3, 3, 3, 3, 3), (4, 4, 4, 4, 4);
+insert into t select a+4, b+4, c, d, e from t;
+insert into t select a+8, b+8, c, d, e from t;
+insert into t select a+16, b+16, c, d, e from t;
+insert into t select a+32, b+32, c, d, e from t;
+insert into t select a+64, b+64, c, d, e from t;
+insert into t select a+128, b+128, c, d, e from t;
+insert into t select a+256, b+256, c, d, e from t;
+insert into t select a+512, b+512, c, d, e from t;
+insert into t select a+1024, b+1024, c, d, e from t;
+analyze table t;
+Table	Op	Msg_type	Msg_text
+test.t	analyze	status	OK
+flush tables;
+show indexes in t;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment	Index_comment	Visible	Expression
+t	0	PRIMARY	1	a	A	2048	NULL	NULL		BTREE			YES	NULL
+t	1	innodb_index_lock_test	1	b	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+t	1	innodb_index_lock_test	2	c	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+t	1	innodb_index_lock_test	3	d	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+set information_schema_stats_expiry = 0;
+set innodb_stats_on_metadata = on;
+set debug_sync = "after_empty_index SIGNAL parked WAIT_FOR go";
+select * from information_schema.tables where table_name = 't';
+set information_schema_stats_expiry = 0;
+set debug_sync='now WAIT_FOR parked';
+show indexes in t;
+set debug_sync='now SIGNAL go';
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment	Index_comment	Visible	Expression
+t	0	PRIMARY	1	a	A	2048	NULL	NULL		BTREE			YES	NULL
+t	1	innodb_index_lock_test	1	b	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+t	1	innodb_index_lock_test	2	c	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+t	1	innodb_index_lock_test	3	d	A	2048	NULL	NULL	YES	BTREE			YES	NULL
+drop table t;
+set debug_sync = 'RESET';
+set global innodb_stats_locked_reads = default;

mysql-test/suite/innodb/t/innodb_stats_race.test

@@ -0,0 +1,60 @@
+--source include/have_debug_sync.inc
+--source include/count_sessions.inc
+
+set global innodb_stats_locked_reads = on;
+
+CREATE TABLE t (a int,
+                b int,
+                c int,
+                d int,
+                e int,
+                PRIMARY KEY(a),
+                KEY innodb_index_lock_test (b, c, d)) ENGINE=InnoDB;
+
+insert into t values (1, 1, 1, 1, 1), (2, 2, 2, 2, 2), (3, 3, 3, 3, 3), (4, 4, 4, 4, 4);
+insert into t select a+4, b+4, c, d, e from t;
+insert into t select a+8, b+8, c, d, e from t;
+insert into t select a+16, b+16, c, d, e from t;
+insert into t select a+32, b+32, c, d, e from t;
+insert into t select a+64, b+64, c, d, e from t;
+insert into t select a+128, b+128, c, d, e from t;
+insert into t select a+256, b+256, c, d, e from t;
+insert into t select a+512, b+512, c, d, e from t;
+insert into t select a+1024, b+1024, c, d, e from t;
+
+analyze table t;
+flush tables;
+
+show indexes in t;
+
+set information_schema_stats_expiry = 0;
+
+set innodb_stats_on_metadata = on;
+set debug_sync = "after_empty_index SIGNAL parked WAIT_FOR go";
+send select * from information_schema.tables where table_name = 't';
+
+connect (con1,localhost,root,,);
+set information_schema_stats_expiry = 0;
+set debug_sync='now WAIT_FOR parked';
+send show indexes in t;
+
+connect (con2,localhost,root,,);
+set debug_sync='now SIGNAL go';
+disconnect con2;
+
+connection con1;
+reap;
+
+disconnect con1;
+
+connection default;
+--disable_result_log
+reap;
+--enable_result_log
+
+drop table t;
+set debug_sync = 'RESET';
+
+set global innodb_stats_locked_reads = default;
+
+--source include/wait_until_count_sessions.inc

mysql-test/suite/sys_vars/r/innodb_stats_locked_reads_basic.result

@@ -0,0 +1,24 @@
+SELECT @@innodb_stats_locked_reads;
+@@innodb_stats_locked_reads
+0
+SET GLOBAL innodb_stats_locked_reads=ON;
+SELECT @@innodb_stats_locked_reads;
+@@innodb_stats_locked_reads
+1
+SET GLOBAL innodb_stats_locked_reads=OFF;
+SELECT @@innodb_stats_locked_reads;
+@@innodb_stats_locked_reads
+0
+SET GLOBAL innodb_stats_locked_reads=1;
+SELECT @@innodb_stats_locked_reads;
+@@innodb_stats_locked_reads
+1
+SET GLOBAL innodb_stats_locked_reads=0;
+SELECT @@innodb_stats_locked_reads;
+@@innodb_stats_locked_reads
+0
+SET GLOBAL innodb_stats_locked_reads=123;
+ERROR 42000: Variable 'innodb_stats_locked_reads' can't be set to the value of '123'
+SET GLOBAL innodb_stats_locked_reads='foo';
+ERROR 42000: Variable 'innodb_stats_locked_reads' can't be set to the value of 'foo'
+SET GLOBAL innodb_stats_locked_reads=default;

mysql-test/suite/sys_vars/t/innodb_stats_locked_reads_basic.test

@@ -0,0 +1,30 @@
+#
+# innodb_stats_locked_reads
+#
+
+
+# show the default value
+SELECT @@innodb_stats_locked_reads;
+
+# check that it is writeable
+SET GLOBAL innodb_stats_locked_reads=ON;
+SELECT @@innodb_stats_locked_reads;
+
+SET GLOBAL innodb_stats_locked_reads=OFF;
+SELECT @@innodb_stats_locked_reads;
+
+SET GLOBAL innodb_stats_locked_reads=1;
+SELECT @@innodb_stats_locked_reads;
+
+SET GLOBAL innodb_stats_locked_reads=0;
+SELECT @@innodb_stats_locked_reads;
+
+# should be a boolean
+-- error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL innodb_stats_locked_reads=123;
+
+-- error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL innodb_stats_locked_reads='foo';
+
+# restore the environment
+SET GLOBAL innodb_stats_locked_reads=default;

storage/innobase/dict/dict0stats.cc

@@ -24,6 +24,8 @@ this program; if not, write to the Free Software Foundation, Inc.,
 
 *****************************************************************************/
 
+#include "current_thd.h"
+#include "debug_sync.h"
 #include "my_dbug.h"
 
 /** @file dict/dict0stats.cc
@@ -2066,6 +2068,11 @@ static dberr_t dict_stats_update_persistent(
     }
 
     dict_stats_empty_index(index);
+#ifndef NDEBUG
+    if (current_thd && strcmp(index->name, "innodb_index_lock_test") == 0) {
+      DEBUG_SYNC(current_thd, "after_empty_index");
+    }
+#endif
 
     if (dict_stats_should_ignore_index(index)) {
       continue;

storage/innobase/handler/ha_innodb.cc

@@ -313,6 +313,7 @@ static ulong innobase_commit_concurrency = 0;
 
 /* Boolean @@innodb_buffer_pool_in_core_file. */
 bool srv_buffer_pool_in_core_file = true;
+bool srv_stats_locked_reads = false;
 
 /** Percentage of the buffer pool to reserve for 'old' blocks.
 Connected to buf_LRU_old_ratio. */
@@ -8039,7 +8040,9 @@ int ha_innobase::open(const char *name, int, uint open_flags,
     }
   }
 
-  info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
+  uint flags = HA_STATUS_VARIABLE | HA_STATUS_CONST;
+  if (!srv_stats_locked_reads) flags |= HA_STATUS_NO_LOCK;
+  info(flags);
 
   dberr_t err =
       dict_set_compression(m_prebuilt->table, table->s->compress.str, false);
@@ -18230,6 +18233,11 @@ static bool innobase_get_index_column_cardinality(
       }
 
       DEBUG_SYNC(thd, "innodb.after_init_check");
+      bool need_unlock = false;
+      if (srv_stats_locked_reads) {
+        need_unlock = true;
+        dict_table_stats_lock(ib_table, RW_S_LATCH);
+      }
       if (index->type & (DICT_FTS | DICT_SPATIAL)) {
         /* For these indexes innodb_rec_per_key is
         fixed as 1.0 */
@@ -18242,6 +18250,8 @@ static bool innobase_get_index_column_cardinality(
         *cardinality = static_cast<ulonglong>(round(records));
       }
 
+      if (need_unlock) dict_table_stats_unlock(ib_table, RW_S_LATCH);
+
       failure = false;
       break;
     }
@@ -22674,6 +22684,11 @@ static MYSQL_SYSVAR_ULONGLONG(
     " statistics (by ANALYZE, default 20)",
     nullptr, nullptr, 20, 1, ~0ULL, 0);
 
+static MYSQL_SYSVAR_BOOL(stats_locked_reads, srv_stats_locked_reads,
+                         PLUGIN_VAR_OPCMDARG,
+                         "Controls if InnoDB stats are locked for reading.",
+                         nullptr, nullptr, false);
+
 static MYSQL_SYSVAR_BOOL(
     stats_update_online_ddl, srv_stats_update_online_ddl, PLUGIN_VAR_OPCMDARG,
     "Control If InnoDB should update index statistics, While related index is"
@@ -23821,6 +23836,7 @@ static SYS_VAR *innobase_system_variables[] = {
     MYSQL_SYSVAR(stats_persistent),
     MYSQL_SYSVAR(stats_persistent_sample_pages),
     MYSQL_SYSVAR(stats_auto_recalc),
+    MYSQL_SYSVAR(stats_locked_reads),
     MYSQL_SYSVAR(stats_update_online_ddl),
     MYSQL_SYSVAR(adaptive_hash_index),
     MYSQL_SYSVAR(adaptive_hash_index_parts),