Skip to content

Commit

Permalink
MDEV-31080 fil_validate() failures during deferred tablespace recovery
Browse files Browse the repository at this point in the history 
fil_space_t::create(), fil_space_t::add(): Expect the caller to
acquire and release fil_system.mutex. In this way, creating a tablespace
and adding the first (usually only) data file will be atomic.

recv_sys_t::recover_deferred(): Correctly protect some changes by
holding fil_system.mutex.

Tested by: Matthias Leich
  • Loading branch information
@dr-m
dr-m committed Apr 19, 2023
1 parent 78368e5 commit 0cda0e4
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 42 deletions.
17 changes: 10 additions & 7 deletions extra/mariabackup/xtrabackup.cc
View file
Expand Up @@ -3454,20 +3454,20 @@ static void xb_load_single_table_tablespace(const char *dirname,
bool is_empty_file = file->exists() && file->is_empty_file();

if (err == DB_SUCCESS && file->space_id() != SRV_TMP_SPACE_ID) {
mysql_mutex_lock(&fil_system.mutex);
space = fil_space_t::create(
file->space_id(), file->flags(),
FIL_TYPE_TABLESPACE, nullptr/* TODO: crypt_data */,
FIL_ENCRYPTION_DEFAULT,
file->handle() != OS_FILE_CLOSED);

ut_a(space != NULL);
ut_ad(space);
fil_node_t* node= space->add(
file->filepath(),
skip_node_page0 ? file->detach() : pfs_os_file_t(),
0, false, false);
node->deferred= defer;
mysql_mutex_lock(&fil_system.mutex);
space->read_page0();
if (!space->read_page0())
err = DB_CANNOT_OPEN_FILE;
mysql_mutex_unlock(&fil_system.mutex);

if (srv_operation == SRV_OPERATION_RESTORE_DELTA
Expand Down Expand Up @@ -5324,9 +5324,12 @@ xb_delta_open_matching_space(
ut_ad(fil_space_t::zip_size(flags) == info.zip_size);
ut_ad(fil_space_t::physical_size(flags) == info.page_size);

if (fil_space_t::create(info.space_id, flags,
FIL_TYPE_TABLESPACE, 0, FIL_ENCRYPTION_DEFAULT,
true)) {
mysql_mutex_lock(&fil_system.mutex);
fil_space_t* space = fil_space_t::create(info.space_id, flags,
FIL_TYPE_TABLESPACE, 0,
FIL_ENCRYPTION_DEFAULT, true);
mysql_mutex_unlock(&fil_system.mutex);
if (space) {
*success = xb_space_create_file(real_name, info.space_id,
flags, &file);
} else {
Expand Down
29 changes: 19 additions & 10 deletions storage/innobase/fil/fil0fil.cc
View file
Expand Up @@ -311,6 +311,8 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,
uint32_t size, bool is_raw, bool atomic_write,
uint32_t max_pages)
{
mysql_mutex_assert_owner(&fil_system.mutex);

fil_node_t* node;

ut_ad(name != NULL);
Expand All @@ -335,7 +337,6 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,

node->atomic_write = atomic_write;

mysql_mutex_lock(&fil_system.mutex);
this->size += size;
UT_LIST_ADD_LAST(chain, node);
if (node->is_open()) {
Expand All @@ -346,7 +347,6 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,
release();
}
}
mysql_mutex_unlock(&fil_system.mutex);

return node;
}
Expand Down Expand Up @@ -946,6 +946,7 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
{
fil_space_t* space;

mysql_mutex_assert_owner(&fil_system.mutex);
ut_ad(fil_system.is_initialised());
ut_ad(fil_space_t::is_valid_flags(flags & ~FSP_FLAGS_MEM_MASK, id));
ut_ad(srv_page_size == UNIV_PAGE_SIZE_ORIG || flags != 0);
Expand Down Expand Up @@ -978,16 +979,13 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,

space->latch.SRW_LOCK_INIT(fil_space_latch_key);

mysql_mutex_lock(&fil_system.mutex);

if (const fil_space_t *old_space = fil_space_get_by_id(id)) {
ib::error() << "Trying to add tablespace with id " << id
<< " to the cache, but tablespace '"
<< (old_space->chain.start
? old_space->chain.start->name
: "")
<< "' already exists in the cache!";
mysql_mutex_unlock(&fil_system.mutex);
space->~fil_space_t();
ut_free(space);
return(NULL);
Expand Down Expand Up @@ -1034,12 +1032,12 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
if (rotate) {
fil_system.default_encrypt_tables.push_back(*space);
space->is_in_default_encrypt = true;
}

mysql_mutex_unlock(&fil_system.mutex);

if (rotate && srv_n_fil_crypt_threads_started) {
fil_crypt_threads_signal();
if (srv_n_fil_crypt_threads_started) {
mysql_mutex_unlock(&fil_system.mutex);
fil_crypt_threads_signal();
mysql_mutex_lock(&fil_system.mutex);
}
}

return(space);
Expand Down Expand Up @@ -1998,16 +1996,20 @@ fil_ibd_create(
DBUG_EXECUTE_IF("checkpoint_after_file_create",
log_make_checkpoint(););

mysql_mutex_lock(&fil_system.mutex);
if (fil_space_t* space = fil_space_t::create(space_id, flags,
FIL_TYPE_TABLESPACE,
crypt_data, mode, true)) {
fil_node_t* node = space->add(path, file, size, false, true);
mysql_mutex_unlock(&fil_system.mutex);
IF_WIN(node->find_metadata(), node->find_metadata(file, true));
mtr.start();
mtr.set_named_space(space);
ut_a(fsp_header_init(space, size, &mtr) == DB_SUCCESS);
mtr.commit();
return space;
} else {
mysql_mutex_unlock(&fil_system.mutex);
}

if (space_name.data()) {
Expand Down Expand Up @@ -2267,8 +2269,10 @@ fil_ibd_open(
first_page)
: NULL;

mysql_mutex_lock(&fil_system.mutex);
space = fil_space_t::create(id, flags, purpose, crypt_data);
if (!space) {
mysql_mutex_unlock(&fil_system.mutex);
goto error;
}

Expand All @@ -2278,6 +2282,7 @@ fil_ibd_open(
space->add(
df_remote.is_open() ? df_remote.filepath() :
df_default.filepath(), OS_FILE_CLOSED, 0, false, true);
mysql_mutex_unlock(&fil_system.mutex);

if (must_validate && !srv_read_only_mode) {
df_remote.close();
Expand Down Expand Up @@ -2566,10 +2571,13 @@ fil_ibd_load(
return FIL_LOAD_INVALID;
}

mysql_mutex_lock(&fil_system.mutex);

space = fil_space_t::create(
space_id, flags, FIL_TYPE_TABLESPACE, crypt_data);

if (space == NULL) {
mysql_mutex_unlock(&fil_system.mutex);
return(FIL_LOAD_INVALID);
}

Expand All @@ -2581,6 +2589,7 @@ fil_ibd_load(
let fil_node_open() do that task. */

space->add(file.filepath(), OS_FILE_CLOSED, 0, false, false);
mysql_mutex_unlock(&fil_system.mutex);

return(FIL_LOAD_OK);
}
Expand Down
24 changes: 13 additions & 11 deletions storage/innobase/fsp/fsp0space.cc
View file
Expand Up @@ -88,25 +88,25 @@ Tablespace::open_or_create(bool is_temp)
ut_ad(!m_files.empty());

for (iterator it = begin(); it != end(); ++it) {

if (it->m_exists) {
err = it->open_or_create(
m_ignore_read_only
? false : srv_read_only_mode);
if (err != DB_SUCCESS) {
return err;
}
} else {
err = it->open_or_create(
m_ignore_read_only
? false : srv_read_only_mode);

/* Set the correct open flags now that we have
successfully created the file. */
if (err == DB_SUCCESS) {
file_found(*it);
if (err != DB_SUCCESS) {
return err;
}
}

if (err != DB_SUCCESS) {
break;
/* Set the correct open flags now that we have
successfully created the file. */
file_found(*it);
}

/* We can close the handle now and open the tablespace
Expand All @@ -130,20 +130,22 @@ Tablespace::open_or_create(bool is_temp)
fsp_flags = FSP_FLAGS_PAGE_SSIZE();
}

mysql_mutex_lock(&fil_system.mutex);
space = fil_space_t::create(
m_space_id, fsp_flags,
is_temp
? FIL_TYPE_TEMPORARY : FIL_TYPE_TABLESPACE,
NULL);
if (!space) {
mysql_mutex_unlock(&fil_system.mutex);
return DB_ERROR;
}
} else {
mysql_mutex_lock(&fil_system.mutex);
}

ut_a(fil_validate());

space->add(it->m_filepath, OS_FILE_CLOSED, it->m_size,
false, true);
mysql_mutex_unlock(&fil_system.mutex);
}

return(err);
Expand Down
10 changes: 6 additions & 4 deletions storage/innobase/fsp/fsp0sysspace.cc
View file
Expand Up @@ -921,6 +921,7 @@ SysTablespace::open_or_create(
/* Close the curent handles, add space and file info to the
fil_system cache and the Data Dictionary, and re-open them
in file_system cache so that they stay open until shutdown. */
mysql_mutex_lock(&fil_system.mutex);
ulint node_counter = 0;
for (files_t::iterator it = begin; it != end; ++it) {
it->close();
Expand All @@ -934,7 +935,8 @@ SysTablespace::open_or_create(
FIL_TYPE_TEMPORARY, NULL);
ut_ad(space == fil_system.temp_space);
if (!space) {
return DB_ERROR;
err = DB_ERROR;
break;
}
ut_ad(!space->is_compressed());
ut_ad(space->full_crc32());
Expand All @@ -945,12 +947,11 @@ SysTablespace::open_or_create(
FIL_TYPE_TABLESPACE, NULL);
ut_ad(space == fil_system.sys_space);
if (!space) {
return DB_ERROR;
err = DB_ERROR;
break;
}
}

ut_a(fil_validate());

uint32_t max_size = (++node_counter == m_files.size()
? (m_last_file_size_max == 0
? UINT32_MAX
Expand All @@ -961,6 +962,7 @@ SysTablespace::open_or_create(
it->m_type != SRV_NOT_RAW, true, max_size);
}

mysql_mutex_unlock(&fil_system.mutex);
return(err);
}

Expand Down
18 changes: 12 additions & 6 deletions storage/innobase/log/log0recv.cc
View file
Expand Up @@ -785,9 +785,10 @@ static struct
if (!os_file_status(name->c_str(), &exists, &ftype) || !exists)
goto processed;
}
create(it, *name, static_cast<uint32_t>
(1U << FSP_FLAGS_FCRC32_POS_MARKER |
FSP_FLAGS_FCRC32_PAGE_SSIZE()), nullptr, 0);
if (create(it, *name, static_cast<uint32_t>
(1U << FSP_FLAGS_FCRC32_POS_MARKER |
FSP_FLAGS_FCRC32_PAGE_SSIZE()), nullptr, 0))
mysql_mutex_unlock(&fil_system.mutex);
}
}
else
Expand Down Expand Up @@ -816,7 +817,7 @@ static struct
@param flags FSP_SPACE_FLAGS
@param crypt_data encryption metadata
@param size tablespace size in pages
@return tablespace
@return tablespace; the caller must release fil_system.mutex
@retval nullptr if crypt_data is invalid */
static fil_space_t *create(const recv_spaces_t::const_iterator &it,
const std::string &name, uint32_t flags,
Expand All @@ -828,6 +829,7 @@ static struct
ut_free(crypt_data);
return nullptr;
}
mysql_mutex_lock(&fil_system.mutex);
fil_space_t *space= fil_space_t::create(it->first, flags,
FIL_TYPE_TABLESPACE, crypt_data);
ut_ad(space);
Expand Down Expand Up @@ -900,12 +902,13 @@ static struct
space->free_limit= fsp_header_get_field(page, FSP_FREE_LIMIT);
space->free_len= flst_get_len(FSP_HEADER_OFFSET + FSP_FREE + page);
fil_node_t *node= UT_LIST_GET_FIRST(space->chain);
mysql_mutex_unlock(&fil_system.mutex);
if (!space->acquire())
{
{
free_space:
fil_space_free(it->first, false);
goto next_item;
}
}
if (os_file_write(IORequestWrite, node->name, node->handle,
page, 0, fil_space_t::physical_size(flags)) !=
DB_SUCCESS)
Expand Down Expand Up @@ -975,6 +978,7 @@ bool recv_sys_t::recover_deferred(recv_sys_t::map::iterator &p,
space->free_len= flst_get_len(FSP_HEADER_OFFSET + FSP_FREE + page);
fil_node_t *node= UT_LIST_GET_FIRST(space->chain);
node->deferred= true;
mysql_mutex_unlock(&fil_system.mutex);
if (!space->acquire())
goto release_and_fail;
fil_names_dirty(space);
Expand All @@ -998,8 +1002,10 @@ bool recv_sys_t::recover_deferred(recv_sys_t::map::iterator &p,
uint32_t(file_size / fil_space_t::physical_size(flags));
if (n_pages > size)
{
mysql_mutex_lock(&fil_system.mutex);
space->size= node->size= n_pages;
space->set_committed_size();
mysql_mutex_unlock(&fil_system.mutex);
goto size_set;
}
}
Expand Down
6 changes: 2 additions & 4 deletions storage/innobase/srv/srv0start.cc
View file
Expand Up @@ -559,14 +559,12 @@ static ulint srv_undo_tablespace_open(bool create, const char* name, ulint i)

fil_set_max_space_id_if_bigger(space_id);

mysql_mutex_lock(&fil_system.mutex);
fil_space_t *space= fil_space_t::create(space_id, fsp_flags,
FIL_TYPE_TABLESPACE, nullptr,
FIL_ENCRYPTION_DEFAULT, true);
ut_a(fil_validate());
ut_a(space);

ut_ad(space);
fil_node_t *file= space->add(name, fh, 0, false, true);
mysql_mutex_lock(&fil_system.mutex);

if (create)
{
Expand Down

0 comments on commit 0cda0e4

Please sign in to comment.