Skip to content
/ server Public
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions mysql-test/suite/innodb/r/alter_crash.result
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,10 @@ CHECK TABLE t1 EXTENDED;
Table Op Msg_type Msg_text
test.t1 check status OK
DROP TABLE t1;
#
# MDEV-38993 Assertion `trx->undo_no == 1' fails upon ALTER IGNORE
#
CREATE TABLE t (a INT) ENGINE=InnoDB;
INSERT INTO t VALUES (1),(2),(1),(2);
ALTER IGNORE TABLE t ADD UNIQUE(a);
DROP TABLE t;
8 changes: 8 additions & 0 deletions mysql-test/suite/innodb/t/alter_crash.test
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,11 @@ INSERT INTO t1 VALUES(1), (1);
ALTER IGNORE TABLE t1 ADD UNIQUE(f1);
CHECK TABLE t1 EXTENDED;
DROP TABLE t1;

--echo #
--echo # MDEV-38993 Assertion `trx->undo_no == 1' fails upon ALTER IGNORE
--echo #
CREATE TABLE t (a INT) ENGINE=InnoDB;
INSERT INTO t VALUES (1),(2),(1),(2);
ALTER IGNORE TABLE t ADD UNIQUE(a);
DROP TABLE t;
54 changes: 32 additions & 22 deletions storage/innobase/trx/trx0rec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1807,6 +1807,30 @@ static bool trx_has_lock_x(const trx_t &trx, dict_table_t& table)
return false;
}

/** For ALTER TABLE...IGNORE ALGORITHM=COPY, rewind the undo log
to maintain only the latest insert undo record. This allows easy
rollback of the last inserted row on duplicate key errors.
@param table table being altered
@param trx transaction
@param undo insert undo log
@param undo_block undo log page
@param mtr mini-transaction
@param m mod_tables entry to reinitialize */
static ATTRIBUTE_COLD ATTRIBUTE_NOINLINE
void trx_undo_rewrite_ignore(dict_table_t *table, trx_t *trx, trx_undo_t *undo,
buf_block_t *undo_block, mtr_t *mtr,
std::pair<trx_mod_tables_t::iterator, bool> &m)
{
ut_ad(trx->undo_no == 1);
undo->top_offset= undo->old_offset;
undo->top_undo_no= 0;
trx->undo_no= 0;
trx->mod_tables.clear();
m= trx->mod_tables.emplace(table, 0);
mtr->write<2>(*undo_block, undo_block->page.frame + TRX_UNDO_PAGE_HDR +
TRX_UNDO_PAGE_FREE, undo->old_offset);
}
Comment on lines +1819 to +1832
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameters will not fit in the 6 registers that are available in the commonly used AMD64 calling conventions. I assume that this is the limiting factor on our common target platforms. On ARMv8 there seem to be 8 registers for passing parameters.

We can freely choose the order of the function body. I would make mtr, undo_block the first parameters so that no registers have to be shuffled for the first call, like this:

static ATTRIBUTE_COLD ATTRIBUTE_NOINLINE
std::pair<trx_mod_tables_t::iterator, bool>
trx_undo_rewrite_ignore(mtr_t *mtr, buf_block_t *undo_block,
                        dict_table_t *table, trx_t *trx, trx_undo_t *undo)
  noexcept
{
  mtr->write<2>(*undo_block, undo_block->page.frame + TRX_UNDO_PAGE_HDR +
                TRX_UNDO_PAGE_FREE, undo->old_offset);
  ut_ad(trx->undo_no == 1);
  undo->top_offset= undo->old_offset;
  undo->top_undo_no= 0;
  trx->undo_no= 0;
  trx->mod_tables.clear();
  return trx->mod_tables.emplace(table, 0);
}

This will also enable a nice tail call optimization for the return value, which the caller would assign to its local variable m.


/***********************************************************************//**
Writes information to an undo log about an insert, update, or a delete marking
of a clustered index record. This information is used in a rollback of the
Expand Down Expand Up @@ -1912,32 +1936,18 @@ trx_undo_report_row_operation(
ut_ad(!trx->read_only);
ut_ad(trx->id);
pundo = &trx->rsegs.m_redo.undo;
const bool empty{!*pundo};
const bool clear_ignore{*pundo && trx->undo_no &&
(*pundo)->old_offset <=
(*pundo)->top_offset &&
index->table->skip_alter_undo ==
dict_table_t::IGNORE_UNDO};
Comment on lines +1939 to +1943
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation is a bit weird here. I would format it as follows to conform to the original InnoDB formatting style that is being used in this function:

		const bool clear_ignore = *pundo && trx->undo_no
			&& (*pundo)->old_offset <= (*pundo)->top_offset
			&& index->table->skip_alter_undo
			== dict_table_t::IGNORE_UNDO;

rseg = trx->rsegs.m_redo.rseg;
undo_block = trx_undo_assign_low<false>(trx, rseg, pundo,
&mtr, &err);
/* For ALTER IGNORE, implement undo log rewriting
to maintain only the latest insert undo record.
This allows easy rollback of the last inserted row
on duplicate key errors. Before writing a new undo
record, rewind the undo log to the previous record
position, effectively discarding all intermediate
undo records and keeping only the most recent one. */
if (!empty && (*pundo)->old_offset <= (*pundo)->top_offset &&
index->table->skip_alter_undo ==
dict_table_t::IGNORE_UNDO) {

if (clear_ignore) {
ut_ad(!rec);
ut_ad(trx->undo_no == 1);
(*pundo)->top_offset = (*pundo)->old_offset;
(*pundo)->top_undo_no = 0;
trx->undo_no = 0;
trx->mod_tables.clear();
m = trx->mod_tables.emplace(index->table, 0);
mtr.write<2>(
*undo_block,
undo_block->page.frame + TRX_UNDO_PAGE_HDR +
TRX_UNDO_PAGE_FREE, (*pundo)->old_offset);
trx_undo_rewrite_ignore(index->table, trx, *pundo,
undo_block, &mtr, m);
}
}

Expand Down