Skip to content

Commit

Permalink
MDEV-28289 fts_optimize_sync_table() is acquiring dict_sys.latch whil…
Browse files Browse the repository at this point in the history
…e holding it

dict_acquire_mdl_shared(): Invoke the correct variant
dict_table_t::parse_name<true>() also when trylock=true,
that is, we are being called from fts_optimize_sync_table().

Ever since commit 82b7c56 (MDEV-24258)
this code was prone to a hang. If another thread requested an
exclusive dict_sys.latch between the time
dict_acquire_mdl_shared<trylock=true>() acquired a shared dict_sys.latch
and dict_table_t::parse_name<false>() was trying to acquire another
shared dict_sys.latch, InnoDB would get into an infinite livelock
of threads, because the futex-based srw-lock implementation prioritizes
exclusive latch requests.

dict_table_t::parse_name(): Rename the template parameter dict_locked
into dict_frozen.
  • Loading branch information
dr-m committed Apr 11, 2022
1 parent 7bccf3d commit 840bab8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 23 deletions.
18 changes: 9 additions & 9 deletions storage/innobase/dict/dict0dict.cc
Original file line number Diff line number Diff line change
Expand Up @@ -602,21 +602,21 @@ dict_index_get_nth_field_pos(
}

/** Parse the table file name into table name and database name.
@tparam dict_locked whether dict_sys.lock() was called
@param[in,out] db_name database name buffer
@param[in,out] tbl_name table name buffer
@param[out] db_name_len database name length
@param[out] tbl_name_len table name length
@tparam dict_frozen whether the caller holds dict_sys.latch
@param[in,out] db_name database name buffer
@param[in,out] tbl_name table name buffer
@param[out] db_name_len database name length
@param[out] tbl_name_len table name length
@return whether the table name is visible to SQL */
template<bool dict_locked>
template<bool dict_frozen>
bool dict_table_t::parse_name(char (&db_name)[NAME_LEN + 1],
char (&tbl_name)[NAME_LEN + 1],
size_t *db_name_len, size_t *tbl_name_len) const
{
char db_buf[MAX_DATABASE_NAME_LEN + 1];
char tbl_buf[MAX_TABLE_NAME_LEN + 1];

if (!dict_locked)
if (!dict_frozen)
dict_sys.freeze(SRW_LOCK_CALL); /* protect against renaming */
ut_ad(dict_sys.frozen());
const size_t db_len= name.dblen();
Expand All @@ -636,7 +636,7 @@ bool dict_table_t::parse_name(char (&db_name)[NAME_LEN + 1],
memcpy(tbl_buf, mdl_name.m_name + db_len + 1, tbl_len);
tbl_buf[tbl_len]= 0;

if (!dict_locked)
if (!dict_frozen)
dict_sys.unfreeze();

*db_name_len= filename_to_tablename(db_buf, db_name,
Expand Down Expand Up @@ -782,7 +782,7 @@ dict_acquire_mdl_shared(dict_table_t *table,

size_t db1_len, tbl1_len;

if (!table->parse_name<!trylock>(db_buf1, tbl_buf1, &db1_len, &tbl1_len))
if (!table->parse_name<true>(db_buf1, tbl_buf1, &db1_len, &tbl1_len))
{
/* The table was renamed to #sql prefix.
Release MDL (if any) for the old name and return. */
Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/include/dict0dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Copyright (c) 1996, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2012, Facebook Inc.
Copyright (c) 2013, 2021, MariaDB Corporation.
Copyright (c) 2013, 2022, 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 @@ -1570,7 +1570,7 @@ class dict_sys_t
}
else
lock_wait(SRW_LOCK_ARGS(file, line));
}
}

#ifdef UNIV_PFS_RWLOCK
/** Unlock the data dictionary cache. */
Expand Down
24 changes: 12 additions & 12 deletions storage/innobase/include/dict0mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Copyright (c) 1996, 2017, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2012, Facebook Inc.
Copyright (c) 2013, 2021, MariaDB Corporation.
Copyright (c) 2013, 2022, 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 @@ -1934,17 +1934,17 @@ struct dict_table_t {
/** For overflow fields returns potential max length stored inline */
inline size_t get_overflow_field_local_len() const;

/** Parse the table file name into table name and database name.
@tparam dict_locked whether dict_sys.lock() was called
@param[in,out] db_name database name buffer
@param[in,out] tbl_name table name buffer
@param[out] db_name_len database name length
@param[out] tbl_name_len table name length
@return whether the table name is visible to SQL */
template<bool dict_locked= false>
bool parse_name(char (&db_name)[NAME_LEN + 1],
char (&tbl_name)[NAME_LEN + 1],
size_t *db_name_len, size_t *tbl_name_len) const;
/** Parse the table file name into table name and database name.
@tparam dict_frozen whether the caller holds dict_sys.latch
@param[in,out] db_name database name buffer
@param[in,out] tbl_name table name buffer
@param[out] db_name_len database name length
@param[out] tbl_name_len table name length
@return whether the table name is visible to SQL */
template<bool dict_frozen= false>
bool parse_name(char (&db_name)[NAME_LEN + 1],
char (&tbl_name)[NAME_LEN + 1],
size_t *db_name_len, size_t *tbl_name_len) const;

/** Clear the table when rolling back TRX_UNDO_EMPTY */
void clear(que_thr_t *thr);
Expand Down

0 comments on commit 840bab8

Please sign in to comment.