Skip to content

Commit 054c8ab

Browse files
lthinikep
authored andcommitted
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
1 parent 1a18285 commit 054c8ab

File tree

6 files changed

+182
-1
lines changed

6 files changed

+182
-1
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
set global innodb_stats_locked_reads = on;
2+
CREATE TABLE t (a int,
3+
b int,
4+
c int,
5+
d int,
6+
e int,
7+
PRIMARY KEY(a),
8+
KEY innodb_index_lock_test (b, c, d)) ENGINE=InnoDB;
9+
insert into t values (1, 1, 1, 1, 1), (2, 2, 2, 2, 2), (3, 3, 3, 3, 3), (4, 4, 4, 4, 4);
10+
insert into t select a+4, b+4, c, d, e from t;
11+
insert into t select a+8, b+8, c, d, e from t;
12+
insert into t select a+16, b+16, c, d, e from t;
13+
insert into t select a+32, b+32, c, d, e from t;
14+
insert into t select a+64, b+64, c, d, e from t;
15+
insert into t select a+128, b+128, c, d, e from t;
16+
insert into t select a+256, b+256, c, d, e from t;
17+
insert into t select a+512, b+512, c, d, e from t;
18+
insert into t select a+1024, b+1024, c, d, e from t;
19+
analyze table t;
20+
Table Op Msg_type Msg_text
21+
test.t analyze status OK
22+
flush tables;
23+
show indexes in t;
24+
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment Visible Expression
25+
t 0 PRIMARY 1 a A 2048 NULL NULL BTREE YES NULL
26+
t 1 innodb_index_lock_test 1 b A 2048 NULL NULL YES BTREE YES NULL
27+
t 1 innodb_index_lock_test 2 c A 2048 NULL NULL YES BTREE YES NULL
28+
t 1 innodb_index_lock_test 3 d A 2048 NULL NULL YES BTREE YES NULL
29+
set information_schema_stats_expiry = 0;
30+
set innodb_stats_on_metadata = on;
31+
set debug_sync = "after_empty_index SIGNAL parked WAIT_FOR go";
32+
select * from information_schema.tables where table_name = 't';
33+
set information_schema_stats_expiry = 0;
34+
set debug_sync='now WAIT_FOR parked';
35+
show indexes in t;
36+
set debug_sync='now SIGNAL go';
37+
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment Visible Expression
38+
t 0 PRIMARY 1 a A 2048 NULL NULL BTREE YES NULL
39+
t 1 innodb_index_lock_test 1 b A 2048 NULL NULL YES BTREE YES NULL
40+
t 1 innodb_index_lock_test 2 c A 2048 NULL NULL YES BTREE YES NULL
41+
t 1 innodb_index_lock_test 3 d A 2048 NULL NULL YES BTREE YES NULL
42+
drop table t;
43+
set debug_sync = 'RESET';
44+
set global innodb_stats_locked_reads = default;
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
--source include/have_debug_sync.inc
2+
--source include/count_sessions.inc
3+
4+
set global innodb_stats_locked_reads = on;
5+
6+
CREATE TABLE t (a int,
7+
b int,
8+
c int,
9+
d int,
10+
e int,
11+
PRIMARY KEY(a),
12+
KEY innodb_index_lock_test (b, c, d)) ENGINE=InnoDB;
13+
14+
insert into t values (1, 1, 1, 1, 1), (2, 2, 2, 2, 2), (3, 3, 3, 3, 3), (4, 4, 4, 4, 4);
15+
insert into t select a+4, b+4, c, d, e from t;
16+
insert into t select a+8, b+8, c, d, e from t;
17+
insert into t select a+16, b+16, c, d, e from t;
18+
insert into t select a+32, b+32, c, d, e from t;
19+
insert into t select a+64, b+64, c, d, e from t;
20+
insert into t select a+128, b+128, c, d, e from t;
21+
insert into t select a+256, b+256, c, d, e from t;
22+
insert into t select a+512, b+512, c, d, e from t;
23+
insert into t select a+1024, b+1024, c, d, e from t;
24+
25+
analyze table t;
26+
flush tables;
27+
28+
show indexes in t;
29+
30+
set information_schema_stats_expiry = 0;
31+
32+
set innodb_stats_on_metadata = on;
33+
set debug_sync = "after_empty_index SIGNAL parked WAIT_FOR go";
34+
send select * from information_schema.tables where table_name = 't';
35+
36+
connect (con1,localhost,root,,);
37+
set information_schema_stats_expiry = 0;
38+
set debug_sync='now WAIT_FOR parked';
39+
send show indexes in t;
40+
41+
connect (con2,localhost,root,,);
42+
set debug_sync='now SIGNAL go';
43+
disconnect con2;
44+
45+
connection con1;
46+
reap;
47+
48+
disconnect con1;
49+
50+
connection default;
51+
--disable_result_log
52+
reap;
53+
--enable_result_log
54+
55+
drop table t;
56+
set debug_sync = 'RESET';
57+
58+
set global innodb_stats_locked_reads = default;
59+
60+
--source include/wait_until_count_sessions.inc
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
SELECT @@innodb_stats_locked_reads;
2+
@@innodb_stats_locked_reads
3+
0
4+
SET GLOBAL innodb_stats_locked_reads=ON;
5+
SELECT @@innodb_stats_locked_reads;
6+
@@innodb_stats_locked_reads
7+
1
8+
SET GLOBAL innodb_stats_locked_reads=OFF;
9+
SELECT @@innodb_stats_locked_reads;
10+
@@innodb_stats_locked_reads
11+
0
12+
SET GLOBAL innodb_stats_locked_reads=1;
13+
SELECT @@innodb_stats_locked_reads;
14+
@@innodb_stats_locked_reads
15+
1
16+
SET GLOBAL innodb_stats_locked_reads=0;
17+
SELECT @@innodb_stats_locked_reads;
18+
@@innodb_stats_locked_reads
19+
0
20+
SET GLOBAL innodb_stats_locked_reads=123;
21+
ERROR 42000: Variable 'innodb_stats_locked_reads' can't be set to the value of '123'
22+
SET GLOBAL innodb_stats_locked_reads='foo';
23+
ERROR 42000: Variable 'innodb_stats_locked_reads' can't be set to the value of 'foo'
24+
SET GLOBAL innodb_stats_locked_reads=default;
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#
2+
# innodb_stats_locked_reads
3+
#
4+
5+
6+
# show the default value
7+
SELECT @@innodb_stats_locked_reads;
8+
9+
# check that it is writeable
10+
SET GLOBAL innodb_stats_locked_reads=ON;
11+
SELECT @@innodb_stats_locked_reads;
12+
13+
SET GLOBAL innodb_stats_locked_reads=OFF;
14+
SELECT @@innodb_stats_locked_reads;
15+
16+
SET GLOBAL innodb_stats_locked_reads=1;
17+
SELECT @@innodb_stats_locked_reads;
18+
19+
SET GLOBAL innodb_stats_locked_reads=0;
20+
SELECT @@innodb_stats_locked_reads;
21+
22+
# should be a boolean
23+
-- error ER_WRONG_VALUE_FOR_VAR
24+
SET GLOBAL innodb_stats_locked_reads=123;
25+
26+
-- error ER_WRONG_VALUE_FOR_VAR
27+
SET GLOBAL innodb_stats_locked_reads='foo';
28+
29+
# restore the environment
30+
SET GLOBAL innodb_stats_locked_reads=default;

storage/innobase/dict/dict0stats.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ this program; if not, write to the Free Software Foundation, Inc.,
2424
2525
*****************************************************************************/
2626

27+
#include "current_thd.h"
28+
#include "debug_sync.h"
2729
#include "my_dbug.h"
2830

2931
/** @file dict/dict0stats.cc
@@ -2057,6 +2059,11 @@ static dberr_t dict_stats_update_persistent(
20572059
}
20582060

20592061
dict_stats_empty_index(index);
2062+
#ifndef NDEBUG
2063+
if (current_thd && strcmp(index->name, "innodb_index_lock_test") == 0) {
2064+
DEBUG_SYNC(current_thd, "after_empty_index");
2065+
}
2066+
#endif
20602067

20612068
if (dict_stats_should_ignore_index(index)) {
20622069
continue;

storage/innobase/handler/ha_innodb.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ static ulong innobase_commit_concurrency = 0;
303303

304304
/* Boolean @@innodb_buffer_pool_in_core_file. */
305305
bool srv_buffer_pool_in_core_file = true;
306+
bool srv_stats_locked_reads = false;
306307

307308
/** Percentage of the buffer pool to reserve for 'old' blocks.
308309
Connected to buf_LRU_old_ratio. */
@@ -7688,7 +7689,9 @@ int ha_innobase::open(const char *name, int, uint open_flags,
76887689
}
76897690
}
76907691

7691-
info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
7692+
uint flags = HA_STATUS_VARIABLE | HA_STATUS_CONST;
7693+
if (!srv_stats_locked_reads) flags |= HA_STATUS_NO_LOCK;
7694+
info(flags);
76927695

76937696
dberr_t err =
76947697
dict_set_compression(m_prebuilt->table, table->s->compress.str, false);
@@ -17879,6 +17882,11 @@ static bool innobase_get_index_column_cardinality(
1787917882
}
1788017883

1788117884
DEBUG_SYNC(thd, "innodb.after_init_check");
17885+
bool need_unlock = false;
17886+
if (srv_stats_locked_reads) {
17887+
need_unlock = true;
17888+
dict_table_stats_lock(ib_table, RW_S_LATCH);
17889+
}
1788217890
if (index->type & (DICT_FTS | DICT_SPATIAL)) {
1788317891
/* For these indexes innodb_rec_per_key is
1788417892
fixed as 1.0 */
@@ -17891,6 +17899,8 @@ static bool innobase_get_index_column_cardinality(
1789117899
*cardinality = static_cast<ulonglong>(round(records));
1789217900
}
1789317901

17902+
if (need_unlock) dict_table_stats_unlock(ib_table, RW_S_LATCH);
17903+
1789417904
failure = false;
1789517905
break;
1789617906
}
@@ -22316,6 +22326,11 @@ static MYSQL_SYSVAR_ULONGLONG(
2231622326
" statistics (by ANALYZE, default 20)",
2231722327
nullptr, nullptr, 20, 1, ~0ULL, 0);
2231822328

22329+
static MYSQL_SYSVAR_BOOL(stats_locked_reads, srv_stats_locked_reads,
22330+
PLUGIN_VAR_OPCMDARG,
22331+
"Controls if InnoDB stats are locked for reading.",
22332+
nullptr, nullptr, false);
22333+
2231922334
static MYSQL_SYSVAR_BOOL(
2232022335
stats_update_online_ddl, srv_stats_update_online_ddl, PLUGIN_VAR_OPCMDARG,
2232122336
"Control If InnoDB should update index statistics, While related index is"
@@ -23465,6 +23480,7 @@ static SYS_VAR *innobase_system_variables[] = {
2346523480
MYSQL_SYSVAR(stats_persistent),
2346623481
MYSQL_SYSVAR(stats_persistent_sample_pages),
2346723482
MYSQL_SYSVAR(stats_auto_recalc),
23483+
MYSQL_SYSVAR(stats_locked_reads),
2346823484
MYSQL_SYSVAR(stats_update_online_ddl),
2346923485
MYSQL_SYSVAR(adaptive_hash_index),
2347023486
MYSQL_SYSVAR(adaptive_hash_index_parts),

0 commit comments

Comments
 (0)