Skip to content

Commit

Permalink
MDEV-28870 InnoDB: Missing FILE_CREATE, FILE_DELETE or FILE_MODIFY be…
Browse files Browse the repository at this point in the history 
…fore FILE_CHECKPOINT

There was a race condition between log_checkpoint_low() and
deleting or renaming data files. The scenario is as follows:

1. The buffer pool does not contain dirty pages.
2. A FILE_DELETE or FILE_RENAME record is written.
3. The checkpoint LSN will be moved ahead of the write of the record.
4. The server is killed before the file is actually renamed or deleted.

We will prevent this race condition by ensuring that a log checkpoint
cannot occur between the durable write and the file system operation:

1. Durably write the FILE_DELETE or FILE_RENAME record.
2. Perform the file system operation.
3. Allow any log checkpoint to proceed.

mtr_t::commit_file(): Implement the DELETE or RENAME logic.

fil_delete_tablespace(): Delegate some of the logic to
mtr_t::commit_file().

fil_space_t::rename(): Delegate some logic to mtr_t::commit_file().
Remove the debug injection point fil_rename_tablespace_failure_2
because we do test RENAME failures without any debug injection.

fil_name_write_rename_low(), fil_name_write_rename(): Remove.

Tested by Matthias Leich
  • Loading branch information
@dr-m
dr-m committed Jun 21, 2022
1 parent 55f02c2 commit 2e43af6
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 263 deletions.
27 changes: 0 additions & 27 deletions mysql-test/suite/innodb/r/innodb-wl5980-debug.result
View file

This file was deleted.

52 changes: 0 additions & 52 deletions mysql-test/suite/innodb/t/innodb-wl5980-debug.test
View file

This file was deleted.

223 changes: 39 additions & 184 deletions storage/innobase/fil/fil0fil.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,6 @@ bool fil_space_t::try_to_close(bool print_info)
return false;
}

/** Rename a single-table tablespace.
The tablespace must exist in the memory cache.
@param[in] id tablespace identifier
@param[in] old_path old file name
@param[in] new_path_in new file name,
or NULL if it is located in the normal data directory
@return true if success */
static bool
fil_rename_tablespace(
ulint id,
const char* old_path,
const char* new_path_in);

/*
IMPLEMENTATION OF THE TABLESPACE MEMORY CACHE
=============================================
Expand Down Expand Up @@ -1523,40 +1510,6 @@ inline void mtr_t::log_file_op(mfile_type_t type, ulint space_id,
m_log.push(reinterpret_cast<const byte*>(path), uint32_t(len));
}

/** Write redo log for renaming a file.
@param[in] space_id tablespace id
@param[in] old_name tablespace file name
@param[in] new_name tablespace file name after renaming
@param[in,out] mtr mini-transaction */
static
void
fil_name_write_rename_low(
ulint space_id,
const char* old_name,
const char* new_name,
mtr_t* mtr)
{
ut_ad(!is_predefined_tablespace(space_id));
mtr->log_file_op(FILE_RENAME, space_id, old_name, new_name);
}

/** Write redo log for renaming a file.
@param[in] space_id tablespace id
@param[in] old_name tablespace file name
@param[in] new_name tablespace file name after renaming */
static void
fil_name_write_rename(
ulint space_id,
const char* old_name,
const char* new_name)
{
mtr_t mtr;
mtr.start();
fil_name_write_rename_low(space_id, old_name, new_name, &mtr);
mtr.commit();
log_write_up_to(mtr.commit_lsn(), true);
}

/** Write FILE_MODIFY for a file.
@param[in] space_id tablespace id
@param[in] name tablespace file name
Expand Down Expand Up @@ -1688,41 +1641,8 @@ pfs_os_file_t fil_delete_tablespace(ulint id)
mtr_t mtr;
mtr.start();
mtr.log_file_op(FILE_DELETE, id, space->chain.start->name);
mtr.commit();
log_write_up_to(mtr.commit_lsn(), true);

/* Remove any additional files. */
if (char *cfg_name= fil_make_filepath(space->chain.start->name,
fil_space_t::name_type{}, CFG,
false))
{
os_file_delete_if_exists(innodb_data_file_key, cfg_name, nullptr);
ut_free(cfg_name);
}
if (FSP_FLAGS_HAS_DATA_DIR(space->flags))
RemoteDatafile::delete_link_file(space->name());

/* Remove the directory entry. The file will actually be deleted
when our caller closes the handle. */
os_file_delete(innodb_data_file_key, space->chain.start->name);

mysql_mutex_lock(&fil_system.mutex);
/* Sanity checks after reacquiring fil_system.mutex */
ut_ad(space == fil_space_get_by_id(id));
ut_ad(!space->referenced());
ut_ad(space->is_stopping());
ut_ad(UT_LIST_GET_LEN(space->chain) == 1);
/* Detach the file handle. */
handle= fil_system.detach(space, true);
mysql_mutex_unlock(&fil_system.mutex);

mysql_mutex_lock(&log_sys.mutex);
if (space->max_lsn)
{
ut_d(space->max_lsn = 0);
fil_system.named_spaces.remove(*space);
}
mysql_mutex_unlock(&log_sys.mutex);
handle= space->chain.start->handle;
mtr.commit_file(*space, nullptr);

fil_space_free_low(space);
}
Expand Down Expand Up @@ -1847,120 +1767,55 @@ char *fil_make_filepath(const char* path, const table_name_t name,
dberr_t fil_space_t::rename(const char *path, bool log, bool replace)
{
ut_ad(UT_LIST_GET_LEN(chain) == 1);
ut_ad(!is_system_tablespace(id));
ut_ad(!is_predefined_tablespace(id));

const char *old_path= chain.start->name;

ut_ad(strchr(old_path, '/'));
ut_ad(strchr(path, '/'));

if (!strcmp(path, old_path))
return DB_SUCCESS;

if (log)
if (!log)
{
bool exists= false;
os_file_type_t ftype;

if (os_file_status(old_path, &exists, &ftype) && !exists)
{
ib::error() << "Cannot rename '" << old_path << "' to '" << path
<< "' because the source file does not exist.";
return DB_TABLESPACE_NOT_FOUND;
}

exists= false;
if (replace);
else if (!os_file_status(path, &exists, &ftype) || exists)
{
ib::error() << "Cannot rename '" << old_path << "' to '" << path
<< "' because the target file exists.";
return DB_TABLESPACE_EXISTS;
}

fil_name_write_rename(id, old_path, path);
if (!os_file_rename(innodb_data_file_key, old_path, path))
return DB_ERROR;
mysql_mutex_lock(&fil_system.mutex);
ut_free(chain.start->name);
chain.start->name= mem_strdup(path);
mysql_mutex_unlock(&fil_system.mutex);
return DB_SUCCESS;
}

return fil_rename_tablespace(id, old_path, path) ? DB_SUCCESS : DB_ERROR;
}

/** Rename a single-table tablespace.
The tablespace must exist in the memory cache.
@param[in] id tablespace identifier
@param[in] old_path old file name
@param[in] new_path_in new file name,
or NULL if it is located in the normal data directory
@return true if success */
static bool
fil_rename_tablespace(
ulint id,
const char* old_path,
const char* new_path_in)
{
fil_space_t* space;
fil_node_t* node;
ut_a(id != 0);

mysql_mutex_lock(&fil_system.mutex);

space = fil_space_get_by_id(id);

if (space == NULL) {
ib::error() << "Cannot find space id " << id
<< " in the tablespace memory cache, though the file '"
<< old_path
<< "' in a rename operation should have that id.";
mysql_mutex_unlock(&fil_system.mutex);
return(false);
}

/* The following code must change when InnoDB supports
multiple datafiles per tablespace. */
ut_a(UT_LIST_GET_LEN(space->chain) == 1);
node = UT_LIST_GET_FIRST(space->chain);
space->reacquire();
bool exists= false;
os_file_type_t ftype;

mysql_mutex_unlock(&fil_system.mutex);

char* new_file_name = mem_strdup(new_path_in);
char* old_file_name = node->name;

ut_ad(strchr(old_file_name, '/'));
ut_ad(strchr(new_file_name, '/'));

if (!recv_recovery_is_on()) {
mysql_mutex_lock(&log_sys.mutex);
}

/* log_sys.mutex is above fil_system.mutex in the latching order */
mysql_mutex_assert_owner(&log_sys.mutex);
mysql_mutex_lock(&fil_system.mutex);
space->release();
ut_ad(node->name == old_file_name);
bool success;
DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
goto skip_second_rename; );
success = os_file_rename(innodb_data_file_key,
old_file_name,
new_file_name);
DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
skip_second_rename:
success = false; );

ut_ad(node->name == old_file_name);

if (success) {
node->name = new_file_name;
} else {
old_file_name = new_file_name;
}

if (!recv_recovery_is_on()) {
mysql_mutex_unlock(&log_sys.mutex);
}

mysql_mutex_unlock(&fil_system.mutex);
/* Check upfront if the rename operation might succeed, because we
must durably write redo log before actually attempting to execute
the rename in the file system. */
if (os_file_status(old_path, &exists, &ftype) && !exists)
{
sql_print_error("InnoDB: Cannot rename '%s' to '%s'"
" because the source file does not exist.",
old_path, path);
return DB_TABLESPACE_NOT_FOUND;
}

ut_free(old_file_name);
exists= false;
if (replace);
else if (!os_file_status(path, &exists, &ftype) || exists)
{
sql_print_error("InnoDB: Cannot rename '%s' to '%s'"
" because the target file exists.",
old_path, path);
return DB_TABLESPACE_EXISTS;
}

return(success);
mtr_t mtr;
mtr.start();
mtr.log_file_op(FILE_RENAME, id, old_path, path);
return mtr.commit_file(*this, path) ? DB_SUCCESS : DB_ERROR;
}

/** Create a tablespace file.
Expand Down
6 changes: 6 additions & 0 deletions storage/innobase/include/mtr0mtr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ struct mtr_t {
@param space tablespace that is being shrunk */
ATTRIBUTE_COLD void commit_shrink(fil_space_t &space);

/** Commit a mini-transaction that is deleting or renaming a file.
@param space tablespace that is being renamed or deleted
@param name new file name (nullptr=the file will be deleted)
@return whether the operation succeeded */
ATTRIBUTE_COLD bool commit_file(fil_space_t &space, const char *name);

/** Commit a mini-transaction that did not modify any pages,
but generated some redo log on a higher level, such as
FILE_MODIFY records and an optional FILE_CHECKPOINT marker.
Expand Down
Loading

0 comments on commit 2e43af6

Please sign in to comment.