Commit 201cfc33 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-30638 Deadlock between INSERT and InnoDB non-persistent statistics update

This is a partial revert of
commit 8b6a308e (MDEV-29883)
and a follow-up to the
merge commit 394fc71f (MDEV-24569).

The latching order related to any operation that accesses the allocation
metadata of an InnoDB index tree is as follows:

1. Acquire dict_index_t::lock in non-shared mode.
2. Acquire the index root page latch in non-shared mode.
3. Possibly acquire further index page latches. Unless an exclusive
dict_index_t::lock is held, this must follow the root-to-leaf,
left-to-right order.
4. Acquire a *non-shared* fil_space_t::latch.
5. Acquire latches on the allocation metadata pages.
6. Possibly allocate and write some pages, or free some pages.

btr_get_size_and_reserved(), dict_stats_update_transient_for_index(),
dict_stats_analyze_index(): Acquire an exclusive fil_space_t::latch
in order to avoid a deadlock in fseg_n_reserved_pages() in case of
concurrent access to multiple indexes sharing the same "inode page".

fseg_page_is_allocated(): Acquire an exclusive fil_space_t::latch
in order to avoid deadlocks. All callers are holding latches
on a buffer pool page, or an index, or both.
Before commit edbde4a1 (MDEV-24167)
a third mode was available that would not conflict with the shared
fil_space_t::latch acquired by ha_innobase::info_low(),
i_s_sys_tablespaces_fill_table(),
or i_s_tablespaces_encryption_fill_table().
Because those calls should be rather rare, it makes sense to use
the simple rw_lock with only shared and exclusive modes.

fil_crypt_get_page_throttle(): Avoid invoking fseg_page_is_allocated()
on an allocation bitmap page (which can never be freed), to avoid
acquiring a shared latch on top of an exclusive one.

mtr_t::s_lock_space(), MTR_MEMO_SPACE_S_LOCK: Remove.
parent 54c0ac72
......@@ -314,7 +314,7 @@ btr_get_size_and_reserved(
return ULINT_UNDEFINED;
}
mtr->s_lock_space(index->table->space);
mtr->x_lock_space(index->table->space);
ulint n = fseg_n_reserved_pages(*root, PAGE_HEADER + PAGE_BTR_SEG_LEAF
+ root->page.frame, used, mtr);
......
......@@ -1490,7 +1490,7 @@ dict_stats_update_transient_for_index(
goto invalid;
}
mtr.s_lock_space(index->table->space);
mtr.x_lock_space(index->table->space);
ulint dummy, size;
index->stat_index_size
......@@ -2697,7 +2697,7 @@ static index_stats_t dict_stats_analyze_index(dict_index_t* index)
}
uint16_t root_level = btr_page_get_level(root->page.frame);
mtr.s_lock_space(index->table->space);
mtr.x_lock_space(index->table->space);
ulint dummy, size;
result.index_size
= fseg_n_reserved_pages(*root, PAGE_HEADER + PAGE_BTR_SEG_LEAF
......
......@@ -1677,8 +1677,9 @@ fil_crypt_get_page_throttle(
return NULL;
}
if (DB_SUCCESS_LOCKED_REC
!= fseg_page_is_allocated(space, state->offset)) {
if (offset % (zip_size ? zip_size : srv_page_size)
&& DB_SUCCESS_LOCKED_REC
!= fseg_page_is_allocated(space, offset)) {
/* page is already freed */
return NULL;
}
......
......@@ -2653,7 +2653,7 @@ dberr_t fseg_page_is_allocated(fil_space_t *space, unsigned page)
mtr.start();
if (!space->is_owner())
mtr.s_lock_space(space);
mtr.x_lock_space(space);
if (page >= space->free_limit || page >= space->size_in_header);
else if (const buf_block_t *b=
......
......@@ -282,17 +282,6 @@ struct mtr_t {
memo_push(lock, MTR_MEMO_SX_LOCK);
}
/** Acquire a tablespace S-latch.
@param space tablespace */
void s_lock_space(fil_space_t *space)
{
ut_ad(space->purpose == FIL_TYPE_TEMPORARY ||
space->purpose == FIL_TYPE_IMPORT ||
space->purpose == FIL_TYPE_TABLESPACE);
memo_push(space, MTR_MEMO_SPACE_S_LOCK);
space->s_lock();
}
/** Acquire an exclusive tablespace latch.
@param space tablespace */
void x_lock_space(fil_space_t *space);
......@@ -357,9 +346,8 @@ struct mtr_t {
/** Check if we are holding tablespace latch
@param space tablespace to search for
@param shared whether to look for shared latch, instead of exclusive
@return whether space.latch is being held */
bool memo_contains(const fil_space_t& space, bool shared= false) const
bool memo_contains(const fil_space_t& space) const
MY_ATTRIBUTE((warn_unused_result));
#ifdef UNIV_DEBUG
/** Check if we are holding an rw-latch in this mini-transaction
......@@ -412,7 +400,7 @@ struct mtr_t {
break;
case MTR_MEMO_MODIFY:
case MTR_MEMO_S_LOCK: case MTR_MEMO_X_LOCK: case MTR_MEMO_SX_LOCK:
case MTR_MEMO_SPACE_X_LOCK: case MTR_MEMO_SPACE_S_LOCK:
case MTR_MEMO_SPACE_X_LOCK:
ut_ad("invalid type" == 0);
}
#endif
......
......@@ -337,8 +337,6 @@ enum mtr_memo_type_t {
MTR_MEMO_SX_LOCK = RW_SX_LATCH << 5,
/** wr_lock() on fil_space_t::latch */
MTR_MEMO_SPACE_X_LOCK = MTR_MEMO_SX_LOCK << 1,
/** rd_lock() on fil_space_t::latch */
MTR_MEMO_SPACE_S_LOCK = MTR_MEMO_SX_LOCK << 2
MTR_MEMO_SPACE_X_LOCK = MTR_MEMO_SX_LOCK << 1
};
#endif /* !UNIV_INNOCHECKSUM */
......@@ -55,9 +55,6 @@ void mtr_memo_slot_t::release() const
static_cast<fil_space_t*>(object)->set_committed_size();
static_cast<fil_space_t*>(object)->x_unlock();
break;
case MTR_MEMO_SPACE_S_LOCK:
static_cast<fil_space_t*>(object)->s_unlock();
break;
default:
buf_page_t *bpage= static_cast<buf_page_t*>(object);
ut_d(const auto s=)
......@@ -912,18 +909,14 @@ bool mtr_t::have_u_or_x_latch(const buf_block_t &block) const
/** Check if we are holding exclusive tablespace latch
@param space tablespace to search for
@param shared whether to look for shared latch, instead of exclusive
@return whether space.latch is being held */
bool mtr_t::memo_contains(const fil_space_t& space, bool shared) const
bool mtr_t::memo_contains(const fil_space_t& space) const
{
const mtr_memo_type_t type= shared
? MTR_MEMO_SPACE_S_LOCK : MTR_MEMO_SPACE_X_LOCK;
for (const mtr_memo_slot_t &slot : m_memo)
{
if (slot.object == &space && slot.type == type)
if (slot.object == &space && slot.type == MTR_MEMO_SPACE_X_LOCK)
{
ut_ad(shared || space.is_owner());
ut_ad(space.is_owner());
return true;
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment