From ea2b19dee6a6ed69faffa368c9b1ce9338556299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 27 Sep 2019 16:01:55 +0300 Subject: [PATCH] MDEV-20117: Fix another scenario Thanks to Eugene Kosov for noting that the fix is incomplete. It turns out that on instant DROP/reorder column (MDEV-15562), we must always write the metadata record, even though the table was empty. Alternatively, we should guarantee that all undo log records for the table have been purged. (Attempting to do that by updating table_id leads to other problems; see commit 1b31d8852c00b4bab6e6fe179b97db45ccb8d535.) It would be tempting to remove dict_index_t::clear_instant_alter() altogether, but it turns that we need that when the instant ALTER TABLE operation of a first-time DROP COLUMN is being rolled back. innobase_instant_try(): Clarify a comment. Purge never calls dict_index_t::clear_instant_alter(), but it may invoke dict_index_t::clear_instant_add(). On first-time instant DROP/reorder, always write a metadata record, even if the table is empty. --- .../suite/innodb/r/instant_alter_bugs.result | 15 +++++++++++++ .../suite/innodb/t/instant_alter_bugs.test | 21 +++++++++++++++++++ storage/innobase/handler/handler0alter.cc | 9 +++++--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/innodb/r/instant_alter_bugs.result b/mysql-test/suite/innodb/r/instant_alter_bugs.result index c630432585a..661db48099d 100644 --- a/mysql-test/suite/innodb/r/instant_alter_bugs.result +++ b/mysql-test/suite/innodb/r/instant_alter_bugs.result @@ -1,3 +1,5 @@ +SET @save_frequency= @@GLOBAL.innodb_purge_rseg_truncate_frequency; +SET GLOBAL innodb_purge_rseg_truncate_frequency=1; # # MDEV-17821 Assertion `!page_rec_is_supremum(rec)' failed # in btr_pcur_store_position @@ -295,3 +297,16 @@ ALTER TABLE t DROP COLUMN c, ALGORITHM=INSTANT; SELECT * FROM t; a b DROP TABLE t; +CREATE TABLE t1 (a INT PRIMARY KEY, b INT, c INT, d INT, e INT) ENGINE=InnoDB; +INSERT INTO t1 SET a=1; +INSERT INTO t1 SET a=2; +BEGIN; +UPDATE t1 SET b=1; +DELETE FROM t1; +COMMIT; +ALTER TABLE t1 DROP b, DROP c, DROP d, DROP e; +InnoDB 0 transactions not purged +SELECT * FROM t1; +a +DROP TABLE t1; +SET GLOBAL innodb_purge_rseg_truncate_frequency=@save_frequency; diff --git a/mysql-test/suite/innodb/t/instant_alter_bugs.test b/mysql-test/suite/innodb/t/instant_alter_bugs.test index 338d6972c43..9f49d1d4682 100644 --- a/mysql-test/suite/innodb/t/instant_alter_bugs.test +++ b/mysql-test/suite/innodb/t/instant_alter_bugs.test @@ -1,5 +1,8 @@ --source include/have_innodb.inc +SET @save_frequency= @@GLOBAL.innodb_purge_rseg_truncate_frequency; +SET GLOBAL innodb_purge_rseg_truncate_frequency=1; + --echo # --echo # MDEV-17821 Assertion `!page_rec_is_supremum(rec)' failed --echo # in btr_pcur_store_position @@ -313,3 +316,21 @@ ALTER TABLE t ADD COLUMN c INT, ALGORITHM=INSTANT; ALTER TABLE t DROP COLUMN c, ALGORITHM=INSTANT; SELECT * FROM t; DROP TABLE t; + +# The following is nondeterministically repeating the bug in a +# different scenario: the table is empty at the time the ALTER TABLE +# is invoked, apparently because purge already processed the records +# for the DELETE, but not the record for the UPDATE. +CREATE TABLE t1 (a INT PRIMARY KEY, b INT, c INT, d INT, e INT) ENGINE=InnoDB; +INSERT INTO t1 SET a=1; +INSERT INTO t1 SET a=2; +BEGIN; +UPDATE t1 SET b=1; +DELETE FROM t1; +COMMIT; + +ALTER TABLE t1 DROP b, DROP c, DROP d, DROP e; +--source include/wait_all_purged.inc +SELECT * FROM t1; +DROP TABLE t1; +SET GLOBAL innodb_purge_rseg_truncate_frequency=@save_frequency; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index ab3ec7ba249..5c52f74e79e 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5630,7 +5630,7 @@ static bool innobase_instant_try( dict_index_t* index = dict_table_get_first_index(user_table); mtr_t mtr; mtr.start(); - /* Prevent purge from calling dict_index_t::clear_instant_alter(), + /* Prevent purge from calling dict_index_t::clear_instant_add(), to protect index->n_core_fields, index->table->instant and others from changing during ctx->instant_column(). */ instant_metadata_lock(*index, mtr); @@ -5821,6 +5821,7 @@ add_all_virtual: if (rec_is_metadata(rec, *index)) { ut_ad(page_rec_is_user_rec(rec)); if (!rec_is_alter_metadata(rec, *index) + && !index->table->instant && !page_has_next(block->frame) && page_rec_is_last(rec, block->frame)) { goto empty_table; @@ -5902,7 +5903,7 @@ add_all_virtual: } btr_pcur_close(&pcur); goto func_exit; - } else if (page_rec_is_supremum(rec)) { + } else if (page_rec_is_supremum(rec) && !index->table->instant) { empty_table: /* The table is empty. */ ut_ad(fil_page_index_page_check(block->frame)); @@ -5910,7 +5911,9 @@ empty_table: ut_ad(block->page.id.page_no() == index->page); /* MDEV-17383: free metadata BLOBs! */ btr_page_empty(block, NULL, index, 0, &mtr); - index->clear_instant_alter(); + if (index->is_instant()) { + index->clear_instant_add(); + } goto func_exit; } else if (!user_table->is_instant()) { ut_ad(!user_table->not_redundant());