[MDEV-17735] Assertion failure in row_parse_int() on first ADD/DROP COLUMN when an AUTO_INCREMENT column is not in PRIMARY KEY - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Major
Resolution: Fixed
Affects Version/s: 10.4.0
Fix Version/s: 10.4.1
Component/s: Storage Engine - InnoDB
Labels:
- crash
- instant

Description

This was first reported by elenst in ~~MDEV-17721~~:

diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test

index fd7fbe94450..4dd00a227f9 100644

--- a/mysql-test/suite/innodb/t/instant_alter.test

+++ b/mysql-test/suite/innodb/t/instant_alter.test

@@ -397,6 +397,14 @@ ALTER TABLE t1 ADD COLUMN t TEXT;

 SELECT * FROM t1;

 DROP TABLE t1;

+eval CREATE TABLE t1 (a INT AUTO_INCREMENT, b INT, KEY(a)) $engine;

+INSERT INTO t1 SET a=NULL;

+ALTER TABLE t1 DROP COLUMN b;

+INSERT INTO t1 SET a=NULL;

+UPDATE t1 SET a=a+2;

+SELECT * FROM t1;

+DROP TABLE t1;

 dec $format;

 disconnect analyze;

With this change to the test, InnoDB would crash as follows:

10.4 89337d510e9780d6533c3db4d42969b2338a7610
CURRENT_TEST: innodb.instant_alter
mysqltest: At line 402: query 'ALTER TABLE t1 DROP COLUMN b' failed: 2013: Lost connection to MySQL server during query
…
2018-11-16 12:36:36 0x7ff5ac4c1700 InnoDB: Assertion failure in file /mariadb/10.4/storage/innobase/include/row0row.ic line 215
…
#6 0x000056295ac4c0e5 in ut_dbg_assertion_failed (expr=0x0, file=0x56295b2b6780 "/mariadb/10.4/storage/innobase/include/row0row.ic", line=215) at /mariadb/10.4/storage/innobase/ut/ut0dbg.cc:61
#7 0x000056295ab702a2 in row_parse_int (data=0x7ff5541cfbf0 "", len=8, mtype=5, unsigned_type=false) at /mariadb/10.4/storage/innobase/include/row0row.ic:215
#8 0x000056295ab6c432 in row_ins_clust_index_entry_low (flags=2, mode=33, index=0x7ff554106a28, n_uniq=1, entry=0x7ff5541cfb18, n_ext=0, thr=0x7ff5541cfcb8, dup_chk_only=false) at /mariadb/10.4/storage/innobase/row/row0ins.cc:2613
#9 0x000056295aa6726c in innobase_instant_try (ha_alter_info=0x7ff5ac4bdb60, ctx=0x7ff5540153e8, altered_table=0x7ff5541cd528, table=0x7ff55419c618, trx=0x7ff5ac75b300) at /mariadb/10.4/storage/innobase/handler/handler0alter.cc:5522

This should only occur if a table contains an AUTO_INCREMENT column that is not part of the PRIMARY KEY, and if the first instant ALTER TABLE is one of the ~~MDEV-15562~~ operations: permuting columns, adding a column somewhere else than the last column, or dropping a column.
If there was a prior ~~MDEV-11369~~ instant ADD COLUMN operation on the table, then this code would not be executed.

This code is part of the ~~MDEV-6076~~ logic, to ensure that the persistent AUTO_INCREMENT value of the table will be updated on every INSERT. At this point of the execution, index->table->persistent_autoinc is pointing to the wrong index field.

So, we are trying to read an AUTO_INCREMENT value from the wrong position, from a hidden metadata record where the AUTO_INCREMENT value surely does not matter. The position is off by one, because in the ~~MDEV-15562~~ metadata record, there is a metadata BLOB after the index->first_user_record().

In 10.3, there is no issue, because the dummy value for the AUTO_INCREMENT column in the ~~MDEV-11369~~ metadata record would be 0 or NULL, and because the field will be (unnecessarily) accessed at the correct position.

We can avoid the unnecessary and harmful access during ALTER TABLE:

diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc

index 777d69cf127..1014168d389 100644

--- a/storage/innobase/row/row0ins.cc

+++ b/storage/innobase/row/row0ins.cc

@@ -2597,25 +2597,32 @@ row_ins_clust_index_entry_low(

 	} else {

 		index->set_modified(mtr);

-		if (mode == BTR_MODIFY_LEAF

-		    && dict_index_is_online_ddl(index)) {

-			mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED;

-			mtr_s_lock(dict_index_get_lock(index), &mtr);

-		}

+		if (UNIV_UNLIKELY(entry->is_metadata())) {

+			ut_ad(index->is_instant());

+			ut_ad(!dict_index_is_online_ddl(index));

+			ut_ad(mode == BTR_MODIFY_TREE);

+		} else {

+			if (mode == BTR_MODIFY_LEAF

+			    && dict_index_is_online_ddl(index)) {

+				mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED;

+				mtr_s_lock(dict_index_get_lock(index), &mtr);

+			}

-		if (unsigned ai = index->table->persistent_autoinc) {

-			/* Prepare to persist the AUTO_INCREMENT value

-			from the index entry to PAGE_ROOT_AUTO_INC. */

-			const dfield_t* dfield = dtuple_get_nth_field(

-				entry, ai - 1);

-			auto_inc = dfield_is_null(dfield)

-				? 0

-				: row_parse_int(static_cast<const byte*>(

+			if (unsigned ai = index->table->persistent_autoinc) {

+				/* Prepare to persist the AUTO_INCREMENT value

+				from the index entry to PAGE_ROOT_AUTO_INC. */

+				const dfield_t* dfield = dtuple_get_nth_field(

+					entry, ai - 1);

+				if (!dfield_is_null(dfield)) {

+					auto_inc = row_parse_int(

+						static_cast<const byte*>(

 							dfield->data),

 						dfield->len,

 						dfield->type.mtype,

 						dfield->type.prtype

 						& DATA_UNSIGNED);

+				}

+			}

Apart from this piece of code, index->table->persistent_autoinc plays a role in ha_innobase::update_row(), and in ALTER TABLE operations. In those cases, the field would have been initialized by ha_innobase::open() invoking initialize_auto_increment(), or by create_table_info_t::create_table_update_dict().

Attachments

Issue Links

is caused by

MDEV-15562 Instant DROP COLUMN or changing the order of columns

Closed

relates to

MDEV-17721 Various assertion failures due to MDEV-15562 instant ALTER TABLE

Closed

MDEV-17901 Assertion failure in file storage/innobase/include/row0row.ic line 211 after dropping column

Closed

Activity

Ascending order - Click to sort in descending order

Marko Mäkelä created issue - 2018-11-16 10:41

Marko Mäkelä made changes - 2018-11-16 11:24

Field	Original Value	New Value
Link		This issue is caused by ~~MDEV-15562~~ [ ~~MDEV-15562~~ ]

Marko Mäkelä made changes - 2018-11-16 11:24

Description	This was first reported by [~elenst] in ~~MDEV-17721~~: {code:diff} diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test index fd7fbe94450..84de6bcc771 100644 --- a/mysql-test/suite/innodb/t/instant_alter.test +++ b/mysql-test/suite/innodb/t/instant_alter.test @@ -397,6 +397,13 @@ ALTER TABLE t1 ADD COLUMN t TEXT; SELECT * FROM t1; DROP TABLE t1; +eval CREATE TABLE t1 (a INT AUTO_INCREMENT, b INT, KEY(a)) $engine; +INSERT INTO t1 SET a=NULL; +ALTER TABLE t1 DROP COLUMN b; +SELECT * FROM t1; +SHOW CREATE TABLE t1; +DROP TABLE t1; + dec $format; } disconnect analyze; {code} With this change to the test, InnoDB would crash as follows: {noformat:title=10.4 89337d510e9780d6533c3db4d42969b2338a7610} CURRENT_TEST: innodb.instant_alter mysqltest: At line 402: query 'ALTER TABLE t1 DROP COLUMN b' failed: 2013: Lost connection to MySQL server during query … 2018-11-16 12:36:36 0x7ff5ac4c1700 InnoDB: Assertion failure in file /mariadb/10.4/storage/innobase/include/row0row.ic line 215 {noformat}	This was first reported by [~elenst] in ~~MDEV-17721~~: {code:diff} diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test index fd7fbe94450..4dd00a227f9 100644 --- a/mysql-test/suite/innodb/t/instant_alter.test +++ b/mysql-test/suite/innodb/t/instant_alter.test @@ -397,6 +397,14 @@ ALTER TABLE t1 ADD COLUMN t TEXT; SELECT * FROM t1; DROP TABLE t1; +eval CREATE TABLE t1 (a INT AUTO_INCREMENT, b INT, KEY(a)) $engine; +INSERT INTO t1 SET a=NULL; +ALTER TABLE t1 DROP COLUMN b; +INSERT INTO t1 SET a=NULL; +UPDATE t1 SET a=a+2; +SELECT * FROM t1; +DROP TABLE t1; + dec $format; } disconnect analyze; {code} With this change to the test, InnoDB would crash as follows: {noformat:title=10.4 89337d510e9780d6533c3db4d42969b2338a7610} CURRENT_TEST: innodb.instant_alter mysqltest: At line 402: query 'ALTER TABLE t1 DROP COLUMN b' failed: 2013: Lost connection to MySQL server during query … 2018-11-16 12:36:36 0x7ff5ac4c1700 InnoDB: Assertion failure in file /mariadb/10.4/storage/innobase/include/row0row.ic line 215 … #6 0x000056295ac4c0e5 in ut_dbg_assertion_failed (expr=0x0, file=0x56295b2b6780 "/mariadb/10.4/storage/innobase/include/row0row.ic", line=215) at /mariadb/10.4/storage/innobase/ut/ut0dbg.cc:61 #7 0x000056295ab702a2 in row_parse_int (data=0x7ff5541cfbf0 "", len=8, mtype=5, unsigned_type=false) at /mariadb/10.4/storage/innobase/include/row0row.ic:215 #8 0x000056295ab6c432 in row_ins_clust_index_entry_low (flags=2, mode=33, index=0x7ff554106a28, n_uniq=1, entry=0x7ff5541cfb18, n_ext=0, thr=0x7ff5541cfcb8, dup_chk_only=false) at /mariadb/10.4/storage/innobase/row/row0ins.cc:2613 #9 0x000056295aa6726c in innobase_instant_try (ha_alter_info=0x7ff5ac4bdb60, ctx=0x7ff5540153e8, altered_table=0x7ff5541cd528, table=0x7ff55419c618, trx=0x7ff5ac75b300) at /mariadb/10.4/storage/innobase/handler/handler0alter.cc:5522 {noformat} This should only occur if a table contains an {{AUTO_INCREMENT}} column that is not part of the {{PRIMARY KEY}}, and if the first instant {{ALTER TABLE}} is one of the ~~MDEV-15562~~ operations: permuting columns, adding a column somewhere else than the last column, or dropping a column. If there was a prior ~~MDEV-11369~~ instant {{ADD COLUMN}} operation on the table, then this code would not be executed. This code is part of the ~~MDEV-6076~~ logic, to ensure that the persistent {{AUTO_INCREMENT}} value of the table will be updated on every {{INSERT}}. At this point of the execution, {{index->table->persistent_autoinc}} is pointing to the wrong index field. So, we are trying to read an {{AUTO_INCREMENT}} value from the wrong position, from a hidden metadata record where the {{AUTO_INCREMENT}} value surely does not matter. The position is off by one, because in the ~~MDEV-15562~~ metadata record, there is a metadata BLOB after the {{index->first_user_record()}}. In 10.3, there is no issue, because the dummy value for the {{AUTO_INCREMENT}} column in the ~~MDEV-11369~~ metadata record would be 0 or {{NULL}}, and because the field will be (unnecessarily) accessed at the correct position. We can avoid the unnecessary and harmful access during {{ALTER TABLE}}: {code:diff} diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index 777d69cf127..1014168d389 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -2597,25 +2597,32 @@ row_ins_clust_index_entry_low( } else { index->set_modified(mtr); - if (mode == BTR_MODIFY_LEAF - && dict_index_is_online_ddl(index)) { - mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED; - mtr_s_lock(dict_index_get_lock(index), &mtr); - } + if (UNIV_UNLIKELY(entry->is_metadata())) { + ut_ad(index->is_instant()); + ut_ad(!dict_index_is_online_ddl(index)); + ut_ad(mode == BTR_MODIFY_TREE); + } else { + if (mode == BTR_MODIFY_LEAF + && dict_index_is_online_ddl(index)) { + mode = BTR_MODIFY_LEAF_ALREADY_S_LATCHED; + mtr_s_lock(dict_index_get_lock(index), &mtr); + } - if (unsigned ai = index->table->persistent_autoinc) { - /* Prepare to persist the AUTO_INCREMENT value - from the index entry to PAGE_ROOT_AUTO_INC. / - const dfield_t dfield = dtuple_get_nth_field( - entry, ai - 1); - auto_inc = dfield_is_null(dfield) - ? 0 - : row_parse_int(static_cast<const byte>( + if (unsigned ai = index->table->persistent_autoinc) { + / Prepare to persist the AUTO_INCREMENT value + from the index entry to PAGE_ROOT_AUTO_INC. / + const dfield_t dfield = dtuple_get_nth_field( + entry, ai - 1); + if (!dfield_is_null(dfield)) { + auto_inc = row_parse_int( + static_cast<const byte*>( dfield->data), dfield->len, dfield->type.mtype, dfield->type.prtype & DATA_UNSIGNED); + } + } } } {code} Apart from this piece of code, {{index->table->persistent_autoinc}} plays a role in {{ha_innobase::update_row()}}, and in {{ALTER TABLE}} operations. In those cases, the field would have been initialized by {{ha_innobase::open()}} invoking {{initialize_auto_increment()}}, or by {{create_table_info_t::create_table_update_dict()}}.
Labels		crash instant

Marko Mäkelä made changes - 2018-11-16 11:27

Summary

Assertion failure in row_parse_int() on first ADD/DROP COLUMN when an AUTO_INCREMENT column exists

Assertion failure in row_parse_int() on first ADD/DROP COLUMN when an AUTO_INCREMENT column is not in PRIMARY KEY

Marko Mäkelä made changes - 2018-11-16 11:35

issue.field.resolutiondate

2018-11-16 11:35:46.0

2018-11-16 11:35:46.734

Marko Mäkelä made changes - 2018-11-16 11:35

Resolution		Fixed [ 1 ]
Status	Open [ 1 ]	Closed [ 6 ]

Marko Mäkelä made changes - 2018-11-16 12:01

Link

This issue relates to ~~MDEV-17721~~ [ ~~MDEV-17721~~ ]

Elena Stepanova made changes - 2018-12-04 18:18

Link

This issue relates to ~~MDEV-17901~~ [ ~~MDEV-17901~~ ]

Sergei Golubchik made changes - 2021-12-06 21:48

Workflow

MariaDB v3 [ 90725 ]

MariaDB v4 [ 155216 ]

People

Assignee:: Marko Mäkelä

Reporter:: Marko Mäkelä

Votes:: 0 Vote for this issue

Watchers:: 1 Start watching this issue

Dates

Created:: 2018-11-16 10:41

Updated:: 2018-12-04 18:18

Resolved:: 2018-11-16 11:35

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB Server