Skip to content

Commit

Permalink
MDEV-25506 (2 of 3): Kill during DDL leaves orphan .ibd file
Browse files Browse the repository at this point in the history
dict_drop_index_tree(): Even if SYS_INDEXES.PAGE contains the
special value FIL_NULL, the tablespace identified by SYS_INDEXES.SPACE
may exist and may need to be dropped. This would definitely be the case
if the server had been killed right after a FILE_CREATE record was
persistently written during CREATE TABLE, but before the transaction
was committed.

btr_free_if_exists(): Simplify the interface, to avoid repeated
tablespace lookup.

One more scenario is known to be broken: If the server is killed
during DROP TABLE (or table-rebuilding ALTER TABLE) right after a
FILE_DELETE record has been persistently written but before the
file was deleted, then we could end up recovering no tablespace
at all, and failing to delete the file, in either of fil_name_process()
or dict_drop_index_tree().

Thanks to Elena Stepanova for providing "rr replay" and data directories
of these scenarios.
  • Loading branch information
dr-m committed May 6, 2021
1 parent cc2ddde commit 2ceadb3
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 33 deletions.
21 changes: 9 additions & 12 deletions storage/innobase/btr/btr0btr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1225,23 +1225,20 @@ void dict_index_t::clear(que_thr_t *thr)
}

/** Free a persistent index tree if it exists.
@param[in] page_id root page id
@param[in] zip_size ROW_FORMAT=COMPRESSED page size, or 0
@param[in,out] space tablespce
@param[in] page root page number
@param[in] index_id PAGE_INDEX_ID contents
@param[in,out] mtr mini-transaction */
void btr_free_if_exists(const page_id_t page_id, ulint zip_size,
void btr_free_if_exists(fil_space_t *space, uint32_t page,
index_id_t index_id, mtr_t *mtr)
{
if (fil_space_t *space= fil_space_t::get(page_id.space()))
if (buf_block_t *root= btr_free_root_check(page_id_t(space->id, page),
space->zip_size(),
index_id, mtr))
{
if (buf_block_t *root= btr_free_root_check(page_id, zip_size, index_id,
mtr))
{
btr_free_but_not_root(root, mtr->get_log_mode());
mtr->set_named_space(space);
btr_free_root(root, mtr);
}
space->release();
btr_free_but_not_root(root, mtr->get_log_mode());
mtr->set_named_space(space);
btr_free_root(root, mtr);
}
}

Expand Down
22 changes: 10 additions & 12 deletions storage/innobase/dict/dict0crea.cc
Original file line number Diff line number Diff line change
Expand Up @@ -871,17 +871,8 @@ void dict_drop_index_tree(btr_pcur_t *pcur, trx_t *trx, dict_table_t *table,
if (len != 4)
goto rec_corrupted;

if (root_page_no == FIL_NULL)
/* The tree has already been freed */
return;

static_assert(FIL_NULL == 0xffffffff, "compatibility");
static_assert(DICT_FLD__SYS_INDEXES__PAGE_NO ==
DICT_FLD__SYS_INDEXES__SPACE + 1, "compatibility");
mtr->memset(btr_pcur_get_block(pcur), page_offset(p + 4), 4, 0xff);

const uint32_t space_id= mach_read_from_4(p);
ut_ad(space_id < SRV_TMP_SPACE_ID);
ut_ad(root_page_no == FIL_NULL || space_id <= SRV_SPACE_ID_UPPER_BOUND);

if (space_id && (type & DICT_CLUSTERED))
{
Expand All @@ -891,13 +882,20 @@ void dict_drop_index_tree(btr_pcur_t *pcur, trx_t *trx, dict_table_t *table,
ut_ad(!table);
fil_delete_tablespace(space_id, true);
}
else if (root_page_no == FIL_NULL)
/* The tree has already been freed */;
else if (fil_space_t*s= fil_space_t::get(space_id))
{
/* Ensure that the tablespace file exists
in order to avoid a crash in buf_page_get_gen(). */
if (root_page_no < s->get_size())
btr_free_if_exists(page_id_t(space_id, root_page_no), s->zip_size(),
mach_read_from_8(rec + 8), mtr);
{
static_assert(FIL_NULL == 0xffffffff, "compatibility");
static_assert(DICT_FLD__SYS_INDEXES__PAGE_NO ==
DICT_FLD__SYS_INDEXES__SPACE + 1, "compatibility");
mtr->memset(btr_pcur_get_block(pcur), page_offset(p + 4), 4, 0xff);
btr_free_if_exists(s, root_page_no, mach_read_from_8(rec + 8), mtr);
}
s->release();
}
}
Expand Down
14 changes: 5 additions & 9 deletions storage/innobase/include/btr0btr.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Copyright (c) 1994, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2012, Facebook Inc.
Copyright (c) 2014, 2020, MariaDB Corporation.
Copyright (c) 2014, 2021, 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 @@ -330,16 +330,12 @@ btr_create(
mtr_t* mtr);

/** Free a persistent index tree if it exists.
@param[in] page_id root page id
@param[in] zip_size ROW_FORMAT=COMPRESSED page size, or 0
@param[in,out] space tablespce
@param[in] page root page number
@param[in] index_id PAGE_INDEX_ID contents
@param[in,out] mtr mini-transaction */
void
btr_free_if_exists(
const page_id_t page_id,
ulint zip_size,
index_id_t index_id,
mtr_t* mtr);
void btr_free_if_exists(fil_space_t *space, uint32_t page,
index_id_t index_id, mtr_t *mtr);

/** Free an index tree in a temporary tablespace.
@param[in] page_id root page id */
Expand Down

0 comments on commit 2ceadb3

Please sign in to comment.