From eb38b1f703fb84299680f9c5a75ea970be7aee1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 10:41:06 +0300 Subject: [PATCH 1/6] Revert "MDEV-23776 Test encryption.create_or_replace fails with a warning" This reverts commit e33f7b6faaacf49881fd913115b0735c62aba395. The change seems to have introduced intermittent failures of the test encryption.innodb-bad-key-change on many platforms. The failure that we were trying to address was not reproduced on 10.2. It could be related to commit a7dd7c899356b2d3a7f79e6ebba5d854ed63ae9d (MDEV-23651) or de942c9f618b590a01a7960c171d7e50e435708f (MDEV-15983) or other changes that reduced contention on fil_system.mutex in 10.3. The fix that we are hereby reverting from 10.2 seems to work fine on 10.3 and 10.4. --- storage/innobase/fil/fil0crypt.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 16b2093b691..09f2730edec 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1483,11 +1483,6 @@ static bool fil_crypt_find_space_to_rotate( { /* we need iops to start rotating */ while (!state->should_shutdown() && !fil_crypt_alloc_iops(state)) { - if (state->space && state->space->is_stopping()) { - fil_space_release(state->space); - state->space = NULL; - } - os_event_reset(fil_crypt_threads_event); os_event_wait_time(fil_crypt_threads_event, 100000); } @@ -2511,7 +2506,6 @@ fil_space_crypt_close_tablespace( /* wakeup throttle (all) sleepers */ os_event_set(fil_crypt_throttle_sleep_event); - os_event_set(fil_crypt_threads_event); os_thread_sleep(20000); dict_mutex_enter_for_mysql(); From 732cd7fd5346885ae41293d7ebe44110188155d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 11:13:51 +0300 Subject: [PATCH 2/6] MDEV-23705 Assertion 'table->data_dir_path || !space' After DISCARD TABLESPACE, the tablespace of a table will no longer exist, and dict_get_and_save_data_dir_path() would invoke dict_get_first_path() to read an entry from SYS_DATAFILES. For some reason, DISCARD TABLESPACE would not to remove the entry from there. dict_get_and_save_data_dir_path(): If the tablespace has been discarded, do not bother trying to read the name. Side note: The tables SYS_TABLESPACES and SYS_DATAFILES are redundant and subject to removal in MDEV-22343. --- mysql-test/suite/innodb/r/truncate.result | 11 +++++++++++ mysql-test/suite/innodb/t/truncate.test | 10 ++++++++++ storage/innobase/dict/dict0load.cc | 3 ++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/innodb/r/truncate.result b/mysql-test/suite/innodb/r/truncate.result index 5e9fd60d4fe..571aa2da1c5 100644 --- a/mysql-test/suite/innodb/r/truncate.result +++ b/mysql-test/suite/innodb/r/truncate.result @@ -39,3 +39,14 @@ TRUNCATE t1; SELECT * FROM t1; a DROP TEMPORARY TABLE t1; +# +# MDEV-23705 Assertion 'table->data_dir_path || !space' +# +CREATE TABLE t(c INT) ENGINE=InnoDB; +ALTER TABLE t DISCARD TABLESPACE; +RENAME TABLE t TO u; +TRUNCATE u; +Warnings: +Warning 1814 Tablespace has been discarded for table `u` +TRUNCATE u; +DROP TABLE u; diff --git a/mysql-test/suite/innodb/t/truncate.test b/mysql-test/suite/innodb/t/truncate.test index cd1d827e157..ca9ccb677e9 100644 --- a/mysql-test/suite/innodb/t/truncate.test +++ b/mysql-test/suite/innodb/t/truncate.test @@ -50,3 +50,13 @@ INSERT INTO t1 VALUES(1); TRUNCATE t1; SELECT * FROM t1; DROP TEMPORARY TABLE t1; + +--echo # +--echo # MDEV-23705 Assertion 'table->data_dir_path || !space' +--echo # +CREATE TABLE t(c INT) ENGINE=InnoDB; +ALTER TABLE t DISCARD TABLESPACE; +RENAME TABLE t TO u; +TRUNCATE u; +TRUNCATE u; +DROP TABLE u; diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc index 4014d28c2fb..af14c6a4770 100644 --- a/storage/innobase/dict/dict0load.cc +++ b/storage/innobase/dict/dict0load.cc @@ -2739,7 +2739,8 @@ dict_get_and_save_data_dir_path( { ut_ad(!dict_table_is_temporary(table)); - if (!table->data_dir_path && table->space) { + if (!table->data_dir_path && table->space + && !dict_table_is_discarded(table)) { char* path = fil_space_get_first_path(table->space); if (!dict_mutex_own) { From 2af8f712de3a9d518a70904e65ff8cf18beb5f1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 13:08:09 +0300 Subject: [PATCH 3/6] MDEV-23776: Re-apply the fix and make the test more robust The test that was added in commit e05650e6868eab2dbb9f58c4786bcc71afc4ffce would break a subsequent run of a test encryption.innodb-bad-key-change because some pages in the system tablespace would be encrypted with a different key. The failure was repeatable with the following invocation: ./mtr --no-reorder \ encryption.create_or_replace,cbc \ encryption.innodb-bad-key-change,cbc Because the crash was unrelated to the code changes that we reverted in commit eb38b1f703fb84299680f9c5a75ea970be7aee1d we can safely re-apply those fixes. --- mysql-test/suite/encryption/r/create_or_replace.result | 2 -- mysql-test/suite/encryption/t/create_or_replace.opt | 1 + mysql-test/suite/encryption/t/create_or_replace.test | 2 -- storage/innobase/fil/fil0crypt.cc | 6 ++++++ 4 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 mysql-test/suite/encryption/t/create_or_replace.opt diff --git a/mysql-test/suite/encryption/r/create_or_replace.result b/mysql-test/suite/encryption/r/create_or_replace.result index 62dac4ccc43..69ea113289b 100644 --- a/mysql-test/suite/encryption/r/create_or_replace.result +++ b/mysql-test/suite/encryption/r/create_or_replace.result @@ -1,5 +1,4 @@ SET @save_threads = @@GLOBAL.innodb_encryption_threads; -SET @save_tables = @@GLOBAL.innodb_encrypt_tables; SET default_storage_engine = InnoDB; SET GLOBAL innodb_encryption_threads = 4; CREATE TABLE `table10_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int, key (`col_int_key` ),primary key (pk)) engine=innodb; @@ -18,4 +17,3 @@ connection default; drop table create_or_replace_t, table1_int_autoinc, table0_int_autoinc, table10_int_autoinc; SET GLOBAL innodb_encryption_threads = @save_threads; -SET GLOBAL innodb_encrypt_tables = @save_tables; diff --git a/mysql-test/suite/encryption/t/create_or_replace.opt b/mysql-test/suite/encryption/t/create_or_replace.opt new file mode 100644 index 00000000000..66892f34897 --- /dev/null +++ b/mysql-test/suite/encryption/t/create_or_replace.opt @@ -0,0 +1 @@ +--innodb-encrypt-tables diff --git a/mysql-test/suite/encryption/t/create_or_replace.test b/mysql-test/suite/encryption/t/create_or_replace.test index 8d571794713..2ebd599d460 100644 --- a/mysql-test/suite/encryption/t/create_or_replace.test +++ b/mysql-test/suite/encryption/t/create_or_replace.test @@ -3,7 +3,6 @@ --source include/count_sessions.inc SET @save_threads = @@GLOBAL.innodb_encryption_threads; -SET @save_tables = @@GLOBAL.innodb_encrypt_tables; SET default_storage_engine = InnoDB; @@ -76,5 +75,4 @@ drop table create_or_replace_t, table1_int_autoinc, table0_int_autoinc, table10_int_autoinc; SET GLOBAL innodb_encryption_threads = @save_threads; -SET GLOBAL innodb_encrypt_tables = @save_tables; --source include/wait_until_count_sessions.inc diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 09f2730edec..16b2093b691 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1483,6 +1483,11 @@ static bool fil_crypt_find_space_to_rotate( { /* we need iops to start rotating */ while (!state->should_shutdown() && !fil_crypt_alloc_iops(state)) { + if (state->space && state->space->is_stopping()) { + fil_space_release(state->space); + state->space = NULL; + } + os_event_reset(fil_crypt_threads_event); os_event_wait_time(fil_crypt_threads_event, 100000); } @@ -2506,6 +2511,7 @@ fil_space_crypt_close_tablespace( /* wakeup throttle (all) sleepers */ os_event_set(fil_crypt_throttle_sleep_event); + os_event_set(fil_crypt_threads_event); os_thread_sleep(20000); dict_mutex_enter_for_mysql(); From e5e83daf32300115d857b49fdbd35460bbf51d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 13:09:51 +0300 Subject: [PATCH 4/6] Make DISCARD TABLESPACE more robust dict_load_table_low(): Copy the 'discarded' flag to file_unreadable. This allows to avoid a potentially harmful call to dict_stats_init() in ha_innobase::open(). --- storage/innobase/dict/dict0load.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc index af14c6a4770..22fc70a4e42 100644 --- a/storage/innobase/dict/dict0load.cc +++ b/storage/innobase/dict/dict0load.cc @@ -2686,7 +2686,7 @@ static const char* dict_load_table_low(const table_name_t& name, *table = dict_mem_table_create( name.m_name, space_id, n_cols + n_v_col, n_v_col, flags, flags2); (*table)->id = table_id; - (*table)->file_unreadable = false; + (*table)->file_unreadable = !!(flags2 & DICT_TF2_DISCARDED); return(NULL); } From 3eb81136e1c087af3fef38ae89e3357f4c306f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 13:40:05 +0300 Subject: [PATCH 5/6] MDEV-22939 Server crashes in row_make_new_pathname() The statement ALTER TABLE...DISCARD TABLESPACE is problematic, because its designed purpose is to break the referential integrity of the data dictionary and make a table point to nowhere. ha_innobase::commit_inplace_alter_table(): Check whether the table has been discarded. (This is a bit late to check it, right before committing the change.) Previously, we performed this check only in a specific branch of the function commit_set_autoinc(). Note: We intentionally allow non-rebuilding ALTER TABLE even if the tablespace has been discarded, to remain compatible with MySQL. (See the various tests with "wl5522" in the name, such as innodb.innodb-wl5522.) The test case would crash starting with 10.3 only, but it does not hurt to minimize the code and test difference between 10.2 and 10.3. --- mysql-test/suite/innodb/r/alter_table.result | 10 ++++++++++ mysql-test/suite/innodb/t/alter_table.test | 11 +++++++++++ storage/innobase/handler/handler0alter.cc | 18 +++++++++--------- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/mysql-test/suite/innodb/r/alter_table.result b/mysql-test/suite/innodb/r/alter_table.result index 94262ac29c3..fc08f29e515 100644 --- a/mysql-test/suite/innodb/r/alter_table.result +++ b/mysql-test/suite/innodb/r/alter_table.result @@ -60,3 +60,13 @@ CREATE TABLE t1(a INT NOT NULL UNIQUE) ENGINE=InnoDB; INSERT INTO t1 SELECT * FROM seq_1_to_128; ALTER TABLE t1 ADD b TINYINT AUTO_INCREMENT PRIMARY KEY, DROP KEY a; DROP TABLE t1; +# +# MDEV-22939 Server crashes in row_make_new_pathname() +# +CREATE TABLE t (a INT) ENGINE=INNODB; +ALTER TABLE t DISCARD TABLESPACE; +ALTER TABLE t ENGINE INNODB; +ERROR HY000: Tablespace has been discarded for table `t` +ALTER TABLE t FORCE; +ERROR HY000: Tablespace has been discarded for table `t` +DROP TABLE t; diff --git a/mysql-test/suite/innodb/t/alter_table.test b/mysql-test/suite/innodb/t/alter_table.test index 6bc30d2e8ee..5050abdc087 100644 --- a/mysql-test/suite/innodb/t/alter_table.test +++ b/mysql-test/suite/innodb/t/alter_table.test @@ -68,3 +68,14 @@ CREATE TABLE t1(a INT NOT NULL UNIQUE) ENGINE=InnoDB; INSERT INTO t1 SELECT * FROM seq_1_to_128; ALTER TABLE t1 ADD b TINYINT AUTO_INCREMENT PRIMARY KEY, DROP KEY a; DROP TABLE t1; + +--echo # +--echo # MDEV-22939 Server crashes in row_make_new_pathname() +--echo # +CREATE TABLE t (a INT) ENGINE=INNODB; +ALTER TABLE t DISCARD TABLESPACE; +--error ER_TABLESPACE_DISCARDED +ALTER TABLE t ENGINE INNODB; +--error ER_TABLESPACE_DISCARDED +ALTER TABLE t FORCE; +DROP TABLE t; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 09bfc5f5503..f0b12183b22 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -7209,12 +7209,6 @@ commit_set_autoinc( && (ha_alter_info->create_info->used_fields & HA_CREATE_USED_AUTO)) { - if (dict_table_is_discarded(ctx->old_table)) { - my_error(ER_TABLESPACE_DISCARDED, MYF(0), - old_table->s->table_name.str); - DBUG_RETURN(true); - } - /* An AUTO_INCREMENT value was supplied by the user. It must be persisted to the data file. */ const Field* ai = old_table->found_next_number_field; @@ -8411,9 +8405,15 @@ ha_innobase::commit_inplace_alter_table( = static_cast(*pctx); DBUG_ASSERT(new_clustered == ctx->need_rebuild()); - - fail = commit_set_autoinc(ha_alter_info, ctx, altered_table, - table); + if (ctx->need_rebuild() + && dict_table_is_discarded(ctx->old_table)) { + my_error(ER_TABLESPACE_DISCARDED, MYF(0), + table->s->table_name.str); + fail = true; + } else { + fail = commit_set_autoinc(ha_alter_info, ctx, + altered_table, table); + } if (fail) { } else if (ctx->need_rebuild()) { From 78efa1093076307946873f322b45a14d1ed5ffb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 22 Sep 2020 13:55:36 +0300 Subject: [PATCH 6/6] MDEV-22939: Restore an AUTO_INCREMENT check It turns out that we must check for DISCARD TABLESPACE both when the table is being rebuilt and when the AUTO_INCREMENT value of the table is being added. This was caught by the test innodb.alter_missing_tablespace. Somehow I failed to run all tests. Sorry! --- storage/innobase/handler/handler0alter.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index f0b12183b22..7eacb83fa2f 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -7209,6 +7209,12 @@ commit_set_autoinc( && (ha_alter_info->create_info->used_fields & HA_CREATE_USED_AUTO)) { + if (dict_table_is_discarded(ctx->old_table)) { + my_error(ER_TABLESPACE_DISCARDED, MYF(0), + old_table->s->table_name.str); + DBUG_RETURN(true); + } + /* An AUTO_INCREMENT value was supplied by the user. It must be persisted to the data file. */ const Field* ai = old_table->found_next_number_field;