From ebddd3ff14701b84ccd04fc3ad061cd427cd33dd Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 17:39:43 +0400 Subject: [PATCH] MDL_ticket cleanup Removed useless MDL_ticket::create() and MDL_ticket::destroy() indirection. It didn't serve any purpose. Moved mysql_mdl_create() PFS call out of critical section. Moved ticket->m_time assignment out of MDL_lock::m_rwlock protection. It increases critical section size for no good reason. Assigning it before critical section must be good enough for statistics purposes, since we must not do long waits here anyway. 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. --- sql/mdl.cc | 89 +++++++++++++++++------------------------------------- sql/mdl.h | 29 ++---------------- 2 files changed, 29 insertions(+), 89 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 2e1f7b72b61..fd28ecf891a 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -1116,35 +1116,28 @@ void MDL_request::init_by_key_with_source(const MDL_key *key_arg, } -/** - Auxiliary functions needed for creation/destruction of MDL_ticket - objects. - - @todo This naive implementation should be replaced with one that saves - on memory allocation by reusing released objects. -*/ - -MDL_ticket *MDL_ticket::create(MDL_context *ctx_arg, enum_mdl_type type_arg +MDL_ticket::MDL_ticket(MDL_context *ctx_arg, MDL_request *mdl_request): #ifndef DBUG_OFF - , enum_mdl_duration duration_arg + m_duration(mdl_request->duration), #endif - ) + m_time(0), + m_type(mdl_request->type), + m_ctx(ctx_arg), + m_lock(nullptr) { - return new (std::nothrow) - MDL_ticket(ctx_arg, type_arg -#ifndef DBUG_OFF - , duration_arg -#endif - ); + m_psi= mysql_mdl_create(this, + &mdl_request->key, + mdl_request->type, + mdl_request->duration, + PENDING, + mdl_request->m_src_file, + mdl_request->m_src_line); } -void MDL_ticket::destroy(MDL_ticket *ticket) +MDL_ticket::~MDL_ticket() { - mysql_mdl_destroy(ticket->m_psi); - ticket->m_psi= NULL; - - delete ticket; + mysql_mdl_destroy(m_psi); } @@ -2093,7 +2086,7 @@ MDL_context::try_acquire_lock(MDL_request *mdl_request) */ DBUG_ASSERT(! ticket->m_lock->is_empty()); mysql_prlock_unlock(&ticket->m_lock->m_rwlock); - MDL_ticket::destroy(ticket); + delete ticket; } return FALSE; @@ -2169,37 +2162,24 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request, if (fix_pins()) return TRUE; - if (!(ticket= MDL_ticket::create(this, mdl_request->type -#ifndef DBUG_OFF - , mdl_request->duration -#endif - ))) + if (!(ticket= new (std::nothrow) MDL_ticket(this, mdl_request))) return TRUE; + if (metadata_lock_info_plugin_loaded) + ticket->m_time= microsecond_interval_timer(); + /* The below call implicitly locks MDL_lock::m_rwlock on success. */ if (!(lock= mdl_locks.find_or_insert(m_pins, key))) { - MDL_ticket::destroy(ticket); + delete ticket; return TRUE; } - DBUG_ASSERT(ticket->m_psi == NULL); - ticket->m_psi= mysql_mdl_create(ticket, - &mdl_request->key, - mdl_request->type, - mdl_request->duration, - MDL_ticket::PENDING, - mdl_request->m_src_file, - mdl_request->m_src_line); - ticket->m_lock= lock; if (lock->can_grant_lock(mdl_request->type, this, false)) { - if (metadata_lock_info_plugin_loaded) - ticket->m_time= microsecond_interval_timer(); lock->m_granted.add_ticket(ticket); - mysql_prlock_unlock(&lock->m_rwlock); m_tickets[mdl_request->duration].push_front(ticket); @@ -2245,26 +2225,13 @@ MDL_context::clone_ticket(MDL_request *mdl_request) return TRUE; /* - By submitting mdl_request->type to MDL_ticket::create() + By submitting mdl_request->type to MDL_ticket constructor we effectively downgrade the cloned lock to the level of the request. */ - if (!(ticket= MDL_ticket::create(this, mdl_request->type -#ifndef DBUG_OFF - , mdl_request->duration -#endif - ))) + if (!(ticket= new (std::nothrow) MDL_ticket(this, mdl_request))) return TRUE; - DBUG_ASSERT(ticket->m_psi == NULL); - ticket->m_psi= mysql_mdl_create(ticket, - &mdl_request->key, - mdl_request->type, - mdl_request->duration, - MDL_ticket::PENDING, - mdl_request->m_src_file, - mdl_request->m_src_line); - /* clone() is not supposed to be used to get a stronger lock. */ DBUG_ASSERT(mdl_request->ticket->has_stronger_or_equal_type(ticket->m_type)); @@ -2392,7 +2359,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) { DBUG_PRINT("mdl", ("Nowait: %s", ticket_msg)); mysql_prlock_unlock(&lock->m_rwlock); - MDL_ticket::destroy(ticket); + delete ticket; my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); DBUG_RETURN(TRUE); } @@ -2411,8 +2378,6 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) } #endif /* WITH_WSREP */ - if (metadata_lock_info_plugin_loaded) - ticket->m_time= microsecond_interval_timer(); lock->m_waiting.add_ticket(ticket); /* @@ -2533,7 +2498,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); - MDL_ticket::destroy(ticket); + delete ticket; switch (wait_status) { case MDL_wait::VICTIM: @@ -2717,7 +2682,7 @@ MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket, if (is_new_ticket) { m_tickets[MDL_TRANSACTION].remove(mdl_xlock_request.ticket); - MDL_ticket::destroy(mdl_xlock_request.ticket); + delete mdl_xlock_request.ticket; } DBUG_RETURN(FALSE); @@ -2986,7 +2951,7 @@ void MDL_context::release_lock(enum_mdl_duration duration, MDL_ticket *ticket) lock->release(m_pins, ticket); m_tickets[duration].remove(ticket); - MDL_ticket::destroy(ticket); + delete ticket; DBUG_VOID_RETURN; } diff --git a/sql/mdl.h b/sql/mdl.h index 5d0a24bda18..92fae6942dd 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -750,33 +750,8 @@ private: friend class MDL_context; friend class MDL_lock; - MDL_ticket(MDL_context *ctx_arg, enum_mdl_type type_arg -#ifndef DBUG_OFF - , enum_mdl_duration duration_arg -#endif - ) - : -#ifndef DBUG_OFF - m_duration(duration_arg), -#endif - m_time(0), - m_type(type_arg), - m_ctx(ctx_arg), - m_lock(NULL), - m_psi(NULL) - {} - - virtual ~MDL_ticket() - { - DBUG_ASSERT(m_psi == NULL); - } - - static MDL_ticket *create(MDL_context *ctx_arg, enum_mdl_type type_arg -#ifndef DBUG_OFF - , enum_mdl_duration duration_arg -#endif - ); - static void destroy(MDL_ticket *ticket); + MDL_ticket(MDL_context *ctx_arg, MDL_request *request); + ~MDL_ticket(); private: /** Type of metadata lock. Externally accessible. */ enum enum_mdl_type m_type;