Skip to content

Commit

Permalink
MDEV-14407 Assertion failure during rollback
Browse files Browse the repository at this point in the history
Rollback attempted to dereference DB_ROLL_PTR=0, which cannot possibly
be a valid undo log pointer. A safe canonical value would be
roll_ptr_t(1) << ROLL_PTR_INSERT_FLAG_POS
which is what was chosen in MDEV-12288.

This bug was reproduced in 10.3 only. Potentially, the problem could
have been introduced by MDEV-11415, which suppresses undo logging for
ALGORITHM=COPY operations. In those operations, we should actually
have written the safe value of DB_ROLL_PTR instead of writing 0.
However, the test in commit 5421e3a
demonstrates that access to the rebuilt table by earlier-started
transactions should actually have been refused with ER_TABLE_DEF_CHANGED.

btr_cur_ins_lock_and_undo(): When undo logging is disabled, use the
safe value of DB_ROLL_PTR.

btr_cur_optimistic_insert(): Validate the DB_TRX_ID,DB_ROLL_PTR before
inserting into a clustered index leaf page.

ins_node_t::sys_buf[]: Replaces row_id_buf and trx_id_buf and some
heap usage.

row_ins_alloc_sys_fields(): Initialize ins_node_t::sys_buf[].

trx_undo_page_report_modify(): Assert that the DB_ROLL_PTR is not 0.

trx_undo_get_undo_rec_low(): Assert that the roll_ptr is valid before
trying to dereference it.

dict_index_t::is_primary(): Check if the index is the primary key.
  • Loading branch information
dr-m committed Feb 8, 2018
1 parent be6307c commit db25305
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 27 deletions.
27 changes: 25 additions & 2 deletions storage/innobase/btr/btr0cur.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2811,7 +2811,7 @@ btr_cur_ins_lock_and_undo(
}

if (flags & BTR_NO_UNDO_LOG_FLAG) {
roll_ptr = 0;
roll_ptr = roll_ptr_t(1) << ROLL_PTR_INSERT_FLAG_POS;
} else {
err = trx_undo_report_row_operation(thr, index, entry,
NULL, 0, NULL, NULL,
Expand Down Expand Up @@ -3016,7 +3016,7 @@ btr_cur_optimistic_insert(

DBUG_LOG("ib_cur",
"insert " << index->name << " (" << index->id << ") by "
<< ib::hex(thr ? trx_get_id_for_print(thr_get_trx(thr)) : 0)
<< ib::hex(thr ? thr->graph->trx->id : 0)
<< ' ' << rec_printer(entry).str());
DBUG_EXECUTE_IF("do_page_reorganize",
btr_page_reorganize(page_cursor, index, mtr););
Expand All @@ -3033,6 +3033,29 @@ btr_cur_optimistic_insert(
goto fail_err;
}

#ifdef UNIV_DEBUG
if (!(flags & BTR_CREATE_FLAG)
&& index->is_primary() && page_is_leaf(page)) {
const dfield_t* trx_id = dtuple_get_nth_field(
entry, dict_col_get_clust_pos(
dict_table_get_sys_col(index->table,
DATA_TRX_ID),
index));

ut_ad(trx_id->len == DATA_TRX_ID_LEN);
ut_ad(trx_id[1].len == DATA_ROLL_PTR_LEN);
ut_ad(*static_cast<const byte*>
(trx_id[1].data) & 0x80);
if (!(flags & BTR_NO_UNDO_LOG_FLAG)) {
ut_ad(thr->graph->trx->id);
ut_ad(thr->graph->trx->id
== trx_read_trx_id(
static_cast<const byte*>(
trx_id->data)));
}
}
#endif

*rec = page_cur_tuple_insert(
page_cursor, entry, index, offsets, heap,
n_ext, mtr);
Expand Down
7 changes: 7 additions & 0 deletions storage/innobase/include/dict0mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,13 @@ struct dict_index_t{
and the .ibd file is missing, or a
page cannot be read or decrypted */
inline bool is_readable() const;

/** @return whether the index is the primary key index
(not the clustered index of the change buffer) */
bool is_primary() const
{
return DICT_CLUSTERED == (type & (DICT_CLUSTERED | DICT_IBUF));
}
};

/** The status of online index creation */
Expand Down
7 changes: 4 additions & 3 deletions storage/innobase/include/row0ins.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
Copyright (c) 2017, 2018, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -198,10 +198,11 @@ struct ins_node_t{
this should be reset to NULL */
UT_LIST_BASE_NODE_T(dtuple_t)
entry_list;/* list of entries, one for each index */
byte* row_id_buf;/* buffer for the row id sys field in row */
/** buffer for the system columns */
byte sys_buf[DATA_ROW_ID_LEN
+ DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN];
trx_id_t trx_id; /*!< trx id or the last trx which executed the
node */
byte* trx_id_buf;/* buffer for the trx id sys field in row */
mem_heap_t* entry_sys_heap;
/* memory heap used as auxiliary storage;
entry_list and sys fields are stored here;
Expand Down
36 changes: 15 additions & 21 deletions storage/innobase/row/row0ins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,49 +139,44 @@ row_ins_alloc_sys_fields(
{
dtuple_t* row;
dict_table_t* table;
mem_heap_t* heap;
const dict_col_t* col;
dfield_t* dfield;
byte* ptr;

row = node->row;
table = node->table;
heap = node->entry_sys_heap;

ut_ad(row && table && heap);
ut_ad(dtuple_get_n_fields(row) == dict_table_get_n_cols(table));

/* allocate buffer to hold the needed system created hidden columns. */
const uint len = DATA_ROW_ID_LEN + DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN;
ptr = static_cast<byte*>(mem_heap_zalloc(heap, len));
compile_time_assert(DATA_ROW_ID_LEN
+ DATA_TRX_ID_LEN + DATA_ROLL_PTR_LEN
== sizeof node->sys_buf);
memset(node->sys_buf, 0, sizeof node->sys_buf);
/* Assign DB_ROLL_PTR to 1 << ROLL_PTR_INSERT_FLAG_POS */
node->sys_buf[DATA_ROW_ID_LEN + DATA_TRX_ID_LEN] = 0x80;

/* 1. Populate row-id */
col = dict_table_get_sys_col(table, DATA_ROW_ID);

dfield = dtuple_get_nth_field(row, dict_col_get_no(col));

dfield_set_data(dfield, ptr, DATA_ROW_ID_LEN);

node->row_id_buf = ptr;

ptr += DATA_ROW_ID_LEN;
dfield_set_data(dfield, node->sys_buf, DATA_ROW_ID_LEN);

/* 2. Populate trx id */
col = dict_table_get_sys_col(table, DATA_TRX_ID);

dfield = dtuple_get_nth_field(row, dict_col_get_no(col));

dfield_set_data(dfield, ptr, DATA_TRX_ID_LEN);

node->trx_id_buf = ptr;

ptr += DATA_TRX_ID_LEN;
dfield_set_data(dfield, &node->sys_buf[DATA_ROW_ID_LEN],
DATA_TRX_ID_LEN);

col = dict_table_get_sys_col(table, DATA_ROLL_PTR);

dfield = dtuple_get_nth_field(row, dict_col_get_no(col));

dfield_set_data(dfield, ptr, DATA_ROLL_PTR_LEN);
dfield_set_data(dfield, &node->sys_buf[DATA_ROW_ID_LEN
+ DATA_TRX_ID_LEN],
DATA_ROLL_PTR_LEN);
}

/*********************************************************************//**
Expand Down Expand Up @@ -3474,7 +3469,7 @@ row_ins_alloc_row_id_step(

row_id = dict_sys_get_new_row_id();

dict_sys_write_row_id(node->row_id_buf, row_id);
dict_sys_write_row_id(node->sys_buf, row_id);
}

/***********************************************************//**
Expand Down Expand Up @@ -3767,10 +3762,9 @@ row_ins_step(
This happens, for example, when a row update moves it to another
partition. In that case, we have already set the IX lock on the
table during the search operation, and there is no need to set
it again here. But we must write trx->id to node->trx_id_buf. */
it again here. But we must write trx->id to node->sys_buf. */

memset(node->trx_id_buf, 0, DATA_TRX_ID_LEN);
trx_write_trx_id(node->trx_id_buf, trx->id);
trx_write_trx_id(&node->sys_buf[DATA_ROW_ID_LEN], trx->id);

if (node->state == INS_NODE_SET_IX_LOCK) {

Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/row/row0mysql.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ row_insert_for_mysql(

if (prebuilt->clust_index_was_generated) {
/* set row id to prebuilt */
ut_memcpy(prebuilt->row_id, node->row_id_buf, DATA_ROW_ID_LEN);
memcpy(prebuilt->row_id, node->sys_buf, DATA_ROW_ID_LEN);
}

dict_stats_update_if_needed(table);
Expand Down
3 changes: 3 additions & 0 deletions storage/innobase/trx/trx0rec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,7 @@ trx_undo_page_report_modify(
dict_index_get_sys_col_pos(
index, DATA_ROLL_PTR), &flen);
ut_ad(flen == DATA_ROLL_PTR_LEN);
ut_ad(memcmp(field, field_ref_zero, DATA_ROLL_PTR_LEN));

ptr += mach_u64_write_compressed(ptr, trx_read_roll_ptr(field));

Expand Down Expand Up @@ -2106,6 +2107,8 @@ trx_undo_get_undo_rec_low(

trx_undo_decode_roll_ptr(roll_ptr, &is_insert, &rseg_id, &page_no,
&offset);
ut_ad(page_no > FSP_FIRST_INODE_PAGE_NO);
ut_ad(offset >= TRX_UNDO_PAGE_HDR + TRX_UNDO_PAGE_HDR_SIZE);
rseg = is_temp
? trx_sys->temp_rsegs[rseg_id]
: trx_sys->rseg_array[rseg_id];
Expand Down

0 comments on commit db25305

Please sign in to comment.