Skip to content

Commit 85b93d8

Browse files
lthinikep
lth
authored and
inikep
committed
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 ff29638 commit 85b93d8

File tree

6 files changed

+182
-1
lines changed

6 files changed

+182
-1
lines changed

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

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;

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

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

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

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;

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

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
@@ -304,6 +304,7 @@ static ulong innobase_commit_concurrency = 0;
304304

305305
/* Boolean @@innodb_buffer_pool_in_core_file. */
306306
bool srv_buffer_pool_in_core_file = true;
307+
bool srv_stats_locked_reads = false;
307308

308309
/** Percentage of the buffer pool to reserve for 'old' blocks.
309310
Connected to buf_LRU_old_ratio. */
@@ -7684,7 +7685,9 @@ int ha_innobase::open(const char *name, int, uint open_flags,
76847685
}
76857686
}
76867687

7687-
info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
7688+
uint flags = HA_STATUS_VARIABLE | HA_STATUS_CONST;
7689+
if (!srv_stats_locked_reads) flags |= HA_STATUS_NO_LOCK;
7690+
info(flags);
76887691

76897692
dberr_t err =
76907693
dict_set_compression(m_prebuilt->table, table->s->compress.str, false);
@@ -17872,6 +17875,11 @@ static bool innobase_get_index_column_cardinality(
1787217875
}
1787317876

1787417877
DEBUG_SYNC(thd, "innodb.after_init_check");
17878+
bool need_unlock = false;
17879+
if (srv_stats_locked_reads) {
17880+
need_unlock = true;
17881+
dict_table_stats_lock(ib_table, RW_S_LATCH);
17882+
}
1787517883
if (index->type & (DICT_FTS | DICT_SPATIAL)) {
1787617884
/* For these indexes innodb_rec_per_key is
1787717885
fixed as 1.0 */
@@ -17884,6 +17892,8 @@ static bool innobase_get_index_column_cardinality(
1788417892
*cardinality = static_cast<ulonglong>(round(records));
1788517893
}
1788617894

17895+
if (need_unlock) dict_table_stats_unlock(ib_table, RW_S_LATCH);
17896+
1788717897
failure = false;
1788817898
break;
1788917899
}
@@ -22315,6 +22325,11 @@ static MYSQL_SYSVAR_ULONGLONG(
2231522325
" statistics (by ANALYZE, default 20)",
2231622326
nullptr, nullptr, 20, 1, ~0ULL, 0);
2231722327

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

0 commit comments

Comments
 (0)