From 1f4ee3fa5a80e9715b0dc77b72fabb95a4045745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 26 Sep 2019 19:45:10 +0300 Subject: [PATCH] MDEV-20117 Assertion 0 failed in row_sel_get_clust_rec_for_mysql The crash scenario is as follows: (1) A non-empty table exists. (2) MDEV-15562 instant ADD/DROP/reorder has been invoked. (3) Some purgeable undo log exists for the table. (4) The table becomes empty, containing not even any delete-marked records, only containing the hidden metadata record that was added in (2). (5) An instant ADD/DROP/reorder column is executed, and the table is emptied and the (2) metadata removed. (6) Purge processes an undo log record from (3), which will refer to a non-existent clustered index field, because the metadata that was created in (2) was remoeved in (5). We fix this by adjusting step (5) so that we will never remove the MDEV-15562-style metadata record. Removing the MDEV-11369 metadata record (instant ADD COLUMN to the end of the table) is completely fine at any time when the table becomes empty, because dict_index_t::n_fields will remain unchanged. innobase_instant_try(): Never remove the MDEV-15562 metadata record. page_cur_delete_rec(): Do not reset FIL_PAGE_TYPE when the MDEV-15562 metadata record is being removed as part of btr_cur_pessimistic_update() invoked by innobase_instant_try(). --- .../suite/innodb/r/instant_alter_bugs.result | 12 +++++++++++ .../suite/innodb/t/instant_alter_bugs.test | 20 +++++++++++++++++++ storage/innobase/handler/handler0alter.cc | 3 ++- storage/innobase/page/page0cur.cc | 3 ++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/innodb/r/instant_alter_bugs.result b/mysql-test/suite/innodb/r/instant_alter_bugs.result index 045b7468049..c630432585a 100644 --- a/mysql-test/suite/innodb/r/instant_alter_bugs.result +++ b/mysql-test/suite/innodb/r/instant_alter_bugs.result @@ -283,3 +283,15 @@ ALTER TABLE t CHANGE COLUMN alpha a INT WITHOUT SYSTEM VERSIONING, ALGORITHM=INSTANT; DROP TABLE t; set @@system_versioning_alter_history = error; +# +# MDEV-20117 Assertion 0 failed in row_sel_get_clust_rec_for_mysql +# +CREATE TABLE t (b INT PRIMARY KEY) ENGINE=InnoDB; +INSERT INTO t SET b=1; +ALTER TABLE t ADD COLUMN a INT FIRST, ALGORITHM=INSTANT; +DELETE FROM t; +ALTER TABLE t ADD COLUMN c INT, ALGORITHM=INSTANT; +ALTER TABLE t DROP COLUMN c, ALGORITHM=INSTANT; +SELECT * FROM t; +a b +DROP TABLE t; diff --git a/mysql-test/suite/innodb/t/instant_alter_bugs.test b/mysql-test/suite/innodb/t/instant_alter_bugs.test index fda8e88d70c..338d6972c43 100644 --- a/mysql-test/suite/innodb/t/instant_alter_bugs.test +++ b/mysql-test/suite/innodb/t/instant_alter_bugs.test @@ -293,3 +293,23 @@ ALTER TABLE t CHANGE COLUMN alpha a INT WITHOUT SYSTEM VERSIONING, ALGORITHM=INSTANT; DROP TABLE t; set @@system_versioning_alter_history = error; + +--echo # +--echo # MDEV-20117 Assertion 0 failed in row_sel_get_clust_rec_for_mysql +--echo # + +# This is not repeating the bug itself, but demonstrating that both +# parts of the fix are needed. +# To repeat the original bug, we should be somehow able to empty +# the table of user records while purgeable undo log records exist. +CREATE TABLE t (b INT PRIMARY KEY) ENGINE=InnoDB; +INSERT INTO t SET b=1; +ALTER TABLE t ADD COLUMN a INT FIRST, ALGORITHM=INSTANT; +DELETE FROM t; +ALTER TABLE t ADD COLUMN c INT, ALGORITHM=INSTANT; +# If page_cur_delete_rec() emptied the page (and wrongly reset the +# page type) during the previous ALTER TABLE, the following would hit +# an assertion failure because of root page type mismatch. +ALTER TABLE t DROP COLUMN c, ALGORITHM=INSTANT; +SELECT * FROM t; +DROP TABLE t; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 14125272daf..ab3ec7ba249 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5820,7 +5820,8 @@ add_all_virtual: dberr_t err = DB_SUCCESS; if (rec_is_metadata(rec, *index)) { ut_ad(page_rec_is_user_rec(rec)); - if (!page_has_next(block->frame) + if (!rec_is_alter_metadata(rec, *index) + && !page_has_next(block->frame) && page_rec_is_last(rec, block->frame)) { goto empty_table; } diff --git a/storage/innobase/page/page0cur.cc b/storage/innobase/page/page0cur.cc index ded90c1c4f8..c2bdaee8c13 100644 --- a/storage/innobase/page/page0cur.cc +++ b/storage/innobase/page/page0cur.cc @@ -2327,7 +2327,8 @@ page_cur_delete_rec( /* The record must not be the supremum or infimum record. */ ut_ad(page_rec_is_user_rec(current_rec)); - if (page_get_n_recs(page) == 1 && !recv_recovery_is_on()) { + if (page_get_n_recs(page) == 1 && !recv_recovery_is_on() + && !rec_is_alter_metadata(current_rec, *index)) { /* Empty the page, unless we are applying the redo log during crash recovery. During normal operation, the page_create_empty() gets logged as one of MLOG_PAGE_CREATE,