From f91798dd1c9e178061ba58c88a42b9cb3701385d Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Wed, 18 Nov 2015 21:31:45 +0300 Subject: [PATCH] MDEV-7370: Server deadlocks on renaming a table for which persistent statistics exists RENAME TABLE code tries to update EITS statistics. It hung, because it used an index on (db_name,table_name) to find the table, and attempted to update these values at the same time. The fix is do what SQL UPDATE statement does when updating index that it's used for scanning: - First, buffer the rowids of rows to be updated, - then make the second pass to actually update the rows Also fixed the call to rename_table_in_stat_tables() in sql_rename.cc to pass the correct new database (before, it passed old db_name so cross- database renames were not handled correctly). Variant #2, with review feedback addressed. --- mysql-test/r/stat_tables.result | 95 +++++++++++++++++++ mysql-test/r/stat_tables_innodb.result | 95 +++++++++++++++++++ mysql-test/t/stat_tables.test | 73 ++++++++++++++ sql/sql_rename.cc | 3 +- sql/sql_statistics.cc | 126 ++++++++++++++++++++++++- 5 files changed, 389 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/stat_tables.result b/mysql-test/r/stat_tables.result index 285284596c4..fcced761283 100644 --- a/mysql-test/r/stat_tables.result +++ b/mysql-test/r/stat_tables.result @@ -421,4 +421,99 @@ id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t1 const PRIMARY PRIMARY 4 const 1 Using index 1 SIMPLE t2 ALL NULL NULL NULL NULL 2 Using where DROP TABLE t1,t2; +# +# MDEV-7370: Server deadlocks on renaming a table for which persistent statistics exists +# +drop database if exists db1; +drop database if exists db1; +create database db1; +create database db2; +use db1; +# +# First, run the original testcase: +# +create table t1 (i int); +insert into t1 values (10),(20); +analyze table t1 persistent for all; +Table Op Msg_type Msg_text +db1.t1 analyze status Engine-independent statistics collected +db1.t1 analyze status OK +rename table t1 to db2.t1; +# Verify that stats in the old database are gone: +select * from mysql.column_stats where db_name='db1' and table_name='t1'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.table_stats where db_name='db1' and table_name='t1'; +db_name table_name cardinality +# Verify that stats are present in the new database: +select * from mysql.column_stats where db_name='db2' and table_name='t1'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t1 i 10 20 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.table_stats where db_name='db2' and table_name='t1'; +db_name table_name cardinality +db2 t1 2 +# +# Now, try with more than one column and with indexes: +# +use test; +create table t1(a int primary key); +insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +use db1; +create table t2 (a int, b int, c int, key IDX1(a), key IDX2(a,b)); +insert into t2 select a/10, a/2, a from test.t1; +analyze table t2 persistent for all; +Table Op Msg_type Msg_text +db1.t2 analyze status Engine-independent statistics collected +db1.t2 analyze status Table is already up to date +alter table t2 rename db2.t2; +# Verify that stats in the old database are gone: +select * from mysql.table_stats where db_name='db1' and table_name='t2'; +db_name table_name cardinality +select * from mysql.column_stats where db_name='db1' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.index_stats where db_name='db1' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +# Verify that stats are present in the new database: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +db_name table_name cardinality +db2 t2 10 +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t2 a 0 1 0.0000 4.0000 5.0000 0 NULL NULL +db2 t2 b 0 5 0.0000 4.0000 1.6667 0 NULL NULL +db2 t2 c 0 9 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.index_stats where db_name='db2' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +db2 t2 IDX1 1 5.0000 +db2 t2 IDX2 1 5.0000 +db2 t2 IDX2 2 1.6667 +use db2; +# +# Now, rename within the same database and verify: +# +rename table t2 to t3; +# No stats under old name: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +db_name table_name cardinality +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.index_stats where db_name='db2' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +# Stats under the new name: +select * from mysql.table_stats where db_name='db2' and table_name='t3'; +db_name table_name cardinality +db2 t3 10 +select * from mysql.column_stats where db_name='db2' and table_name='t3'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t3 a 0 1 0.0000 4.0000 5.0000 0 NULL NULL +db2 t3 b 0 5 0.0000 4.0000 1.6667 0 NULL NULL +db2 t3 c 0 9 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.index_stats where db_name='db2' and table_name='t3'; +db_name table_name index_name prefix_arity avg_frequency +db2 t3 IDX1 1 5.0000 +db2 t3 IDX2 1 5.0000 +db2 t3 IDX2 2 1.6667 +use test; +drop database db1; +drop database db2; +drop table t1; set use_stat_tables=@save_use_stat_tables; diff --git a/mysql-test/r/stat_tables_innodb.result b/mysql-test/r/stat_tables_innodb.result index 301c093ce9c..0e866755532 100644 --- a/mysql-test/r/stat_tables_innodb.result +++ b/mysql-test/r/stat_tables_innodb.result @@ -448,6 +448,101 @@ id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t1 const PRIMARY PRIMARY 4 const 1 Using index 1 SIMPLE t2 ALL NULL NULL NULL NULL 2 Using where DROP TABLE t1,t2; +# +# MDEV-7370: Server deadlocks on renaming a table for which persistent statistics exists +# +drop database if exists db1; +drop database if exists db1; +create database db1; +create database db2; +use db1; +# +# First, run the original testcase: +# +create table t1 (i int); +insert into t1 values (10),(20); +analyze table t1 persistent for all; +Table Op Msg_type Msg_text +db1.t1 analyze status Engine-independent statistics collected +db1.t1 analyze status OK +rename table t1 to db2.t1; +# Verify that stats in the old database are gone: +select * from mysql.column_stats where db_name='db1' and table_name='t1'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.table_stats where db_name='db1' and table_name='t1'; +db_name table_name cardinality +# Verify that stats are present in the new database: +select * from mysql.column_stats where db_name='db2' and table_name='t1'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t1 i 10 20 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.table_stats where db_name='db2' and table_name='t1'; +db_name table_name cardinality +db2 t1 2 +# +# Now, try with more than one column and with indexes: +# +use test; +create table t1(a int primary key); +insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +use db1; +create table t2 (a int, b int, c int, key IDX1(a), key IDX2(a,b)); +insert into t2 select a/10, a/2, a from test.t1; +analyze table t2 persistent for all; +Table Op Msg_type Msg_text +db1.t2 analyze status Engine-independent statistics collected +db1.t2 analyze status OK +alter table t2 rename db2.t2; +# Verify that stats in the old database are gone: +select * from mysql.table_stats where db_name='db1' and table_name='t2'; +db_name table_name cardinality +select * from mysql.column_stats where db_name='db1' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.index_stats where db_name='db1' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +# Verify that stats are present in the new database: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +db_name table_name cardinality +db2 t2 10 +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t2 a 0 1 0.0000 4.0000 5.0000 0 NULL NULL +db2 t2 b 0 5 0.0000 4.0000 1.6667 0 NULL NULL +db2 t2 c 0 9 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.index_stats where db_name='db2' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +db2 t2 IDX1 1 5.0000 +db2 t2 IDX2 1 5.0000 +db2 t2 IDX2 2 1.6667 +use db2; +# +# Now, rename within the same database and verify: +# +rename table t2 to t3; +# No stats under old name: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +db_name table_name cardinality +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +select * from mysql.index_stats where db_name='db2' and table_name='t2'; +db_name table_name index_name prefix_arity avg_frequency +# Stats under the new name: +select * from mysql.table_stats where db_name='db2' and table_name='t3'; +db_name table_name cardinality +db2 t3 10 +select * from mysql.column_stats where db_name='db2' and table_name='t3'; +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram +db2 t3 a 0 1 0.0000 4.0000 5.0000 0 NULL NULL +db2 t3 b 0 5 0.0000 4.0000 1.6667 0 NULL NULL +db2 t3 c 0 9 0.0000 4.0000 1.0000 0 NULL NULL +select * from mysql.index_stats where db_name='db2' and table_name='t3'; +db_name table_name index_name prefix_arity avg_frequency +db2 t3 IDX1 1 5.0000 +db2 t3 IDX2 1 5.0000 +db2 t3 IDX2 2 1.6667 +use test; +drop database db1; +drop database db2; +drop table t1; set use_stat_tables=@save_use_stat_tables; set optimizer_switch=@save_optimizer_switch_for_stat_tables_test; SET SESSION STORAGE_ENGINE=DEFAULT; diff --git a/mysql-test/t/stat_tables.test b/mysql-test/t/stat_tables.test index 25ca322ca0a..4cbaa9e27c8 100644 --- a/mysql-test/t/stat_tables.test +++ b/mysql-test/t/stat_tables.test @@ -232,4 +232,77 @@ SELECT * FROM t1 STRAIGHT_JOIN t2 WHERE name IN ( 'AUS','YEM' ) AND id = 1; DROP TABLE t1,t2; +--echo # +--echo # MDEV-7370: Server deadlocks on renaming a table for which persistent statistics exists +--echo # + +--disable_warnings +drop database if exists db1; +drop database if exists db1; +--enable_warnings + +create database db1; +create database db2; +use db1; +--echo # +--echo # First, run the original testcase: +--echo # +create table t1 (i int); +insert into t1 values (10),(20); +analyze table t1 persistent for all; +rename table t1 to db2.t1; + +--echo # Verify that stats in the old database are gone: +select * from mysql.column_stats where db_name='db1' and table_name='t1'; +select * from mysql.table_stats where db_name='db1' and table_name='t1'; + +--echo # Verify that stats are present in the new database: +select * from mysql.column_stats where db_name='db2' and table_name='t1'; +select * from mysql.table_stats where db_name='db2' and table_name='t1'; + + +--echo # +--echo # Now, try with more than one column and with indexes: +--echo # +use test; +create table t1(a int primary key); +insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); + + +use db1; +create table t2 (a int, b int, c int, key IDX1(a), key IDX2(a,b)); +insert into t2 select a/10, a/2, a from test.t1; +analyze table t2 persistent for all; + +alter table t2 rename db2.t2; + +--echo # Verify that stats in the old database are gone: +select * from mysql.table_stats where db_name='db1' and table_name='t2'; +select * from mysql.column_stats where db_name='db1' and table_name='t2'; +select * from mysql.index_stats where db_name='db1' and table_name='t2'; + +--echo # Verify that stats are present in the new database: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +select * from mysql.index_stats where db_name='db2' and table_name='t2'; + +use db2; +--echo # +--echo # Now, rename within the same database and verify: +--echo # +rename table t2 to t3; +--echo # No stats under old name: +select * from mysql.table_stats where db_name='db2' and table_name='t2'; +select * from mysql.column_stats where db_name='db2' and table_name='t2'; +select * from mysql.index_stats where db_name='db2' and table_name='t2'; +--echo # Stats under the new name: +select * from mysql.table_stats where db_name='db2' and table_name='t3'; +select * from mysql.column_stats where db_name='db2' and table_name='t3'; +select * from mysql.index_stats where db_name='db2' and table_name='t3'; + +use test; +drop database db1; +drop database db2; +drop table t1; + set use_stat_tables=@save_use_stat_tables; diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index 6496e1895fb..e0fd7005cd5 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -274,8 +274,9 @@ do_rename(THD *thd, TABLE_LIST *ren_table, char *new_db, char *new_table_name, LEX_STRING table_name= { ren_table->table_name, ren_table->table_name_length }; LEX_STRING new_table= { (char *) new_alias, strlen(new_alias) }; + LEX_STRING new_db_name= { (char*)new_db, strlen(new_db)}; (void) rename_table_in_stat_tables(thd, &db_name, &table_name, - &db_name, &new_table); + &new_db_name, &new_table); if ((rc= Table_triggers_list::change_table_name(thd, ren_table->db, old_alias, ren_table->table_name, diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index c2150ba9309..e86c84040b4 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -592,6 +592,8 @@ public: stat_file->extra(HA_EXTRA_FLUSH); return FALSE; } + + friend class Stat_table_write_iter; }; @@ -1264,6 +1266,117 @@ public: }; + +/* + An iterator to enumerate statistics table rows which allows to modify + the rows while reading them. + + Used by RENAME TABLE handling to assign new dbname.tablename to statistic + rows. +*/ +class Stat_table_write_iter +{ + Stat_table *owner; + IO_CACHE io_cache; + uchar *rowid_buf; + uint rowid_size; + +public: + Stat_table_write_iter(Stat_table *stat_table_arg) + : owner(stat_table_arg), rowid_buf(NULL), + rowid_size(owner->stat_file->ref_length) + { + my_b_clear(&io_cache); + } + + /* + Initialize the iterator. It will return rows with n_keyparts matching the + curernt values. + + @return false - OK + true - Error + */ + bool init(uint n_keyparts) + { + if (!(rowid_buf= (uchar*)my_malloc(rowid_size, MYF(0)))) + return true; + + if (open_cached_file(&io_cache, mysql_tmpdir, TEMP_PREFIX, + 1024, MYF(MY_WME))) + return true; + + handler *h= owner->stat_file; + uchar key[MAX_KEY_LENGTH]; + uint prefix_len= 0; + for (uint i= 0; i < n_keyparts; i++) + prefix_len += owner->stat_key_info->key_part[i].store_length; + + key_copy(key, owner->record[0], owner->stat_key_info, + prefix_len); + key_part_map prefix_map= (key_part_map) ((1 << n_keyparts) - 1); + h->ha_index_init(owner->stat_key_idx, false); + int res= h->ha_index_read_map(owner->record[0], key, prefix_map, + HA_READ_KEY_EXACT); + if (res) + { + reinit_io_cache(&io_cache, READ_CACHE, 0L, 0, 0); + /* "Key not found" is not considered an error */ + return (res == HA_ERR_KEY_NOT_FOUND)? false: true; + } + + do { + h->position(owner->record[0]); + my_b_write(&io_cache, h->ref, rowid_size); + + } while (!h->ha_index_next_same(owner->record[0], key, prefix_len)); + + /* Prepare for reading */ + reinit_io_cache(&io_cache, READ_CACHE, 0L, 0, 0); + h->ha_index_or_rnd_end(); + if (h->ha_rnd_init(false)) + return true; + + return false; + } + + /* + Read the next row. + + @return + false OK + true No more rows or error. + */ + bool get_next_row() + { + if (!my_b_inited(&io_cache) || my_b_read(&io_cache, rowid_buf, rowid_size)) + return true; /* No more data */ + + handler *h= owner->stat_file; + /* + We should normally be able to find the row that we have rowid for. If we + don't, let's consider this an error. + */ + int res= h->ha_rnd_pos(owner->record[0], rowid_buf); + + return (res==0)? false : true; + } + + void cleanup() + { + if (rowid_buf) + my_free(rowid_buf); + rowid_buf= NULL; + owner->stat_file->ha_index_or_rnd_end(); + close_cached_file(&io_cache); + my_b_clear(&io_cache); + } + + ~Stat_table_write_iter() + { + cleanup(); + } +}; + /* Histogram_builder is a helper class that is used to build histograms for columns @@ -3285,25 +3398,34 @@ int rename_table_in_stat_tables(THD *thd, LEX_STRING *db, LEX_STRING *tab, stat_table= tables[INDEX_STAT].table; Index_stat index_stat(stat_table, db, tab); index_stat.set_full_table_name(); - while (index_stat.find_next_stat_for_prefix(2)) + + Stat_table_write_iter index_iter(&index_stat); + if (index_iter.init(2)) + rc= 1; + while (!index_iter.get_next_row()) { err= index_stat.update_table_name_key_parts(new_db, new_tab); if (err & !rc) rc= 1; index_stat.set_full_table_name(); } + index_iter.cleanup(); /* Rename table in the statistical table column_stats */ stat_table= tables[COLUMN_STAT].table; Column_stat column_stat(stat_table, db, tab); column_stat.set_full_table_name(); - while (column_stat.find_next_stat_for_prefix(2)) + Stat_table_write_iter column_iter(&column_stat); + if (column_iter.init(2)) + rc= 1; + while (!column_iter.get_next_row()) { err= column_stat.update_table_name_key_parts(new_db, new_tab); if (err & !rc) rc= 1; column_stat.set_full_table_name(); } + column_iter.cleanup(); /* Rename table in the statistical table table_stats */ stat_table= tables[TABLE_STAT].table;