From eb7e24932b7cb190a4b96e5d6d9f9262c68814b3 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Mon, 13 Jun 2022 21:32:06 +0530 Subject: [PATCH] MDEV-28806 Assertion `flag == 1' failure in row_build_index_entry_low upon concurrent ALTER and UPDATE - During online ADD INDEX, InnoDB was incorrectly writing log for an UPDATE that does not affect the being-created index. --- .../suite/gcol/r/innodb_virtual_debug.result | 21 +++++++++++++++ .../suite/gcol/t/innodb_virtual_debug.test | 26 +++++++++++++++++++ storage/innobase/row/row0log.cc | 6 +++++ storage/innobase/row/row0upd.cc | 11 ++++---- 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_debug.result b/mysql-test/suite/gcol/r/innodb_virtual_debug.result index 80b2bde6ca5..9cac55c25e3 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_debug.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_debug.result @@ -124,3 +124,24 @@ CHECK TABLE t1; Table Op Msg_type Msg_text test.t1 check status OK DROP TABLE t1; +# +# MDEV-28806 Assertion `flag == 1' failure in +# row_build_index_entry_low upon concurrent ALTER and UPDATE +# +CREATE TABLE t1(a CHAR(8), b INT, c INT AS (b), KEY(a)) ENGINE=InnoDB; +INSERT INTO t1(b) VALUES (1),(2); +connect con1,localhost,root,,test; +SET DEBUG_SYNC="alter_table_inplace_before_lock_upgrade SIGNAL dml_start WAIT_FOR dml_commit"; +ALTER TABLE t1 ADD KEY ind (c); +connection default; +SET DEBUG_SYNC="now WAIT_FOR dml_start"; +UPDATE t1 SET a ='foo'; +SET DEBUG_SYNC="now SIGNAL dml_commit"; +connection con1; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +DROP TABLE t1; +disconnect con1; +connection default; +SET DEBUG_SYNC=RESET; diff --git a/mysql-test/suite/gcol/t/innodb_virtual_debug.test b/mysql-test/suite/gcol/t/innodb_virtual_debug.test index 5ebc90dac19..cd2b860400c 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_debug.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_debug.test @@ -312,4 +312,30 @@ SELECT * FROM t1; CHECK TABLE t1; DROP TABLE t1; +--echo # +--echo # MDEV-28806 Assertion `flag == 1' failure in +--echo # row_build_index_entry_low upon concurrent ALTER and UPDATE +--echo # + +CREATE TABLE t1(a CHAR(8), b INT, c INT AS (b), KEY(a)) ENGINE=InnoDB; +INSERT INTO t1(b) VALUES (1),(2); + +--connect (con1,localhost,root,,test) +SET DEBUG_SYNC="alter_table_inplace_before_lock_upgrade SIGNAL dml_start WAIT_FOR dml_commit"; +send ALTER TABLE t1 ADD KEY ind (c); + +--connection default +SET DEBUG_SYNC="now WAIT_FOR dml_start"; +UPDATE t1 SET a ='foo'; +SET DEBUG_SYNC="now SIGNAL dml_commit"; + +# Cleanup +--connection con1 +--reap +CHECK TABLE t1; +DROP TABLE t1; +--disconnect con1 +connection default; +SET DEBUG_SYNC=RESET; + --source include/wait_until_count_sessions.inc diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/row0log.cc index f2e2511a116..28f0eb42825 100644 --- a/storage/innobase/row/row0log.cc +++ b/storage/innobase/row/row0log.cc @@ -4061,6 +4061,11 @@ void UndorecApplier::log_update(const dtuple_t &tuple, { if (is_update) { + /* Ignore the index if the update doesn't affect the index */ + if (!row_upd_changes_ord_field_binary(index, update, + nullptr, + row, new_ext)) + goto next_index; dtuple_t *old_entry= row_build_index_entry_low( old_row, old_ext, index, heap, ROW_BUILD_NORMAL); @@ -4080,6 +4085,7 @@ void UndorecApplier::log_update(const dtuple_t &tuple, success= row_log_online_op(index, old_entry, 0); } } +next_index: index->lock.s_unlock(); if (!success) { diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index b53416e2976..a2eacaf8e12 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -1261,9 +1261,6 @@ row_upd_changes_ord_field_binary_func( ulint i; const dict_index_t* clust_index; - ut_ad(thr); - ut_ad(thr->graph); - ut_ad(thr->graph->trx); ut_ad(!index->table->skip_alter_undo); n_unique = dict_index_get_n_unique(index); @@ -1463,9 +1460,11 @@ row_upd_changes_ord_field_binary_func( trx_rollback_recovered() when the server had crashed before storing the field. */ - ut_ad(thr->graph->trx->is_recovered); - ut_ad(thr->graph->trx - == trx_roll_crash_recv_trx); + ut_ad(!thr + || thr->graph->trx->is_recovered); + ut_ad(!thr + || thr->graph->trx + == trx_roll_crash_recv_trx); return(TRUE); }