Move m_wait.reset_status() out of critical section

MDL_wait::m_wait_status has to be reset to EMPTY state before going
into waiting routines. There're three options to get it done:

1. Before MDL_lock::rw_lock critical section of
   MDL_context::acquire_lock(). In this case MDL_context is not exposed
   neither via MDL_lock::m_waiting nor has it MDL_context::m_waiting_for
   set.
   Cons: m_wait.reset_status() brings unwanted overhead when lock can
   be served immediately.

2. Current solution. Within MDL_lock::rw_lock critical section of
   MDL_context::acquire_lock(). MDL_context::m_waiting_for is not yet set
   however MDL_context was already exposed via MDL_lock::m_waiting list.
   The latter is not a problem since we're still holding exclusive lock
   on MDL_lock::rw_lock.
   Cons: increases critical section size for no good reason.

3. Whenever MDL_wait is created and after wait in
   MDL_context::acquire_lock() is completed. At this point MDL_context
   is not exposed via MDL_lock::m_waiting anymore and
   MDL_context::m_waiting_for is reset.
   Cons: none, it is just plain beauty.

Now MDL_wait::m_wait_status is manipulated as following:

EMPTY - set whenever MDL_wait object is created and after each wait
GRANTED - can be set by a thread that releases incompatible lock
VICTIM - can be set either by owner thread or by concurrent higher
         priority thread during deadlock detection
TIMEOUT - always set by owner thread
KILLED - always set by owner thread

This is part of broader cleanup, which aims to make large part of
MDL_lock members private. It is needed to simplify further work on
MDEV-19749 - MDL scalability regression after backup locks.
This commit is contained in:
Sergey Vojtovich 2025-05-03 02:10:01 +04:00 committed by Sergei Golubchik
parent 542ba51c4c
commit 39c031bcf9
2 changed files with 5 additions and 11 deletions

View File

@ -2312,6 +2312,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
MDL_ticket *ticket;
MDL_wait::enum_wait_status wait_status;
DBUG_ENTER("MDL_context::acquire_lock");
DBUG_ASSERT(m_wait.get_status() == MDL_wait::EMPTY);
#ifdef DBUG_TRACE
const char *mdl_lock_name= get_mdl_lock_name(
mdl_request->key.mdl_namespace(), mdl_request->type)->str;
@ -2371,14 +2372,6 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
lock= ticket->m_lock;
lock->m_waiting.add_ticket(ticket);
/*
Once we added a pending ticket to the waiting queue,
we must ensure that our wait slot is empty, so
that our lock request can be scheduled. Do that in the
critical section formed by the acquired write lock on MDL_lock.
*/
m_wait.reset_status();
/*
Don't break conflicting locks if timeout is 0 as 0 is used
to check if there is any conflicting locks...
@ -2489,6 +2482,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
if (wait_status != MDL_wait::GRANTED)
{
lock->abort_wait(m_pins, ticket);
m_wait.reset_status();
delete ticket;
switch (wait_status)
{
@ -2518,7 +2512,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
concurrent thread (@sa MDL_lock:reschedule_waiters()).
So all we need to do is to update MDL_context and MDL_request objects.
*/
DBUG_ASSERT(wait_status == MDL_wait::GRANTED);
m_wait.reset_status();
m_tickets[mdl_request->duration].push_front(ticket);

View File

@ -5863,11 +5863,10 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime,
mysql_mutex_assert_owner(&tdc->LOCK_table_share);
DBUG_ASSERT(tdc->flushed);
DBUG_ASSERT(mdl_context->m_wait.get_status() == MDL_wait::EMPTY);
tdc->m_flush_tickets.push_front(&ticket);
mdl_context->m_wait.reset_status();
mysql_mutex_unlock(&tdc->LOCK_table_share);
mdl_context->will_wait_for(&ticket);
@ -5884,6 +5883,7 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime,
mysql_cond_broadcast(&tdc->COND_release);
mysql_mutex_unlock(&tdc->LOCK_table_share);
mdl_context->m_wait.reset_status();
/*
In cases when our wait was aborted by KILL statement,