MDEV-38993 Assertion `trx->undo_no == 1' fails upon ALTER IGNORE#4746
MDEV-38993 Assertion `trx->undo_no == 1' fails upon ALTER IGNORE#4746Thirunarayanan wants to merge 1 commit into10.11from
Conversation
|
|
7a123b5 to
ac0dbc7
Compare
Problem: ======== During ALTER TABLE ... IGNORE, a partial rollback on duplicate key error resets trx->undo_no to 0. The subsequent insert then enters the undo rewrite block with undo_no == 0, hitting the assertion that expected undo_no == 1. Solution: ========= Partial rollback which truncates the last insert undo record via trx_undo_truncate_end(), which rewrites TRX_UNDO_PAGE_FREE on the page. By checking trx->undo_no as part of the rewrite predicate, InnoDB correctly skips the rewrite logic after partial rollback. trx_undo_report_row_operation(): Pre-compute the full predicate (clear_ignore) before trx_undo_assign_low(), since old_offset and top_offset are not modified by that call. trx_undo_rewrite_ignore(): Extract the rewrite body into a separate ATTRIBUTE_COLD ATTRIBUTE_NOINLINE static function.
ac0dbc7 to
6aee1fc
Compare
dr-m
left a comment
There was a problem hiding this comment.
Please check the generated AMD64 code for CMAKE_BUILD_TYPE=RelWithDebInfo.
| const bool clear_ignore{*pundo && trx->undo_no && | ||
| (*pundo)->old_offset <= | ||
| (*pundo)->top_offset && | ||
| index->table->skip_alter_undo == | ||
| dict_table_t::IGNORE_UNDO}; |
There was a problem hiding this comment.
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;| 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); | ||
| } |
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.