Skip to content

Commit

Permalink
MDEV-25051 Race condition between persistent statistics and RENAME TA…
Browse files Browse the repository at this point in the history
…BLE or TRUNCATE

innobase_rename_table(): Invoke dict_stats_wait_bg_to_stop_using_table()
to ensure that dict_stats_update() cannot be accessing the table name
that we will be modifying. If we are executing RENAME rather than TRUNCATE,
reset the flag at the end so that persistent statistics can be calculated
again.

The race condition was encountered with ASAN and rr.
Sorry, there is no test case, like there is for nothing related to
dict_stats_wait_bg_to_stop_using_table(). The entire code is an ugly
work-around for the failure of dict_stats_process_entry_from_recalc_pool()
to acquire MDL.

Note: It appears that an ALTER TABLE that is not rebuilding the table
will fail to reset the flag that blocks the processing of statistics.
  • Loading branch information
dr-m committed Mar 4, 2021
1 parent 5da6ffe commit 978e48c
Showing 1 changed file with 18 additions and 14 deletions.
32 changes: 18 additions & 14 deletions storage/innobase/handler/ha_innodb.cc
Expand Up @@ -13518,17 +13518,10 @@ innobase_drop_database(
@param[in,out] trx InnoDB data dictionary transaction
@param[in] from old table name
@param[in] to new table name
@param[in] commit whether to commit trx
@param[in] use_fk whether to parse and enforce FOREIGN KEY constraints
@param[in] commit whether to commit trx (and to enforce FOREIGN KEY)
@return DB_SUCCESS or error code */
inline
dberr_t
innobase_rename_table(
trx_t* trx,
const char* from,
const char* to,
bool commit,
bool use_fk)
inline dberr_t innobase_rename_table(trx_t *trx, const char *from,
const char *to, bool commit)
{
dberr_t error;
char norm_to[FN_REFLEN];
Expand Down Expand Up @@ -13561,6 +13554,9 @@ innobase_rename_table(
Convert lock_wait_timeout unit from second to 250 milliseconds */
long int lock_wait_timeout = thd_lock_wait_timeout(trx->mysql_thd) * 4;
if (table != NULL) {
if (commit) {
dict_stats_wait_bg_to_stop_using_table(table, trx);
}
for (dict_index_t* index = dict_table_get_first_index(table);
index != NULL;
index = dict_table_get_next_index(index)) {
Expand All @@ -13574,7 +13570,9 @@ innobase_rename_table(
}
}
}
dict_table_close(table, TRUE, FALSE);
if (!commit) {
dict_table_close(table, TRUE, FALSE);
}
}

/* FTS sync is in progress. We shall timeout this operation */
Expand All @@ -13589,7 +13587,7 @@ innobase_rename_table(
ut_a(trx->will_lock > 0);

error = row_rename_table_for_mysql(norm_from, norm_to, trx, commit,
use_fk);
commit);

if (error != DB_SUCCESS) {
if (error == DB_TABLE_NOT_FOUND
Expand Down Expand Up @@ -13641,6 +13639,10 @@ innobase_rename_table(

func_exit:
if (commit) {
if (table) {
table->stats_bg_flag &= ~BG_STAT_SHOULD_QUIT;
dict_table_close(table, TRUE, FALSE);
}
row_mysql_unlock_data_dictionary(trx);
}

Expand Down Expand Up @@ -13726,9 +13728,11 @@ int ha_innobase::truncate()
++trx->will_lock;
trx_set_dict_operation(trx, TRX_DICT_OP_TABLE);
row_mysql_lock_data_dictionary(trx);
dict_stats_wait_bg_to_stop_using_table(ib_table, trx);

int err = convert_error_code_to_mysql(
innobase_rename_table(trx, ib_table->name.m_name, temp_name,
false, false),
false),
ib_table->flags, m_user_thd);
if (err) {
trx_rollback_for_mysql(trx);
Expand Down Expand Up @@ -13811,7 +13815,7 @@ ha_innobase::rename_table(
++trx->will_lock;
trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);

dberr_t error = innobase_rename_table(trx, from, to, true, true);
dberr_t error = innobase_rename_table(trx, from, to, true);

DEBUG_SYNC(thd, "after_innobase_rename_table");

Expand Down

0 comments on commit 978e48c

Please sign in to comment.