[Commits] 3d8d010: MDEV-7370: Server deadlocks on renaming a table for which persistent statistics exists

Sergei Petrunia psergey at askmonty.org
Wed Nov 18 19:52:51 EET 2015


revision-id: 3d8d010594f7b0e20ef32d0d97de4e987427008c
parent(s): 3228ad33f0ae8e2045449c3e67bcd1fbf5ed6543
committer: Sergei Petrunia
branch nick: 10.1-dbg3
timestamp: 2015-11-18 20:52:51 +0300
message:

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, bufferthe 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).

---
 mysql-test/r/stat_tables.result        |   69 +++++++++++++++++
 mysql-test/r/stat_tables_innodb.result |   69 +++++++++++++++++
 mysql-test/t/stat_tables.test          |   59 +++++++++++++++
 sql/sql_rename.cc                      |    3 +-
 sql/sql_statistics.cc                  |  126 +++++++++++++++++++++++++++++++-
 5 files changed, 323 insertions(+), 3 deletions(-)

diff --git a/mysql-test/r/stat_tables.result b/mysql-test/r/stat_tables.result
index 2852845..8c6aa25 100644
--- a/mysql-test/r/stat_tables.result
+++ b/mysql-test/r/stat_tables.result
@@ -421,4 +421,73 @@ 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 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 301c093..8832a7c 100644
--- a/mysql-test/r/stat_tables_innodb.result
+++ b/mysql-test/r/stat_tables_innodb.result
@@ -448,6 +448,75 @@ 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 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 25ca322..a50db8d 100644
--- a/mysql-test/t/stat_tables.test
+++ b/mysql-test/t/stat_tables.test
@@ -232,4 +232,63 @@ 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 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 d9cf034..17b297f 100644
--- a/sql/sql_rename.cc
+++ b/sql/sql_rename.cc
@@ -274,8 +274,9 @@ static TABLE_LIST *reverse_table_list(TABLE_LIST *table_list)
         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 021e4c3..da76c6a 100644
--- a/sql/sql_statistics.cc
+++ b/sql/sql_statistics.cc
@@ -592,6 +592,8 @@ class Stat_table
     stat_file->extra(HA_EXTRA_FLUSH);
     return FALSE;
   } 
+
+  friend class Stat_table_write_iter;
 };
 
 
@@ -1264,6 +1266,117 @@ class Index_stat: public Stat_table
 
 };
 
+
+/*
+  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;


More information about the commits mailing list