From 149b75476837fb96c28739d5368e977e39fd671b Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 7 Mar 2019 13:43:53 +0400 Subject: [PATCH 1/2] MDEV-17595 - ALTER TABLE ADD FOREIGN KEY crash ALTER TABLE ... ADD FOREIGN KEY may trigger assertion failure when it has LOCK=EXCLUSIVE clause or concurrent FLUSH TABLES is being executed. In both cases being altered table is marked as flushed, which forces subsequent attempt to open parent table to re-open. Which in turn is not allowed while transaction is running. Rather than opening parent table, just take appropriate MDL lock. Also removed table_already_fk_prelocked() check: MDL itself has much better methods to handle duplicate locks. E.g. the former won't acquire MDL_SHARED_NO_WRITE if it already has MDL_SHARED_READ. --- mysql-test/suite/innodb/r/foreign-keys.result | 13 ++++++++++++ mysql-test/suite/innodb/t/foreign-keys.test | 15 ++++++++++++++ sql/sql_table.cc | 20 ++++++++----------- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/mysql-test/suite/innodb/r/foreign-keys.result b/mysql-test/suite/innodb/r/foreign-keys.result index 66fc00e34d0..68528521fb6 100644 --- a/mysql-test/suite/innodb/r/foreign-keys.result +++ b/mysql-test/suite/innodb/r/foreign-keys.result @@ -87,3 +87,16 @@ drop table t3; drop table t2; drop table t1; set debug_sync='reset'; +# +# MDEV-17595 - Server crashes in copy_data_between_tables or +# Assertion `thd->transaction.stmt.is_empty() || +# (thd->state_flags & Open_tables_state::BACKUPS_AVAIL)' +# fails in close_tables_for_reopen upon concurrent +# ALTER TABLE and FLUSH +# +CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB; +INSERT INTO t1 VALUES(1),(2); +CREATE TABLE t2 (b INT, KEY(b)) ENGINE=InnoDB; +INSERT INTO t2 VALUES(2); +ALTER TABLE t2 ADD FOREIGN KEY(b) REFERENCES t1(a), LOCK=EXCLUSIVE; +DROP TABLE t2, t1; diff --git a/mysql-test/suite/innodb/t/foreign-keys.test b/mysql-test/suite/innodb/t/foreign-keys.test index 7ef440b260b..ced44a89d7c 100644 --- a/mysql-test/suite/innodb/t/foreign-keys.test +++ b/mysql-test/suite/innodb/t/foreign-keys.test @@ -111,3 +111,18 @@ drop table t3; drop table t2; drop table t1; set debug_sync='reset'; + + +--echo # +--echo # MDEV-17595 - Server crashes in copy_data_between_tables or +--echo # Assertion `thd->transaction.stmt.is_empty() || +--echo # (thd->state_flags & Open_tables_state::BACKUPS_AVAIL)' +--echo # fails in close_tables_for_reopen upon concurrent +--echo # ALTER TABLE and FLUSH +--echo # +CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB; +INSERT INTO t1 VALUES(1),(2); +CREATE TABLE t2 (b INT, KEY(b)) ENGINE=InnoDB; +INSERT INTO t2 VALUES(2); +ALTER TABLE t2 ADD FOREIGN KEY(b) REFERENCES t1(a), LOCK=EXCLUSIVE; +DROP TABLE t2, t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 8a4c6e025fb..3b4ba72b1f1 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -9074,6 +9074,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, new_table->file->get_foreign_key_list(thd, &fk_list); while ((fk= fk_list_it++)) { + MDL_request mdl_request; + if (lower_case_table_names) { char buf[NAME_LEN]; @@ -9085,20 +9087,14 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, len = my_casedn_str(files_charset_info, buf); thd->make_lex_string(fk->referenced_table, buf, len); } - if (table_already_fk_prelocked(table_list, fk->referenced_db, - fk->referenced_table, TL_READ_NO_INSERT)) - continue; - TABLE_LIST *tl= (TABLE_LIST *) thd->alloc(sizeof(TABLE_LIST)); - tl->init_one_table_for_prelocking(fk->referenced_db->str, fk->referenced_db->length, - fk->referenced_table->str, fk->referenced_table->length, - NULL, TL_READ_NO_INSERT, false, NULL, 0, - &thd->lex->query_tables_last); + mdl_request.init(MDL_key::TABLE, + fk->referenced_db->str, fk->referenced_table->str, + MDL_SHARED_NO_WRITE, MDL_TRANSACTION); + if (thd->mdl_context.acquire_lock(&mdl_request, + thd->variables.lock_wait_timeout)) + goto err_new_table_cleanup; } - - if (open_tables(thd, &table_list->next_global, &tables_opened, 0, - &alter_prelocking_strategy)) - goto err_new_table_cleanup; } } /* From 32de60bb2e9f06b24c76fc41bef81e35c14ca528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 12 Mar 2019 12:55:38 +0200 Subject: [PATCH 2/2] MDEV-18749: Fix GCC -flifetime-dse row_merge_create_fts_sort_index(): Initialize dict_col_t in an unambiguous way. GCC 6 and later appear to be able to optimize away the memset() that is part of mem_heap_zalloc() in the placement new call. Let us avoid using placement new in order to ensure that the objects will actually be initialized. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71388 https://gcc.gnu.org/ml/gcc/2016-02/msg00207.html While the latter reference hints that the optimization is only applicable to non-POD types (and dict_col_t does not define any member functions before 10.2), it is most consistent to use the same initialization across all versions. --- storage/innobase/row/row0ftsort.cc | 12 ++++++------ storage/xtradb/row/row0ftsort.cc | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/row0ftsort.cc index 6167fdc63d6..e3ac4e77f0f 100644 --- a/storage/innobase/row/row0ftsort.cc +++ b/storage/innobase/row/row0ftsort.cc @@ -98,8 +98,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 0); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->prtype = idx_field->col->prtype | DATA_NOT_NULL; field->col->mtype = charset == &my_charset_latin1 ? DATA_VARCHAR : DATA_VARMYSQL; @@ -113,8 +113,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 1); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->mtype = DATA_INT; *opt_doc_id_size = FALSE; @@ -152,8 +152,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 2); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->mtype = DATA_INT; field->col->len = 4 ; field->fixed_len = 4; diff --git a/storage/xtradb/row/row0ftsort.cc b/storage/xtradb/row/row0ftsort.cc index 1798f997033..fb30f867f87 100644 --- a/storage/xtradb/row/row0ftsort.cc +++ b/storage/xtradb/row/row0ftsort.cc @@ -101,8 +101,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 0); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->prtype = idx_field->col->prtype | DATA_NOT_NULL; field->col->mtype = charset == &my_charset_latin1 ? DATA_VARCHAR : DATA_VARMYSQL; @@ -116,8 +116,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 1); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->mtype = DATA_INT; *opt_doc_id_size = FALSE; @@ -155,8 +155,8 @@ row_merge_create_fts_sort_index( field = dict_index_get_nth_field(new_index, 2); field->name = NULL; field->prefix_len = 0; - field->col = new(mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))) - dict_col_t(); + field->col = static_cast( + mem_heap_zalloc(new_index->heap, sizeof(dict_col_t))); field->col->mtype = DATA_INT; field->col->len = 4 ; field->fixed_len = 4;