From 19718e1b555f6f798f7d09a0244c4f6e63f6a5db Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 30 Apr 2025 20:03:27 +0400 Subject: [PATCH 01/15] MDL_lock encapsulation: MDL_lock::get_lock_owner() Avoid accessing MDL_lock::m_rwlock from MDL_map::get_lock_owner(), code moved to MDL_lock::get_lock_owner() instead. get_lock_owner() doesn't have to check BACKUP namespace since it is used by user level locks only. 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 | 72 +++++++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 255271ccef951..359076e75fee5 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -584,7 +584,7 @@ class MDL_lock If m_wrlock prefers readers (actually ignoring pending writers is enough) ctxA and ctxB will continue and no deadlock will occur. */ - mysql_prlock_t m_rwlock; + mutable mysql_prlock_t m_rwlock; bool is_empty() const { @@ -601,8 +601,6 @@ class MDL_lock bool can_grant_lock(enum_mdl_type type, MDL_context *requstor_ctx, bool ignore_lock_priority) const; - inline unsigned long get_lock_owner() const; - void reschedule_waiters(); void remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*queue, @@ -686,6 +684,29 @@ class MDL_lock lock->m_strategy= &m_object_lock_strategy; } + + /** + Return thread id of the thread to which the first ticket was + granted. + + Intended for user level locks. They make use of SNW lock only, + thus there can be only one granted ticket. + + We can skip check for m_strategy here, because m_granted + must be empty for such locks anyway. + */ + unsigned long get_lock_owner() const + { + unsigned long res= 0; + DBUG_ASSERT(key.mdl_namespace() == MDL_key::USER_LOCK); + mysql_prlock_rdlock(&m_rwlock); + DBUG_ASSERT(m_strategy || is_empty()); + if (!m_granted.is_empty()) + res= m_granted.begin()->get_ctx()->get_thread_id(); + mysql_prlock_unlock(&m_rwlock); + return res; + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -893,31 +914,15 @@ MDL_lock* MDL_map::find_or_insert(LF_PINS *pins, const MDL_key *mdl_key) unsigned long MDL_map::get_lock_owner(LF_PINS *pins, const MDL_key *mdl_key) { - unsigned long res= 0; - - if (mdl_key->mdl_namespace() == MDL_key::BACKUP) - { - mysql_prlock_rdlock(&m_backup_lock->m_rwlock); - res= m_backup_lock->get_lock_owner(); - mysql_prlock_unlock(&m_backup_lock->m_rwlock); - } - else + DBUG_ASSERT(mdl_key->mdl_namespace() == MDL_key::USER_LOCK); + if (MDL_lock *lock= (MDL_lock*) lf_hash_search(&m_locks, pins, mdl_key->ptr(), + mdl_key->length())) { - MDL_lock *lock= (MDL_lock*) lf_hash_search(&m_locks, pins, mdl_key->ptr(), - mdl_key->length()); - if (lock) - { - /* - We can skip check for m_strategy here, becase m_granted - must be empty for such locks anyway. - */ - mysql_prlock_rdlock(&lock->m_rwlock); - res= lock->get_lock_owner(); - mysql_prlock_unlock(&lock->m_rwlock); - lf_hash_search_unpin(pins); - } + unsigned long res= lock->get_lock_owner(); + lf_hash_search_unpin(pins); + return res; } - return res; + return 0; } @@ -1811,21 +1816,6 @@ MDL_lock::can_grant_lock(enum_mdl_type type_arg, } -/** - Return thread id of the thread to which the first ticket was - granted. -*/ - -inline unsigned long -MDL_lock::get_lock_owner() const -{ - if (m_granted.is_empty()) - return 0; - - return m_granted.begin()->get_ctx()->get_thread_id(); -} - - /** Remove a ticket from waiting or pending queue and wakeup up waiters. */ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, From e617e1dd4dbabd2be88cd035d534bfcbaaf25611 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 30 Apr 2025 20:58:00 +0400 Subject: [PATCH 02/15] MDL_lock encapsulation: MDL_lock::iterate() Avoid accessing MDL_lock::m_waiting, MDL_lock::m_granted and MDL_lock::m_rwlock from mdl_iterate_lock(), use MDL_lock::iterate() instead. 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 | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 359076e75fee5..dedad4f048cc6 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -707,6 +707,35 @@ class MDL_lock return res; } + + /** + Callback for mdl_iterate() + + Another thread may be deleting this MDL_lock concurrently. + Being deleted lock can still be iterated since it must have + valid empty granted/waiting lists. + */ + bool iterate(mdl_iterator_callback callback, void *argument) + { + bool res= true; + mysql_prlock_rdlock(&m_rwlock); + DBUG_ASSERT(m_strategy || is_empty()); + for (MDL_ticket &ticket : m_granted) + { + if (callback(&ticket, argument, true)) + goto end; + } + for (MDL_ticket &ticket : m_waiting) + { + if (callback(&ticket, argument, false)) + goto end; + } + res= false; +end: + mysql_prlock_unlock(&m_rwlock); + return res; + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -787,21 +816,7 @@ static my_bool mdl_iterate_lock(void *lk, void *a) { MDL_lock *lock= static_cast(lk); mdl_iterate_arg *arg= static_cast(a); - /* - We can skip check for m_strategy here, becase m_granted - must be empty for such locks anyway. - */ - mysql_prlock_rdlock(&lock->m_rwlock); - bool res= std::any_of(lock->m_granted.begin(), lock->m_granted.end(), - [arg](MDL_ticket &ticket) { - return arg->callback(&ticket, arg->argument, true); - }); - res= std::any_of(lock->m_waiting.begin(), lock->m_waiting.end(), - [arg](MDL_ticket &ticket) { - return arg->callback(&ticket, arg->argument, false); - }); - mysql_prlock_unlock(&lock->m_rwlock); - return res; + return lock->iterate(arg->callback, arg->argument); } From c8a3544bd94b1765b548be436b3ff3f2b29c878c Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 30 Apr 2025 23:56:15 +0400 Subject: [PATCH 03/15] MDL_lock encapsulation: add_cloned_ticket() Avoid accessing MDL_lock::m_granted and MDL_lock::m_rwlock from MDL_context::clone_ticket(), use MDL_lock::add_cloned_ticket() instead. 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 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index dedad4f048cc6..08a063607e191 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -736,6 +736,14 @@ class MDL_lock return res; } + + void add_cloned_ticket(MDL_ticket *ticket) + { + mysql_prlock_wrlock(&m_rwlock); + m_granted.add_ticket(ticket); + mysql_prlock_unlock(&m_rwlock); + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -2211,9 +2219,7 @@ MDL_context::clone_ticket(MDL_request *mdl_request) ticket->m_time= mdl_request->ticket->m_time; mdl_request->ticket= ticket; - mysql_prlock_wrlock(&ticket->m_lock->m_rwlock); - ticket->m_lock->m_granted.add_ticket(ticket); - mysql_prlock_unlock(&ticket->m_lock->m_rwlock); + ticket->m_lock->add_cloned_ticket(ticket); m_tickets[mdl_request->duration].push_front(ticket); From 9fc7cfa70a9eab3b1427a1441c167df5efbe1e98 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 00:37:01 +0400 Subject: [PATCH 04/15] MDL_lock encapsulation: MDL_lock::downgrade() Avoid accessing MDL_lock::m_granted and MDL_lock::m_rwlock from MDL_ticket::downgrade_lock(), use MDL_lock::downgrade() instead. 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 | 28 ++++++++++++++++++---------- sql/mdl.h | 1 + 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 08a063607e191..aac32a8bc7896 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -744,6 +744,23 @@ class MDL_lock mysql_prlock_unlock(&m_rwlock); } + + /** + MDL_context::downgrade_lock() helper + + To update state of MDL_lock object correctly we need to temporarily + exclude ticket from the granted queue and then include it back. + */ + void downgrade(MDL_ticket *ticket, enum_mdl_type type) + { + mysql_prlock_wrlock(&m_rwlock); + m_granted.remove_ticket(ticket); + ticket->m_type= type; + m_granted.add_ticket(ticket); + reschedule_waiters(); + mysql_prlock_unlock(&m_rwlock); + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -3061,16 +3078,7 @@ void MDL_ticket::downgrade_lock(enum_mdl_type type) m_type == MDL_BACKUP_BLOCK_DDL || m_type == MDL_BACKUP_WAIT_FLUSH))); - mysql_prlock_wrlock(&m_lock->m_rwlock); - /* - To update state of MDL_lock object correctly we need to temporarily - exclude ticket from the granted queue and then include it back. - */ - m_lock->m_granted.remove_ticket(this); - m_type= type; - m_lock->m_granted.add_ticket(this); - m_lock->reschedule_waiters(); - mysql_prlock_unlock(&m_lock->m_rwlock); + m_lock->downgrade(this, type); DBUG_VOID_RETURN; } diff --git a/sql/mdl.h b/sql/mdl.h index 00a129810f039..5d0a24bda18c4 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -748,6 +748,7 @@ class MDL_ticket : public MDL_wait_for_subgraph, public ilist_node<> PRE_ACQUIRE_NOTIFY, POST_RELEASE_NOTIFY }; private: friend class MDL_context; + friend class MDL_lock; MDL_ticket(MDL_context *ctx_arg, enum_mdl_type type_arg #ifndef DBUG_OFF From 7934f1f774dd2813f11e7535d6bb881211eb1dfa Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 01:12:08 +0400 Subject: [PATCH 05/15] MDL_lock encapsulation: MDL_lock::upgrade() Avoid accessing MDL_lock::m_granted and MDL_lock::m_rwlock from MDL_ticket::upgrade_shared_lock(), use MDL_lock::upgrade() instead. 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 | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index aac32a8bc7896..ef93e0e147c09 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -761,6 +761,25 @@ class MDL_lock mysql_prlock_unlock(&m_rwlock); } + + /** + MDL_context::upgrade_shared_lock() helper + + To update state of MDL_lock object correctly we need to temporarily + exclude ticket from the granted queue and then include it back. + */ + void upgrade(MDL_ticket *ticket, enum_mdl_type type, + MDL_ticket *remove) + { + mysql_prlock_wrlock(&m_rwlock); + if (remove) + m_granted.remove_ticket(remove); + m_granted.remove_ticket(ticket); + ticket->m_type= type; + m_granted.add_ticket(ticket); + mysql_prlock_unlock(&m_rwlock); + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -2677,20 +2696,9 @@ MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket, is_new_ticket= ! has_lock(mdl_svp, mdl_xlock_request.ticket); - /* Merge the acquired and the original lock. @todo: move to a method. */ - mysql_prlock_wrlock(&mdl_ticket->m_lock->m_rwlock); - if (is_new_ticket) - mdl_ticket->m_lock->m_granted.remove_ticket(mdl_xlock_request.ticket); - /* - Set the new type of lock in the ticket. To update state of - MDL_lock object correctly we need to temporarily exclude - ticket from the granted queue and then include it back. - */ - mdl_ticket->m_lock->m_granted.remove_ticket(mdl_ticket); - mdl_ticket->m_type= new_type; - mdl_ticket->m_lock->m_granted.add_ticket(mdl_ticket); - - mysql_prlock_unlock(&mdl_ticket->m_lock->m_rwlock); + /* Merge the acquired and the original lock. */ + mdl_ticket->m_lock->upgrade(mdl_ticket, new_type, + is_new_ticket ? mdl_xlock_request.ticket : nullptr); if (is_new_ticket) { From d4fcaee307b1bc6e06cf94c6afabfc39ab9ad619 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 13:40:36 +0400 Subject: [PATCH 06/15] MDL_lock encapsulation: notify_conflicting_locks() Avoid accessing MDL_lock::m_rwlock from MDL_context::acquire_lock(), use MDL_lock::notify_conflicting_locks_if_needed() instead. Also MDL_lock::needs_notification() doesn't require MDL_lock::m_rwlock protection, so it is moved out of critical section. 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 | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index ef93e0e147c09..83faa58d00579 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -780,6 +780,17 @@ class MDL_lock mysql_prlock_unlock(&m_rwlock); } + + void notify_conflicting_locks_if_needed(MDL_ticket *ticket, bool abort_blocking) + { + if (needs_notification(ticket)) + { + mysql_prlock_wrlock(&m_rwlock); + notify_conflicting_locks(ticket->get_ctx(), abort_blocking); + mysql_prlock_unlock(&m_rwlock); + } + } + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -2500,10 +2511,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) break; } - mysql_prlock_wrlock(&lock->m_rwlock); - if (lock->needs_notification(ticket)) - lock->notify_conflicting_locks(this, abort_blocking); - mysql_prlock_unlock(&lock->m_rwlock); + lock->notify_conflicting_locks_if_needed(ticket, abort_blocking); } if (wait_status == MDL_wait::EMPTY) wait_status= m_wait.timed_wait(m_owner, &abs_timeout, TRUE, From 4896bf9298894bda92408a91c90305795cc419bb Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 15:29:53 +0400 Subject: [PATCH 07/15] MDL_lock encapsulation: MDL_lock::remove_ticket() Avoid accessing MDL_lock::m_waiting from MDL_context::acquire_lock(), use MDL_lock::abort_wait() instead. Avoid accessing MDL_lock::m_granted from MDL_context::release_lock(), use MDL_lock::release() instead. Avoid accessing MDL_lock::m_strategy and MDL_lock::m_rwlock from MDL_map::remove(), code moved to MDL_lock::remove_ticket() instead. 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 | 60 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 83faa58d00579..2e1f7b72b6132 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -175,7 +175,8 @@ class MDL_map void destroy(); MDL_lock *find_or_insert(LF_PINS *pins, const MDL_key *key); unsigned long get_lock_owner(LF_PINS *pins, const MDL_key *key); - void remove(LF_PINS *pins, MDL_lock *lock); + void remove(LF_PINS *pins, const MDL_key *key) + { lf_hash_delete(&m_locks, pins, key->ptr(), key->length()); } LF_PINS *get_pins() { return lf_hash_get_pins(&m_locks); } private: LF_HASH m_locks; /**< All acquired locks in the server. */ @@ -791,6 +792,17 @@ class MDL_lock } } + + void release(LF_PINS *pins, MDL_ticket *ticket) + { + remove_ticket(pins, &MDL_lock::m_granted, ticket); + } + + + void abort_wait(LF_PINS *pins, MDL_ticket *ticket) + { remove_ticket(pins, &MDL_lock::m_waiting, ticket); } + + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -996,27 +1008,6 @@ MDL_map::get_lock_owner(LF_PINS *pins, const MDL_key *mdl_key) } -/** - Destroy MDL_lock object or delegate this responsibility to - whatever thread that holds the last outstanding reference to - it. -*/ - -void MDL_map::remove(LF_PINS *pins, MDL_lock *lock) -{ - if (lock->key.mdl_namespace() == MDL_key::BACKUP) - { - /* Never destroy pre-allocated MDL_lock object in BACKUP namespace. */ - mysql_prlock_unlock(&lock->m_rwlock); - return; - } - - lock->m_strategy= 0; - mysql_prlock_unlock(&lock->m_rwlock); - lf_hash_delete(&m_locks, pins, lock->key.ptr(), lock->key.length()); -} - - /** Initialize a metadata locking context. @@ -1886,7 +1877,13 @@ MDL_lock::can_grant_lock(enum_mdl_type type_arg, } -/** Remove a ticket from waiting or pending queue and wakeup up waiters. */ +/** + Removes ticket from waiting or pending queue and awakes waiters. + + Once ticket is removed from the list, this thread doesn't hold + references to this lock and it can be destroyed concurrently. It + means once m_rwlock is released, "this" cannot be referenced anymore. +*/ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, MDL_ticket *ticket) @@ -1894,7 +1891,16 @@ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, mysql_prlock_wrlock(&m_rwlock); (this->*list).remove_ticket(ticket); if (is_empty()) - mdl_locks.remove(pins, this); + { + /* Never destroy pre-allocated MDL_lock object in BACKUP namespace. */ + if (key.mdl_namespace() != MDL_key::BACKUP) + { + m_strategy= 0; + mysql_prlock_unlock(&m_rwlock); + mdl_locks.remove(pins, &key); + return; + } + } else { /* @@ -1912,8 +1918,8 @@ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, pending request). */ reschedule_waiters(); - mysql_prlock_unlock(&m_rwlock); } + mysql_prlock_unlock(&m_rwlock); } @@ -2526,7 +2532,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) if (wait_status != MDL_wait::GRANTED) { - lock->remove_ticket(m_pins, &MDL_lock::m_waiting, ticket); + lock->abort_wait(m_pins, ticket); MDL_ticket::destroy(ticket); switch (wait_status) { @@ -2977,7 +2983,7 @@ void MDL_context::release_lock(enum_mdl_duration duration, MDL_ticket *ticket) DBUG_ASSERT(this == ticket->get_ctx()); DBUG_PRINT("mdl", ("Released: %s", dbug_print_mdl(ticket))); - lock->remove_ticket(m_pins, &MDL_lock::m_granted, ticket); + lock->release(m_pins, ticket); m_tickets[duration].remove(ticket); MDL_ticket::destroy(ticket); From 2866d480fca65f297432af73fb185f06d87870b2 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 May 2025 17:39:43 +0400 Subject: [PATCH 08/15] 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 2e1f7b72b6132..fd28ecf891a1f 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 5d0a24bda18c4..92fae6942dd79 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -750,33 +750,8 @@ class MDL_ticket : public MDL_wait_for_subgraph, public ilist_node<> 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; From 83fb792bf4fb37b40b0cc6360019b7c12a986273 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Fri, 2 May 2025 16:06:25 +0400 Subject: [PATCH 09/15] MDL_lock encapsulation: try_acquire_lock() Avoid accessing MDL_lock::m_rwlock from MDL_context::try_acquire_lock() and MDL_context::acquire_lock(), code moved to MDL_context::try_acquire_lock_impl() instead. It is an intermediate change that reduce uses of MDL_lock::m_rwlock out of MDL_lock class. 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 | 51 +++++++++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index fd28ecf891a1f..2ab15712a2982 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -2071,25 +2071,7 @@ MDL_context::find_ticket(MDL_request *mdl_request, bool MDL_context::try_acquire_lock(MDL_request *mdl_request) { - MDL_ticket *ticket; - - if (try_acquire_lock_impl(mdl_request, &ticket)) - return TRUE; - - if (! mdl_request->ticket) - { - /* - Our attempt to acquire lock without waiting has failed. - Let us release resources which were acquired in the process. - We can't get here if we allocated a new lock object so there - is no need to release it. - */ - DBUG_ASSERT(! ticket->m_lock->is_empty()); - mysql_prlock_unlock(&ticket->m_lock->m_rwlock); - delete ticket; - } - - return FALSE; + return try_acquire_lock_impl(mdl_request, nullptr); } @@ -2188,8 +2170,21 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request, mysql_mdl_set_status(ticket->m_psi, MDL_ticket::GRANTED); } - else + else if (out_ticket) *out_ticket= ticket; + else + { + /* + Our attempt to acquire lock without waiting has failed. + Let us release resources which were acquired in the process. + We can't get here if we allocated a new lock object so there + is no need to release it. + */ + DBUG_ASSERT(!ticket->m_lock->is_empty()); + DBUG_PRINT("mdl", ("Nowait: %s", dbug_print_mdl(ticket))); + mysql_prlock_unlock(&ticket->m_lock->m_rwlock); + delete ticket; + } return FALSE; } @@ -2325,7 +2320,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) mdl_lock_name, lock_wait_timeout)); - if (try_acquire_lock_impl(mdl_request, &ticket)) + if (try_acquire_lock_impl(mdl_request, lock_wait_timeout ? &ticket : nullptr)) { DBUG_PRINT("mdl", ("OOM: %s", mdl_lock_name)); DBUG_RETURN(TRUE); @@ -2343,27 +2338,22 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) DBUG_RETURN(FALSE); } -#ifdef DBUG_TRACE - const char *ticket_msg= dbug_print_mdl(ticket); -#endif - /* Our attempt to acquire lock without waiting has failed. As a result of this attempt we got MDL_ticket with m_lock member pointing to the corresponding MDL_lock object which has MDL_lock::m_rwlock write-locked. */ - lock= ticket->m_lock; - if (lock_wait_timeout == 0) { - DBUG_PRINT("mdl", ("Nowait: %s", ticket_msg)); - mysql_prlock_unlock(&lock->m_rwlock); - delete ticket; my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); DBUG_RETURN(TRUE); } +#ifdef DBUG_TRACE + const char *ticket_msg= dbug_print_mdl(ticket); +#endif + #ifdef WITH_WSREP if (WSREP(get_thd())) { @@ -2378,6 +2368,7 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) } #endif /* WITH_WSREP */ + lock= ticket->m_lock; lock->m_waiting.add_ticket(ticket); /* From ea21571f16013f2e222e06d2b1025fe74baa8dd8 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sat, 3 May 2025 02:10:01 +0400 Subject: [PATCH 10/15] 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. --- sql/mdl.cc | 12 +++--------- sql/table.cc | 4 ++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 2ab15712a2982..3db268333af87 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -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); diff --git a/sql/table.cc b/sql/table.cc index c7623e141f76c..a0c5e9972d928 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -5831,11 +5831,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); @@ -5852,6 +5851,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, From 6778a5a25815bbeeee3ae080b2b45a3ba7d3cb91 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sun, 4 May 2025 00:02:00 +0400 Subject: [PATCH 11/15] MDL_lock encapsulation: try_acquire_lock_impl() Avoid accessing MDL_lock members from MDL_context::acquire_lock() MDL_context::try_acquire_lock_impl() and MDL_map::find_or_insert(). This is done by implementing MDL_map::try_acquire_lock() and MDL_lock::try_acquire_lock(). With this patch MDL_lock members are not accessed outside of the class. 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 | 184 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 111 insertions(+), 73 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 3db268333af87..48e529c89c2e5 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -162,6 +162,8 @@ void MDL_key::init_psi_keys() static bool mdl_initialized= 0; +enum tal_status { TAL_ERROR, TAL_ACQUIRED, TAL_WAIT, TAL_NOWAIT }; + /** A collection of all MDL locks. A singleton, @@ -173,7 +175,8 @@ class MDL_map public: void init(); void destroy(); - MDL_lock *find_or_insert(LF_PINS *pins, const MDL_key *key); + enum tal_status try_acquire_lock(LF_PINS *pins, MDL_key *key, + MDL_ticket *ticket, bool wait); unsigned long get_lock_owner(LF_PINS *pins, const MDL_key *key); void remove(LF_PINS *pins, const MDL_key *key) { lf_hash_delete(&m_locks, pins, key->ptr(), key->length()); } @@ -793,6 +796,84 @@ class MDL_lock } + /** + Attempt to acquire lock + + Before performing any action we must ensure that this lock + is not being deleted by concurrent thread. We're safe when + m_strategy != nullptr. + + When there're no conflicting granted/waiting locks, lock request + can be served immediately. In this case the only action that has + to be performed is adding ticket to m_granted list. + + If it is impossible to serve lock request immediately and the caller + doesn't intend to wait for this lock, no action has to be performed. + This is the case for MDL_context::acquire_lock() with + lock_wait_timeout == 0 and MDL_context::try_acquire_lock(). + + Otherwise it is regular MDL_context::acquire_lock(). Add ticket + to m_waiting list and notify conflicting lock owners. + + Also assert that if we are trying to get an exclusive lock for a slave + running parallel replication, then we are not blocked by another + parallel slave thread that is not committed. This should never happen as + the parallel replication scheduler should never schedule a DDL while + DML's are still running. + + ticket->m_lock has to be updated to point to this lock either before + critical section or inside critical section. Before other threads can + find it via m_granted or m_waiting lists. + + ticket->m_lock is valid only when this method returns either TAL_ACQUIRED + or TAL_WAIT. In other cases caller doesn't hold references to this lock, + which means it can be detroyed. Caller is expected to dispose ticket + immediately anyway. + + @retval TAL_ACQUIRED Acquired + @retval TAL_WAIT Not acquired, must be waited + @retval TAL_NOWAIT Not acquired, cannot be waited + @retval TAL_ERROR Lock is being deleted, retry + */ + enum tal_status try_acquire_lock(MDL_ticket *ticket, bool wait) + { + MDL_context *mdl_context= ticket->get_ctx(); + // TAL_ACQUIRED to make less work on the hot path + enum tal_status res= TAL_ACQUIRED; + ticket->m_lock= this; + mysql_prlock_wrlock(&m_rwlock); + if (likely(m_strategy)) + { + if (can_grant_lock(ticket->get_type(), mdl_context, false)) + m_granted.add_ticket(ticket); + else + { + DBUG_ASSERT(!is_empty()); + if (wait) + { + res= TAL_WAIT; + m_waiting.add_ticket(ticket); + + if (needs_notification(ticket)) + notify_conflicting_locks(mdl_context, false); + + DBUG_SLOW_ASSERT((ticket->get_type() != MDL_INTENTION_EXCLUSIVE && + ticket->get_type() != MDL_EXCLUSIVE) || + !(mdl_context->get_thd()->rgi_slave && + mdl_context->get_thd()->rgi_slave->is_parallel_exec && + check_if_conflicting_replication_locks(mdl_context))); + } + else + res= TAL_NOWAIT; + } + } + else + res= TAL_ERROR; + mysql_prlock_unlock(&m_rwlock); + return res; + } + + void release(LF_PINS *pins, MDL_ticket *ticket) { remove_ticket(pins, &MDL_lock::m_granted, ticket); @@ -947,45 +1028,44 @@ void MDL_map::destroy() Find MDL_lock object corresponding to the key, create it if it does not exist. - @retval non-NULL - Success. MDL_lock instance for the key with - locked MDL_lock::m_rwlock. - @retval NULL - Failure (OOM). + @retval TAL_ACQUIRED Acquired + @retval TAL_WAIT Not acquired, must be waited + @retval TAL_NOWAIT Not acquired, cannot be waited + @retval TAL_ERROR Failure (OOM) */ -MDL_lock* MDL_map::find_or_insert(LF_PINS *pins, const MDL_key *mdl_key) +enum tal_status MDL_map::try_acquire_lock(LF_PINS *pins, MDL_key *mdl_key, + MDL_ticket *ticket, + bool wait) { MDL_lock *lock; if (mdl_key->mdl_namespace() == MDL_key::BACKUP) { /* - Return pointer to pre-allocated MDL_lock instance. Such an optimization - allows to save one hash lookup for any statement changing data. + Use m_backup_lock shortcut. Such optimization allows to save + one hash lookup for any statement changing data. It works since this namespace contains only one element so keys for them look like '\0\0'. */ DBUG_ASSERT(mdl_key->length() == 3); - mysql_prlock_wrlock(&m_backup_lock->m_rwlock); - return m_backup_lock; + DBUG_ASSERT(m_backup_lock->m_strategy); + return m_backup_lock->try_acquire_lock(ticket, wait); } retry: while (!(lock= (MDL_lock*) lf_hash_search(&m_locks, pins, mdl_key->ptr(), mdl_key->length()))) if (lf_hash_insert(&m_locks, pins, (uchar*) mdl_key) == -1) - return NULL; + return TAL_ERROR; - mysql_prlock_wrlock(&lock->m_rwlock); - if (unlikely(!lock->m_strategy)) - { - mysql_prlock_unlock(&lock->m_rwlock); - lf_hash_search_unpin(pins); - goto retry; - } + enum tal_status res= lock->try_acquire_lock(ticket, wait); lf_hash_search_unpin(pins); + if (res == TAL_ERROR) + goto retry; - return lock; + return res; } @@ -2095,8 +2175,6 @@ bool MDL_context::try_acquire_lock_impl(MDL_request *mdl_request, MDL_ticket **out_ticket) { - MDL_lock *lock; - MDL_key *key= &mdl_request->key; MDL_ticket *ticket; enum_mdl_duration found_duration; @@ -2150,42 +2228,25 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request, 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))) - { - delete ticket; - return TRUE; - } - - ticket->m_lock= lock; - - if (lock->can_grant_lock(mdl_request->type, this, false)) - { - lock->m_granted.add_ticket(ticket); - mysql_prlock_unlock(&lock->m_rwlock); - + switch (mdl_locks.try_acquire_lock(m_pins, &mdl_request->key, ticket, + out_ticket != nullptr)) { + case TAL_ACQUIRED: m_tickets[mdl_request->duration].push_front(ticket); - mdl_request->ticket= ticket; - mysql_mdl_set_status(ticket->m_psi, MDL_ticket::GRANTED); - } - else if (out_ticket) + break; + case TAL_WAIT: + DBUG_ASSERT(out_ticket); *out_ticket= ticket; - else - { - /* - Our attempt to acquire lock without waiting has failed. - Let us release resources which were acquired in the process. - We can't get here if we allocated a new lock object so there - is no need to release it. - */ - DBUG_ASSERT(!ticket->m_lock->is_empty()); - DBUG_PRINT("mdl", ("Nowait: %s", dbug_print_mdl(ticket))); - mysql_prlock_unlock(&ticket->m_lock->m_rwlock); + break; + case TAL_NOWAIT: + DBUG_PRINT("mdl", ("Nowait: m_lock unavailable>")); delete ticket; + break; + default: + delete ticket; + return TRUE; } - return FALSE; } @@ -2370,29 +2431,6 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout) #endif /* WITH_WSREP */ lock= ticket->m_lock; - lock->m_waiting.add_ticket(ticket); - - /* - Don't break conflicting locks if timeout is 0 as 0 is used - to check if there is any conflicting locks... - */ - if (lock->needs_notification(ticket) && lock_wait_timeout) - lock->notify_conflicting_locks(this, false); - - /* - Ensure that if we are trying to get an exclusive lock for a slave - running parallel replication, then we are not blocked by another - parallel slave thread that is not committed. This should never happen as - the parallel replication scheduler should never schedule a DDL while - DML's are still running. - */ - DBUG_SLOW_ASSERT((mdl_request->type != MDL_INTENTION_EXCLUSIVE && - mdl_request->type != MDL_EXCLUSIVE) || - !(get_thd()->rgi_slave && - get_thd()->rgi_slave->is_parallel_exec && - lock->check_if_conflicting_replication_locks(this))); - - mysql_prlock_unlock(&lock->m_rwlock); #ifdef HAVE_PSI_INTERFACE PSI_metadata_locker_state state __attribute__((unused)); From 20549f09e743dd1059cdb8a4786e08d85f8d4011 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sun, 4 May 2025 01:59:52 +0400 Subject: [PATCH 12/15] MDL_lock encapsulation: MDL_lock::get_key() Avoid accessing MDL_lock::key from outside of MDL_lock class directly, use MDL_lock::get_key() instead. 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. --- .../metadata_lock_info/metadata_lock_info.cc | 2 +- sql/item_func.cc | 2 +- sql/mdl.cc | 48 +++++++++---------- sql/mdl.h | 2 +- sql/vector_mhnsw.cc | 2 +- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/plugin/metadata_lock_info/metadata_lock_info.cc b/plugin/metadata_lock_info/metadata_lock_info.cc index 7fe4b72b77f20..0e19c9df174a6 100644 --- a/plugin/metadata_lock_info/metadata_lock_info.cc +++ b/plugin/metadata_lock_info/metadata_lock_info.cc @@ -75,7 +75,7 @@ int i_s_metadata_lock_info_fill_row( TABLE *table = param->table; DBUG_ENTER("i_s_metadata_lock_info_fill_row"); MDL_context *mdl_ctx = mdl_ticket->get_ctx(); - MDL_key *mdl_key = mdl_ticket->get_key(); + const MDL_key *mdl_key = mdl_ticket->get_key(); MDL_key::enum_mdl_namespace mdl_namespace = mdl_key->mdl_namespace(); if (!granted) DBUG_RETURN(0); diff --git a/sql/item_func.cc b/sql/item_func.cc index 7f7a12c854094..3edc33fd4c183 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -4165,7 +4165,7 @@ class User_level_lock const uchar *ull_get_key(const void *ptr, size_t *length, my_bool) { User_level_lock *ull = (User_level_lock*) ptr; - MDL_key *key = ull->lock->get_key(); + const MDL_key *key = ull->lock->get_key(); *length= key->length(); return key->ptr(); } diff --git a/sql/mdl.cc b/sql/mdl.cc index 48e529c89c2e5..a4d2186649106 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -257,7 +257,7 @@ class Deadlock_detection_visitor: public MDL_wait_for_graph_visitor const char *dbug_print_mdl(MDL_ticket *mdl_ticket) { thread_local char buffer[256]; - MDL_key *mdl_key= mdl_ticket->get_key(); + const MDL_key *mdl_key= mdl_ticket->get_key(); my_snprintf(buffer, sizeof(buffer) - 1, "%.*s/%.*s (%s)", (int) mdl_key->db_name_length(), mdl_key->db_name(), (int) mdl_key->name_length(), mdl_key->name(), @@ -688,6 +688,14 @@ class MDL_lock lock->m_strategy= &m_object_lock_strategy; } + static const uchar *mdl_locks_key(const void *record, size_t *length, + my_bool) + { + const MDL_lock *lock= static_cast(record); + *length= lock->key.length(); + return lock->key.ptr(); + } + /** Return thread id of the thread to which the first ticket was @@ -884,6 +892,9 @@ class MDL_lock { remove_ticket(pins, &MDL_lock::m_waiting, ticket); } + const MDL_key *get_key() const { return &key; } + + const MDL_lock_strategy *m_strategy; private: static const MDL_backup_lock m_backup_lock_strategy; @@ -900,18 +911,6 @@ const MDL_lock::MDL_object_lock MDL_lock::m_object_lock_strategy; static MDL_map mdl_locks; -extern "C" -{ -static const uchar *mdl_locks_key(const void *record, size_t *length, - my_bool) -{ - const MDL_lock *lock= static_cast(record); - *length= lock->key.length(); - return lock->key.ptr(); -} -} /* extern "C" */ - - /** Initialize the metadata locking subsystem. @@ -1002,7 +1001,7 @@ void MDL_map::init() m_backup_lock= new (std::nothrow) MDL_lock(&backup_lock_key); lf_hash_init(&m_locks, sizeof(MDL_lock), LF_HASH_UNIQUE, 0, 0, - mdl_locks_key, &my_charset_bin); + MDL_lock::mdl_locks_key, &my_charset_bin); m_locks.alloc.constructor= MDL_lock::lf_alloc_constructor; m_locks.alloc.destructor= MDL_lock::lf_alloc_destructor; m_locks.initializer= MDL_lock::lf_hash_initializer; @@ -1230,7 +1229,7 @@ MDL_ticket::~MDL_ticket() uint MDL_ticket::get_deadlock_weight() const { - if (m_lock->key.mdl_namespace() == MDL_key::BACKUP) + if (get_key()->mdl_namespace() == MDL_key::BACKUP) { if (m_type == MDL_BACKUP_FTWRL1) return DEADLOCK_WEIGHT_FTWRL1; @@ -2106,7 +2105,7 @@ MDL_context::find_ticket(MDL_request *mdl_request, while ((ticket= it++)) { - if (mdl_request->key.is_equal(&ticket->m_lock->key) && + if (mdl_request->key.is_equal(ticket->get_key()) && ticket->has_stronger_or_equal_type(mdl_request->type)) { DBUG_PRINT("info", ("Adding mdl lock %s to %s", @@ -2690,7 +2689,7 @@ MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket, mdl_ticket->get_key()->mdl_namespace() != MDL_key::BACKUP) DBUG_RETURN(FALSE); - MDL_REQUEST_INIT_BY_KEY(&mdl_xlock_request, &mdl_ticket->m_lock->key, + MDL_REQUEST_INIT_BY_KEY(&mdl_xlock_request, mdl_ticket->get_key(), new_type, MDL_TRANSACTION); if (acquire_lock(&mdl_xlock_request, lock_wait_timeout)) @@ -2966,7 +2965,8 @@ void MDL_context::release_lock(enum_mdl_duration duration, MDL_ticket *ticket) MDL_lock *lock= ticket->m_lock; DBUG_ENTER("MDL_context::release_lock"); DBUG_PRINT("enter", ("db: '%s' name: '%s'", - lock->key.db_name(), lock->key.name())); + ticket->get_key()->db_name(), + ticket->get_key()->name())); DBUG_ASSERT(this == ticket->get_ctx()); DBUG_PRINT("mdl", ("Released: %s", dbug_print_mdl(ticket))); @@ -3156,9 +3156,9 @@ bool MDL_ticket::has_pending_conflicting_lock() const } /** Return a key identifying this lock. */ -MDL_key *MDL_ticket::get_key() const +const MDL_key *MDL_ticket::get_key() const { - return &m_lock->key; + return m_lock->get_key(); } /** @@ -3389,12 +3389,12 @@ void MDL_ticket::wsrep_report(bool debug) const { if (!debug) return; - const PSI_stage_info *psi_stage= m_lock->key.get_wait_state_name(); + const PSI_stage_info *psi_stage= get_key()->get_wait_state_name(); WSREP_DEBUG("MDL ticket: type: %s space: %s db: %s name: %s (%s)", get_type_name()->str, - wsrep_get_mdl_namespace_name(m_lock->key.mdl_namespace()), - m_lock->key.db_name(), - m_lock->key.name(), + wsrep_get_mdl_namespace_name(get_key()->mdl_namespace()), + get_key()->db_name(), + get_key()->name(), psi_stage->m_name); } #endif /* WITH_WSREP */ diff --git a/sql/mdl.h b/sql/mdl.h index 92fae6942dd79..5af737f7e8bb1 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -730,7 +730,7 @@ class MDL_ticket : public MDL_wait_for_subgraph, public ilist_node<> const LEX_STRING *get_type_name() const; const LEX_STRING *get_type_name(enum_mdl_type type) const; MDL_lock *get_lock() const { return m_lock; } - MDL_key *get_key() const; + const MDL_key *get_key() const; void downgrade_lock(enum_mdl_type type); bool has_stronger_or_equal_type(enum_mdl_type type) const; diff --git a/sql/vector_mhnsw.cc b/sql/vector_mhnsw.cc index 99c96d3d94917..81e2a71592ef8 100644 --- a/sql/vector_mhnsw.cc +++ b/sql/vector_mhnsw.cc @@ -670,7 +670,7 @@ int MHNSW_Trx::do_commit(THD *thd, bool) trx_next= trx->next; if (trx->table_id) { - MDL_key *key= trx->table_id->get_key(); + const MDL_key *key= trx->table_id->get_key(); LEX_CSTRING db= {key->db_name(), key->db_name_length()}, tbl= {key->name(), key->name_length()}; TABLE_LIST tl; From 4194683bc91d2406f4f9eec6e0705023393fd754 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sun, 4 May 2025 01:32:04 +0400 Subject: [PATCH 13/15] MDL_lock encapsulation: final All MDL_lock objects and types are private now. Along with a set of methods which are not used outside. 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 | 61 +++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index a4d2186649106..0aa80541f4cc6 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -388,7 +388,6 @@ Deadlock_detection_visitor::opt_change_victim_to(MDL_context *new_victim) class MDL_lock { -public: typedef mdl_bitmap_t bitmap_t; class Ticket_list @@ -552,9 +551,10 @@ class MDL_lock static const bitmap_t m_waiting_incompatible[MDL_BACKUP_END]; }; -public: + /** The key of the object (data) being protected. */ MDL_key key; + /** Read-write lock protecting this lock context. @@ -590,18 +590,28 @@ class MDL_lock */ mutable mysql_prlock_t m_rwlock; + /** List of granted tickets for this lock. */ + Ticket_list m_granted; + /** Tickets for contexts waiting to acquire a lock. */ + Ticket_list m_waiting; + + /** + Number of times high priority lock requests have been granted while + low priority lock requests were waiting. + */ + ulong m_hog_lock_count; + + const MDL_lock_strategy *m_strategy; + + static const MDL_backup_lock m_backup_lock_strategy; + static const MDL_scoped_lock m_scoped_lock_strategy; + static const MDL_object_lock m_object_lock_strategy; + bool is_empty() const { return (m_granted.is_empty() && m_waiting.is_empty()); } - const bitmap_t *incompatible_granted_types_bitmap() const - { return m_strategy->incompatible_granted_types_bitmap(); } - const bitmap_t *incompatible_waiting_types_bitmap() const - { return m_strategy->incompatible_waiting_types_bitmap(); } - - bool has_pending_conflicting_lock(enum_mdl_type type); - bool can_grant_lock(enum_mdl_type type, MDL_context *requstor_ctx, bool ignore_lock_priority) const; @@ -610,9 +620,6 @@ class MDL_lock void remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*queue, MDL_ticket *ticket); - bool visit_subgraph(MDL_ticket *waiting_ticket, - MDL_wait_for_graph_visitor *gvisitor); - bool needs_notification(const MDL_ticket *ticket) const { return m_strategy->needs_notification(ticket); } void notify_conflicting_locks(MDL_context *ctx, bool abort_blocking) @@ -639,18 +646,16 @@ class MDL_lock bool check_if_conflicting_replication_locks(MDL_context *ctx); #endif - /** List of granted tickets for this lock. */ - Ticket_list m_granted; - /** Tickets for contexts waiting to acquire a lock. */ - Ticket_list m_waiting; +public: + const bitmap_t *incompatible_granted_types_bitmap() const + { return m_strategy->incompatible_granted_types_bitmap(); } + const bitmap_t *incompatible_waiting_types_bitmap() const + { return m_strategy->incompatible_waiting_types_bitmap(); } - /** - Number of times high priority lock requests have been granted while - low priority lock requests were waiting. - */ - ulong m_hog_lock_count; + bool has_pending_conflicting_lock(enum_mdl_type type); -public: + bool visit_subgraph(MDL_ticket *waiting_ticket, + MDL_wait_for_graph_visitor *gvisitor); MDL_lock() : m_hog_lock_count(0), @@ -893,13 +898,6 @@ class MDL_lock const MDL_key *get_key() const { return &key; } - - - const MDL_lock_strategy *m_strategy; -private: - static const MDL_backup_lock m_backup_lock_strategy; - static const MDL_scoped_lock m_scoped_lock_strategy; - static const MDL_object_lock m_object_lock_strategy; }; @@ -1049,7 +1047,6 @@ enum tal_status MDL_map::try_acquire_lock(LF_PINS *pins, MDL_key *mdl_key, for them look like '\0\0'. */ DBUG_ASSERT(mdl_key->length() == 3); - DBUG_ASSERT(m_backup_lock->m_strategy); return m_backup_lock->try_acquire_lock(ticket, wait); } @@ -2033,9 +2030,7 @@ MDL_wait_for_subgraph::~MDL_wait_for_subgraph() bool MDL_ticket::has_stronger_or_equal_type(enum_mdl_type type) const { - const MDL_lock::bitmap_t * - granted_incompat_map= m_lock->incompatible_granted_types_bitmap(); - + const auto *granted_incompat_map= m_lock->incompatible_granted_types_bitmap(); return ! (granted_incompat_map[type] & ~(granted_incompat_map[m_type])); } From 1472f8dc96bacf25c61a88551105fe346a57255a Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 7 May 2025 16:04:07 +0400 Subject: [PATCH 14/15] MDL_lock encapsulation: removed redundant methods Given that MDL_lock::m_strategy is only accessed by MDL_lock methods, there is no point in having MDL_lock::needs_notification() and MDL_lock::hog_lock_types_bitmap() getters anymore. MDL_lock::has_pending_conflicting_lock() moved to MDL_lock class. 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 | 55 +++++++++++++++++++++--------------------------------- sql/mdl.h | 6 ++++++ 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 0aa80541f4cc6..8a5b1f5f20022 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -370,11 +370,6 @@ Deadlock_detection_visitor::opt_change_victim_to(MDL_context *new_victim) } -/** - Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps - and compatibility matrices. -*/ - /** The lock context. Created internally for an acquired lock. For a given name, there exists only one MDL_lock instance, @@ -620,8 +615,6 @@ class MDL_lock void remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*queue, MDL_ticket *ticket); - bool needs_notification(const MDL_ticket *ticket) const - { return m_strategy->needs_notification(ticket); } void notify_conflicting_locks(MDL_context *ctx, bool abort_blocking) { for (const auto &conflicting_ticket : m_granted) @@ -639,9 +632,6 @@ class MDL_lock } } - bitmap_t hog_lock_types_bitmap() const - { return m_strategy->hog_lock_types_bitmap(); } - #ifndef DBUG_OFF bool check_if_conflicting_replication_locks(MDL_context *ctx); #endif @@ -652,7 +642,24 @@ class MDL_lock const bitmap_t *incompatible_waiting_types_bitmap() const { return m_strategy->incompatible_waiting_types_bitmap(); } - bool has_pending_conflicting_lock(enum_mdl_type type); + + /** + Check if we have any pending locks which conflict with existing + shared lock. + + @pre The ticket must match an acquired lock. + + @return TRUE if there is a conflicting lock request, FALSE otherwise. + */ + bool has_pending_conflicting_lock(enum_mdl_type type) + { + bool result; + mysql_prlock_rdlock(&m_rwlock); + result= (m_waiting.bitmap() & incompatible_granted_types_bitmap()[type]); + mysql_prlock_unlock(&m_rwlock); + return result; + } + bool visit_subgraph(MDL_ticket *waiting_ticket, MDL_wait_for_graph_visitor *gvisitor); @@ -800,7 +807,7 @@ class MDL_lock void notify_conflicting_locks_if_needed(MDL_ticket *ticket, bool abort_blocking) { - if (needs_notification(ticket)) + if (m_strategy->needs_notification(ticket)) { mysql_prlock_wrlock(&m_rwlock); notify_conflicting_locks(ticket->get_ctx(), abort_blocking); @@ -867,7 +874,7 @@ class MDL_lock res= TAL_WAIT; m_waiting.add_ticket(ticket); - if (needs_notification(ticket)) + if (m_strategy->needs_notification(ticket)) notify_conflicting_locks(mdl_context, false); DBUG_SLOW_ASSERT((ticket->get_type() != MDL_INTENTION_EXCLUSIVE && @@ -1449,7 +1456,7 @@ void MDL_lock::Ticket_list::remove_ticket(MDL_ticket *ticket) void MDL_lock::reschedule_waiters() { bool skip_high_priority= false; - bitmap_t hog_lock_types= hog_lock_types_bitmap(); + bitmap_t hog_lock_types= m_strategy->hog_lock_types_bitmap(); if (m_hog_lock_count >= max_write_lock_count) { @@ -1992,26 +1999,6 @@ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, } -/** - Check if we have any pending locks which conflict with existing - shared lock. - - @pre The ticket must match an acquired lock. - - @return TRUE if there is a conflicting lock request, FALSE otherwise. -*/ - -bool MDL_lock::has_pending_conflicting_lock(enum_mdl_type type) -{ - bool result; - - mysql_prlock_rdlock(&m_rwlock); - result= (m_waiting.bitmap() & incompatible_granted_types_bitmap()[type]); - mysql_prlock_unlock(&m_rwlock); - return result; -} - - MDL_wait_for_graph_visitor::~MDL_wait_for_graph_visitor() = default; diff --git a/sql/mdl.h b/sql/mdl.h index 5af737f7e8bb1..06bf86105547f 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -31,6 +31,12 @@ class MDL_lock; class MDL_ticket; typedef unsigned short mdl_bitmap_t; + + +/** + Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps + and compatibility matrices. +*/ #define MDL_BIT(A) static_cast(1U << A) From 5e49acbe65e89e57d90a8b2500d0ca31164f19f5 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Mon, 12 May 2025 00:11:31 +0400 Subject: [PATCH 15/15] MDEV-19749 - MDL scalability regression after backup locks Statements that intend to modify data have to acquire protection against ongoing backup. Prior to backup locks, protection against FTWRL was acquired in form of 2 shared metadata locks of GLOBAL (global read lock) and COMMIT namespaces. These two namespaces were separate entities, they didn't share data structures and locking primitives. And thus they were separate contention points. With backup locks, introduced by 7a9dfdd, these namespaces were combined into a single BACKUP namespace. It became a single contention point, which doubled load on BACKUP namespace data structures and locking primitives compared to GLOBAL and COMMIT namespaces. In other words system throughput has halved. MDL fast lanes solve this problem by allowing multiple contention points for single MDL_lock. Fast lane is scalable multi-instance registry for leightweight locks. Internally it is just a list of granted tickets, close counter and a mutex. Number of fast lanes (or contention points) is defined by the metadata_locks_instances system variable. Value of 1 disables fast lanes and lock requests are served by conventional MDL_lock data structures. Since fast lanes allow arbitrary number of contention points, they outperform pre-backup locks GLOBAL and COMMIT. Fast lanes are enabled only for BACKUP namespace. Support for other namespaces is to be implemented separately. Lock types are divided in 2 categories: lightweight and heavyweight. Lightweight lock types represent DML: MDL_BACKUP_DML, MDL_BACKUP_TRANS_DML, MDL_BACKUP_SYS_DML, MDL_BACKUP_DDL, MDL_BACKUP_ALTER_COPY, MDL_BACKUP_COMMIT. They are fully compatible with each other. Normally served by corresponding fast lane, which is determined by thread_id % metadata_locks_instances. Heavyweight lock types represent ongoing backup: MDL_BACKUP_START, MDL_BACKUP_FLUSH, MDL_BACKUP_WAIT_FLUSH, MDL_BACKUP_WAIT_DDL, MDL_BACKUP_WAIT_COMMIT, MDL_BACKUP_FTWRL1, MDL_BACKUP_FTWRL2, MDL_BACKUP_BLOCK_DDL. These locks are always served by conventional MDL_lock data structures. Whenever such lock is requested, fast lanes are closed and all tickets registered in fast lanes are moved to conventional MDL_lock data structures. Until such locks are released or aborted, lightweight lock requests are served by conventional MDL_lock data structures. Strictly speaking moving tickets from fast lanes to conventional MDL_lock data structures is not required. But it allows to reduce complexity and keep intact methods like: MDL_lock::visit_subgraph(), MDL_lock::notify_conflicting_locks(), MDL_lock::reschedule_waiters(), MDL_lock::can_grant_lock(). It is not even required to register tickets in fast lanes. They can be implemented basing on an atomic variable that holds two counters: granted lightweight locks and granted/waiting heavyweight locks. Similarly to MySQL solution, which roughly speaking has "single atomic fast lane". However it appears to be it won't bring any better performance, while code complexity is going to be much higher. --- mysql-test/main/create_or_replace.test | 1 + mysql-test/main/mdl.result | 8 +- mysql-test/main/mdl.test | 2 + mysql-test/main/mysqld--help.result | 6 + .../sys_vars/r/sysvars_server_embedded.result | 10 + .../r/sysvars_server_notembedded.result | 10 + sql/mdl.cc | 539 +++++++++++++++++- sql/mdl.h | 5 + sql/sys_vars.cc | 8 + 9 files changed, 584 insertions(+), 5 deletions(-) diff --git a/mysql-test/main/create_or_replace.test b/mysql-test/main/create_or_replace.test index 05af75b61b73c..736713183ef1c 100644 --- a/mysql-test/main/create_or_replace.test +++ b/mysql-test/main/create_or_replace.test @@ -239,6 +239,7 @@ select LOCK_MODE,LOCK_TYPE, TABLE_SCHEMA,TABLE_NAME from information_schema.meta --error ER_DUP_FIELDNAME create or replace table test.t1 (a int) select 1 as 'a', 2 as 'a'; show tables; +--sorted_result select LOCK_MODE,LOCK_TYPE, TABLE_SCHEMA,TABLE_NAME from information_schema.metadata_lock_info; --sorted_result select LOCK_MODE,LOCK_TYPE, TABLE_SCHEMA,TABLE_NAME from information_schema.metadata_lock_info; diff --git a/mysql-test/main/mdl.result b/mysql-test/main/mdl.result index dd3d923936682..284fd78901240 100644 --- a/mysql-test/main/mdl.result +++ b/mysql-test/main/mdl.result @@ -28,9 +28,9 @@ WHERE TABLE_NAME NOT LIKE 'innodb_%_stats'; LOCK_MODE LOCK_TYPE TABLE_SCHEMA TABLE_NAME MDL_BACKUP_DDL Backup lock MDL_BACKUP_DML Backup lock -MDL_SHARED_WRITE Table metadata lock test t1 -MDL_SHARED_NO_READ_WRITE Table metadata lock test t3 MDL_INTENTION_EXCLUSIVE Schema metadata lock test +MDL_SHARED_NO_READ_WRITE Table metadata lock test t3 +MDL_SHARED_WRITE Table metadata lock test t1 UNLOCK TABLES; LOCK TABLES t3 WRITE, t1 WRITE CONCURRENT; SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.metadata_lock_info @@ -38,9 +38,9 @@ WHERE TABLE_NAME NOT LIKE 'innodb_%_stats'; LOCK_MODE LOCK_TYPE TABLE_SCHEMA TABLE_NAME MDL_BACKUP_DDL Backup lock MDL_BACKUP_DML Backup lock -MDL_SHARED_WRITE Table metadata lock test t1 -MDL_SHARED_NO_READ_WRITE Table metadata lock test t3 MDL_INTENTION_EXCLUSIVE Schema metadata lock test +MDL_SHARED_NO_READ_WRITE Table metadata lock test t3 +MDL_SHARED_WRITE Table metadata lock test t1 UNLOCK TABLES; LOCK TABLES t1 WRITE, mysql.user WRITE; SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.metadata_lock_info diff --git a/mysql-test/main/mdl.test b/mysql-test/main/mdl.test index 6b0c769014e0a..1d5402c6ad883 100644 --- a/mysql-test/main/mdl.test +++ b/mysql-test/main/mdl.test @@ -20,10 +20,12 @@ SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.me WHERE TABLE_NAME NOT LIKE 'innodb_%_stats'; UNLOCK TABLES; LOCK TABLES t1 WRITE CONCURRENT, t3 WRITE; +--sorted_result SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.metadata_lock_info WHERE TABLE_NAME NOT LIKE 'innodb_%_stats'; UNLOCK TABLES; LOCK TABLES t3 WRITE, t1 WRITE CONCURRENT; +--sorted_result SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.metadata_lock_info WHERE TABLE_NAME NOT LIKE 'innodb_%_stats'; UNLOCK TABLES; diff --git a/mysql-test/main/mysqld--help.result b/mysql-test/main/mysqld--help.result index 8b4e9b242c0f4..eba5aa2dcd383 100644 --- a/mysql-test/main/mysqld--help.result +++ b/mysql-test/main/mysqld--help.result @@ -716,6 +716,11 @@ The following specify which files/extra groups are read (specified before remain Unused. Deprecated, will be removed in a future release. --metadata-locks-hash-instances=# Unused. Deprecated, will be removed in a future release. + --metadata-locks-instances=# + Number of fast lanes to create for metadata locks. Can be + used to improve DML scalability by eliminating + MDL_lock::rwlock load. Use 1 to disable MDL fast lanes. + Supported MDL namespaces: BACKUP --mhnsw-default-distance=name Distance function to build the vector index for. One of: euclidean, cosine @@ -1847,6 +1852,7 @@ max-write-lock-count 18446744073709551615 memlock FALSE metadata-locks-cache-size 1024 metadata-locks-hash-instances 8 +metadata-locks-instances 8 mhnsw-default-distance euclidean mhnsw-default-m 6 mhnsw-ef-search 20 diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result index a8c5c335169cd..f02de8fd7bd82 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result @@ -2202,6 +2202,16 @@ NUMERIC_BLOCK_SIZE 1 ENUM_VALUE_LIST NULL READ_ONLY YES COMMAND_LINE_ARGUMENT REQUIRED +VARIABLE_NAME METADATA_LOCKS_INSTANCES +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE INT UNSIGNED +VARIABLE_COMMENT Number of fast lanes to create for metadata locks. Can be used to improve DML scalability by eliminating MDL_lock::rwlock load. Use 1 to disable MDL fast lanes. Supported MDL namespaces: BACKUP +NUMERIC_MIN_VALUE 1 +NUMERIC_MAX_VALUE 256 +NUMERIC_BLOCK_SIZE 1 +ENUM_VALUE_LIST NULL +READ_ONLY YES +COMMAND_LINE_ARGUMENT REQUIRED VARIABLE_NAME MHNSW_DEFAULT_DISTANCE VARIABLE_SCOPE SESSION VARIABLE_TYPE ENUM diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result index e38449d063ba2..bbfc072f1d31e 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result @@ -2422,6 +2422,16 @@ NUMERIC_BLOCK_SIZE 1 ENUM_VALUE_LIST NULL READ_ONLY YES COMMAND_LINE_ARGUMENT REQUIRED +VARIABLE_NAME METADATA_LOCKS_INSTANCES +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE INT UNSIGNED +VARIABLE_COMMENT Number of fast lanes to create for metadata locks. Can be used to improve DML scalability by eliminating MDL_lock::rwlock load. Use 1 to disable MDL fast lanes. Supported MDL namespaces: BACKUP +NUMERIC_MIN_VALUE 1 +NUMERIC_MAX_VALUE 256 +NUMERIC_BLOCK_SIZE 1 +ENUM_VALUE_LIST NULL +READ_ONLY YES +COMMAND_LINE_ARGUMENT REQUIRED VARIABLE_NAME MHNSW_DEFAULT_DISTANCE VARIABLE_SCOPE SESSION VARIABLE_TYPE ENUM diff --git a/sql/mdl.cc b/sql/mdl.cc index 8a5b1f5f20022..15f03929b1e66 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -21,6 +21,7 @@ #include "sql_array.h" #include "rpl_rli.h" #include +#include "aligned.h" #include "unireg.h" #include #include @@ -37,10 +38,12 @@ static PSI_memory_key key_memory_MDL_context_acquire_locks; #ifdef HAVE_PSI_INTERFACE +static PSI_mutex_key key_MDL_lock_fast_lane_mutex; static PSI_mutex_key key_MDL_wait_LOCK_wait_status; static PSI_mutex_info all_mdl_mutexes[]= { + { &key_MDL_lock_fast_lane_mutex, "MDL_lock::Fast_road::Lane::m_mutex", 0 }, { &key_MDL_wait_LOCK_wait_status, "MDL_wait::LOCK_wait_status", 0} }; @@ -161,6 +164,7 @@ void MDL_key::init_psi_keys() #endif static bool mdl_initialized= 0; +uint mdl_instances; enum tal_status { TAL_ERROR, TAL_ACQUIRED, TAL_WAIT, TAL_NOWAIT }; @@ -547,6 +551,405 @@ class MDL_lock }; + /** + Scalable handler for "lightweight" lock types. + */ + class Fast_road + { + class Lane + { + alignas(CPU_LEVEL1_DCACHE_LINESIZE) mutable mysql_mutex_t m_mutex; + ilist m_list; + /** + Counts number of granted/pending non-fast lane locks. + + Having it as a member of Lane allows simpler and cleaner + implementation. Otherwise it is a property of Fast_road. + */ + uint32_t m_close_count; + + bool is_open() const { return m_close_count == 0; } + + + /** + upgrade/downgrade/release helper. + + ticket->m_fast_lane has to be double checked under fast lane mutex + protection. Description in a comment to MDL_lock::release(). + + @retval true Requested action performed + @retval false Lane closed, try conventional action + */ + template + bool ticket_action(MDL_ticket *ticket, Functor action) + { + mysql_mutex_lock(&m_mutex); + DBUG_ASSERT(is_open() || m_list.empty()); + Lane *lane= static_cast( + ticket->m_fast_lane.load(std::memory_order_relaxed)); + if (likely(lane)) + { + DBUG_ASSERT(is_open()); + DBUG_ASSERT(lane == this); + action(); + } + mysql_mutex_unlock(&m_mutex); + return lane != nullptr; + } + + public: + Lane() noexcept: m_close_count(0) + { + mysql_mutex_init(key_MDL_lock_fast_lane_mutex, &m_mutex, + MY_MUTEX_INIT_FAST); + } + + + ~Lane() + { + DBUG_ASSERT(m_close_count <= 1); + DBUG_ASSERT(m_list.empty()); + mysql_mutex_destroy(&m_mutex); + } + + + static void *operator new[](size_t size, const std::nothrow_t) + { + return aligned_malloc(size, CPU_LEVEL1_DCACHE_LINESIZE); + } + + + static void operator delete[](void *ptr) { aligned_free(ptr); } + + + /** + Registers ticket in fast lane. + + ticket->m_fast_lane has to be updated to point to this lane + either before critical section or inside critical section. + Before other threads can find it via m_list. + + In most cases this method is called with open fast lane, + when there're no heavyweight locks in this MDL_lock. There is + no point in attempting to avoid mutex lock for closed lanes + by pre-checking lane_open(). + + @retval true Lock granted + @retval false Lane closed, try conventional lock + */ + bool try_acquire_lock(MDL_ticket *ticket) + { + DBUG_ASSERT(!ticket->m_fast_lane.load(std::memory_order_relaxed)); + mysql_mutex_lock(&m_mutex); + bool res= is_open(); + DBUG_ASSERT(res || m_list.empty()); + if (likely(res)) + { + m_list.push_back(*ticket); + ticket->m_fast_lane.store(this, std::memory_order_relaxed); + } + mysql_mutex_unlock(&m_mutex); + return res; + } + + + /** + Releases previously acquired lock. + + @retval true Lock released + @retval false Lane closed, try conventional unlock + */ + bool release(MDL_ticket *ticket) + { + return ticket_action(ticket, + [this, ticket]() { m_list.remove(*ticket); }); + } + + + /** + Updates ticket type. + + Used by upgrade/downgrade. + + @retval true Success + @retval false Lane closed, try conventional upgrade/downgrade + */ + bool change_ticket_type(MDL_ticket *ticket, enum_mdl_type type) + { + return ticket_action(ticket, + [ticket, type]() { ticket->m_type= type; }); + } + + + /** + Closes fast lane, moves tickets to MDL_lock::m_granted. + + Closed fast lane means the following methods have to retry their + action using conventional methods: + MDL_lock::try_acquire_lock() + MDL_lock::release() + MDL_lock::upgrade() + MDL_lock::downgrade() + + Lane can be closed multiple times. + */ + void close() + { + mysql_mutex_lock(&m_mutex); + DBUG_ASSERT(is_open() || m_list.empty()); + m_close_count++; + while (!m_list.empty()) + { + MDL_ticket *ticket= &m_list.front(); + m_list.pop_front(); + ticket->get_lock()->m_granted.add_ticket(ticket); + DBUG_ASSERT(ticket->m_fast_lane.load(std::memory_order_relaxed) == + this); + ticket->m_fast_lane.store(nullptr, std::memory_order_relaxed); + } + mysql_mutex_unlock(&m_mutex); + } + + + /** + Reopens fast lane. + + Fast lane can be used again once all closers are gone. + */ + void reopen() + { + mysql_mutex_lock(&m_mutex); + DBUG_ASSERT(m_close_count); + DBUG_ASSERT(m_list.empty()); + m_close_count--; + mysql_mutex_unlock(&m_mutex); + } + + + /** + Iterates registered tickets. + + @retval true callback returned true, abort iteration + @retval false success + */ + bool iterate(mdl_iterator_callback callback, void *argument) const + { + bool res= false; + mysql_mutex_lock(&m_mutex); + DBUG_ASSERT(is_open() || m_list.empty()); + for (MDL_ticket &ticket : m_list) + { + if (callback(&ticket, argument, true)) + { + res= true; + break; + } + } + mysql_mutex_unlock(&m_mutex); + return res; + } + + + /** Checks if lane is open, used by assertions. */ + bool is_closed() const + { + mysql_mutex_lock(&m_mutex); + bool res= !is_open(); + mysql_mutex_unlock(&m_mutex); + return res; + } + + + /** Checks if lane is empty, used by assertions. */ + bool is_empty() const + { + mysql_mutex_lock(&m_mutex); + bool res= m_list.empty(); + mysql_mutex_unlock(&m_mutex); + return res; + } + }; + + + Lane *m_fast_lane; + mdl_bitmap_t m_supported_types; + + + /** + Checks if provided lock type can be served by fast lanes. + + Fast lane lock types must be fully compatible between each other. + */ + bool supported_type(enum_mdl_type type) const + { + return MDL_BIT(type) & m_supported_types; + } + + + /** + Checks if ticket is registered in fast lane and performs action(). + + Another thread can be closing fast lanes concurrently and + resetting ticket->m_fast_lane. To handle such scenario properly + action() has to be double checked ticket->m_fast_lane under fast + lane mutex protection. Accessing ticket->m_fast_lane in this + method is the sole reason it is declared atomic. + + @retval true Success + @retval false Failure, try conventional action + */ + template + bool lane_action(MDL_ticket *ticket, Functor action) const + { + if (Lane *lane= static_cast( + ticket->m_fast_lane.load(std::memory_order_relaxed))) + { + DBUG_ASSERT(is_enabled()); + DBUG_ASSERT(supported_type(ticket->get_type())); + if (action(lane)) + { + DBUG_ASSERT(supported_type(ticket->get_type())); + return true; + } + } + return false; + } + + + /** + Iterates available lanes and performs action() for each lane. + + @retval true Iteration was aborted by action() + @retval false action() was called for all available lanes successfully + */ + template + bool all_lanes_action(Functor action) const + { + if (is_enabled()) + { + for (uint i= 0; i < mdl_instances; i++) + { + if (action(&m_fast_lane[i])) + return true; + } + } + return false; + } + + public: + Fast_road(): m_fast_lane(nullptr), m_supported_types(0) {} + ~Fast_road() { delete [] m_fast_lane; } + + + /** + Enables fast lanes. + + Once enabled, supported_types of lock requests can be served via + fast lanes. + */ + void enable(mdl_bitmap_t supported_types) + { + m_fast_lane= mdl_instances > 1 ? new (std::nothrow) Lane[mdl_instances] + : nullptr; + if (m_fast_lane) + m_supported_types= supported_types; + } + + + bool is_enabled() const { return m_fast_lane; } + + + /** + Attempts to acquire lock. + + @retval true Lock acquired + @retval false request cannot be served by fast lanes, + try conventional acquire_lock + */ + bool try_acquire_lock(MDL_ticket *ticket) const + { + if (is_enabled() && supported_type(ticket->get_type())) + { + DBUG_ASSERT(mdl_instances > 1); + uint lane= ticket->get_ctx()->get_thd()->thread_id % mdl_instances; + return m_fast_lane[lane].try_acquire_lock(ticket); + } + return false; + } + + + /** + Attempts to release previously acquired lock. + + @retval true Lock released + @retval false ticket is not registered in fast lanes, + try conventional unlock + */ + bool try_release(MDL_ticket *ticket) const + { + return lane_action( + ticket, [ticket](Lane *lane) { return lane->release(ticket); }); + } + + + /** + Attempts to upgrade or downgrade lock. + + @retval true Lock upgraded/downgraded + @retval false request cannot be served by fast lanes, + try conventional acquire_lock + */ + bool try_change_ticket_type(MDL_ticket *ticket, enum_mdl_type type) const + { + DBUG_ASSERT(supported_type(ticket->get_type()) || is_closed()); + return lane_action(ticket, + [ticket, type](Lane *lane) + { return lane->change_ticket_type(ticket, type); }); + } + + + void close(enum_mdl_type type) const + { + if (!supported_type(type)) + all_lanes_action([](Lane *lane) { lane->close(); return false; }); + } + + + void reopen(enum_mdl_type type) const + { + if (!supported_type(type)) + all_lanes_action([](Lane *lane) { lane->reopen(); return false; }); + } + + + /** + Iterates registered tickets. + + @retval true callback returned true, abort iteration + @retval false success + */ + bool iterate(mdl_iterator_callback callback, void *argument) const + { + return all_lanes_action([callback, argument](Lane *lane) + { return lane->iterate(callback, argument); }); + } + + + /** Checks if fast lanes are closed, used by assertions. */ + bool is_closed() const + { + return !all_lanes_action([](Lane *lane) { return !lane->is_closed(); }); + } + + + /** Checks if fast lanes are empty, used by assertions. */ + bool is_empty() const + { + return !all_lanes_action([](Lane *lane) { return !lane->is_empty(); }); + } + }; + + /** The key of the object (data) being protected. */ MDL_key key; @@ -596,6 +999,17 @@ class MDL_lock */ ulong m_hog_lock_count; + Fast_road m_fast_road; + + /** + Locking strategy, e.g. backup/scoped/object. + + Initialized to appropriate strategy early, before lock becomes + available via MDL_map. Reset to nullptr before lock is removed + from MDL_map. Such value indicates that MDL_lock is being removed + by a concurrent thread and the caller (specifically + MDL_map::try_acquire_lock()) must retry. + */ const MDL_lock_strategy *m_strategy; static const MDL_backup_lock m_backup_lock_strategy; @@ -617,6 +1031,7 @@ class MDL_lock void notify_conflicting_locks(MDL_context *ctx, bool abort_blocking) { + DBUG_ASSERT(m_fast_road.is_closed()); for (const auto &conflicting_ticket : m_granted) { if (conflicting_ticket.get_ctx() != ctx && @@ -654,6 +1069,8 @@ class MDL_lock bool has_pending_conflicting_lock(enum_mdl_type type) { bool result; + DBUG_ASSERT(key.mdl_namespace() == MDL_key::TABLE); + DBUG_ASSERT(!m_fast_road.is_enabled()); mysql_prlock_rdlock(&m_rwlock); result= (m_waiting.bitmap() & incompatible_granted_types_bitmap()[type]); mysql_prlock_unlock(&m_rwlock); @@ -676,6 +1093,12 @@ class MDL_lock { DBUG_ASSERT(key_arg->mdl_namespace() == MDL_key::BACKUP); mysql_prlock_init(key_MDL_lock_rwlock, &m_rwlock); + m_fast_road.enable(MDL_BIT(MDL_BACKUP_DML) | + MDL_BIT(MDL_BACKUP_TRANS_DML) | + MDL_BIT(MDL_BACKUP_SYS_DML) | + MDL_BIT(MDL_BACKUP_DDL) | + MDL_BIT(MDL_BACKUP_ALTER_COPY) | + MDL_BIT(MDL_BACKUP_COMMIT)); } ~MDL_lock() @@ -723,6 +1146,7 @@ class MDL_lock { unsigned long res= 0; DBUG_ASSERT(key.mdl_namespace() == MDL_key::USER_LOCK); + DBUG_ASSERT(!m_fast_road.is_enabled()); mysql_prlock_rdlock(&m_rwlock); DBUG_ASSERT(m_strategy || is_empty()); if (!m_granted.is_empty()) @@ -738,12 +1162,32 @@ class MDL_lock Another thread may be deleting this MDL_lock concurrently. Being deleted lock can still be iterated since it must have valid empty granted/waiting lists. + + Iterate fast lanes first, then go for conventional m_granted + and m_waiting lists. To make MDL_lock snapshot consistent, fast + lanes iteration has to be performed under m_rwlock protection. + + Strictly speaking fast lanes iteration alone doesn't require + m_rwlock protection. However another thread may be moving the + ticket from fast lane to m_granted list concurrently. + If fast lanes are iterated before m_rwlock critical section, + then ticket iterator may see this ticket twice: once from fast + lane and once from m_granted list. + If fast lanes are iterated after m_rwlock critical section, + then ticket iterator may miss this ticket: it can be removed + from fast lane, but not yet inserted to m_granted list. + + @retval true callback returned true, abort iteration + @retval false success */ bool iterate(mdl_iterator_callback callback, void *argument) { bool res= true; mysql_prlock_rdlock(&m_rwlock); DBUG_ASSERT(m_strategy || is_empty()); + DBUG_ASSERT(m_strategy || m_fast_road.is_empty()); + if (m_fast_road.iterate(callback, argument)) + goto end; for (MDL_ticket &ticket : m_granted) { if (callback(&ticket, argument, true)) @@ -761,8 +1205,16 @@ class MDL_lock } + /** + MDL_context::clone_ticket() helper + + Cloned tickets don't seem to be used by performance critical + code, so they're always added to conventional m_granted list. + Support for fast lanes can be trivially implemented if needed. + */ void add_cloned_ticket(MDL_ticket *ticket) { + m_fast_road.close(ticket->get_type()); mysql_prlock_wrlock(&m_rwlock); m_granted.add_ticket(ticket); mysql_prlock_unlock(&m_rwlock); @@ -774,9 +1226,18 @@ class MDL_lock To update state of MDL_lock object correctly we need to temporarily exclude ticket from the granted queue and then include it back. + + fast lane lock types can only be downgraded to weaker fast lane + lock types. non-fast lane lock types can only be downgraded to + weaker non-fast lane lock types. + + Note that we don't have to reschedule_waiters() when we perform + downgrade via m_fast_road. There can't be any waiters in such case. */ void downgrade(MDL_ticket *ticket, enum_mdl_type type) { + if (m_fast_road.try_change_ticket_type(ticket, type)) + return; mysql_prlock_wrlock(&m_rwlock); m_granted.remove_ticket(ticket); ticket->m_type= type; @@ -791,13 +1252,26 @@ class MDL_lock To update state of MDL_lock object correctly we need to temporarily exclude ticket from the granted queue and then include it back. + + fast lane lock types can only be upgraded to stronger fast lane + lock types. non-fast lane lock types can only be upgraded to + stronger non-fast lane lock types. + + Non-fast lane locks close fast lanes whenever they're registered in + MDL_lock. Whenever such locks are being deregistered, fast lanes must + be reopened. Once all closers are gone, that is number of close calls + equals to number of open calls, fast lanes become available again. */ void upgrade(MDL_ticket *ticket, enum_mdl_type type, MDL_ticket *remove) { + DBUG_ASSERT(!ticket->m_fast_lane.load(std::memory_order_relaxed)); mysql_prlock_wrlock(&m_rwlock); - if (remove) + if (remove && !m_fast_road.try_release(remove)) + { + m_fast_road.reopen(remove->get_type()); m_granted.remove_ticket(remove); + } m_granted.remove_ticket(ticket); ticket->m_type= type; m_granted.add_ticket(ticket); @@ -805,6 +1279,12 @@ class MDL_lock } + bool try_fast_upgrade(MDL_ticket *ticket, enum_mdl_type type) + { + return m_fast_road.try_change_ticket_type(ticket, type); + } + + void notify_conflicting_locks_if_needed(MDL_ticket *ticket, bool abort_blocking) { if (m_strategy->needs_notification(ticket)) @@ -850,6 +1330,20 @@ class MDL_lock which means it can be detroyed. Caller is expected to dispose ticket immediately anyway. + Certain lock requests can be served by multi-instance scalable fast lanes. + The following requirements must be met to make it happen: fast lanes must + be enabled for this MDL_lock, fast lanes must be open and lock request + type must satisfy Fast_road::supported_type(). + + Fast lanes are available for certain namespaces, e.g. initial implementation + supported only BACKUP namespace and when fast lanes were allocated + successfully. + + Non-fast lane lock requests are never served by fast lanes. Such lock + requests close all fast lanes and move fast lane tickets to conventional + MDL_lock::m_granted list. Fast lanes are reopened once all such locks are + released or aborted. + @retval TAL_ACQUIRED Acquired @retval TAL_WAIT Not acquired, must be waited @retval TAL_NOWAIT Not acquired, cannot be waited @@ -861,9 +1355,14 @@ class MDL_lock // TAL_ACQUIRED to make less work on the hot path enum tal_status res= TAL_ACQUIRED; ticket->m_lock= this; + + if (m_fast_road.try_acquire_lock(ticket)) + return res; + mysql_prlock_wrlock(&m_rwlock); if (likely(m_strategy)) { + m_fast_road.close(ticket->get_type()); if (can_grant_lock(ticket->get_type(), mdl_context, false)) m_granted.add_ticket(ticket); else @@ -884,18 +1383,40 @@ class MDL_lock check_if_conflicting_replication_locks(mdl_context))); } else + { + m_fast_road.reopen(ticket->get_type()); res= TAL_NOWAIT; + } } } else + { + DBUG_ASSERT(m_fast_road.is_closed()); res= TAL_ERROR; + } mysql_prlock_unlock(&m_rwlock); return res; } + /** + Releases previously acquired lock. + + Lock requests that were served by fast lanes are redirected to fast + lanes. No waiters are possible in this case, there is nobody to awake. + + Lock requests that were served by fast lanes, which were closed + in the meantime, are released conventionally. + + Conventional lock release consists of removing lock from m_granted + list, awaking waiters and destroying MDL_lock if needed. + */ void release(LF_PINS *pins, MDL_ticket *ticket) { + DBUG_ASSERT(key.mdl_namespace() == MDL_key::BACKUP || + !m_fast_road.is_enabled()); + if (m_fast_road.try_release(ticket)) + return; remove_ticket(pins, &MDL_lock::m_granted, ticket); } @@ -1204,6 +1725,7 @@ MDL_ticket::MDL_ticket(MDL_context *ctx_arg, MDL_request *mdl_request): m_duration(mdl_request->duration), #endif m_time(0), + m_fast_lane(nullptr), m_type(mdl_request->type), m_ctx(ctx_arg), m_lock(nullptr) @@ -1959,11 +2481,17 @@ MDL_lock::can_grant_lock(enum_mdl_type type_arg, Once ticket is removed from the list, this thread doesn't hold references to this lock and it can be destroyed concurrently. It means once m_rwlock is released, "this" cannot be referenced anymore. + + Non-fast lane locks close fast lanes whenever they're registered in + MDL_lock. Whenever such locks are being deregistered, fast lanes must + be reopened. Once all closers are gone, that is number of close calls + equals to number of open calls, fast lanes become available again. */ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, MDL_ticket *ticket) { + DBUG_ASSERT(!ticket->m_fast_lane.load(std::memory_order_relaxed)); mysql_prlock_wrlock(&m_rwlock); (this->*list).remove_ticket(ticket); if (is_empty()) @@ -1995,6 +2523,7 @@ void MDL_lock::remove_ticket(LF_PINS *pins, Ticket_list MDL_lock::*list, */ reschedule_waiters(); } + m_fast_road.reopen(ticket->get_type()); mysql_prlock_unlock(&m_rwlock); } @@ -2639,6 +3168,11 @@ bool MDL_context::acquire_locks(MDL_request_list *mdl_requests, This invariant is ensured by the fact that upgradeable locks SU, SNW and SNRW are not compatible with each other and themselves. + If mdl_ticket was granted via fast lanes it can only be upgraded to fast + lane lock type. Try fast upgrade in this case, no need to acquire new + ticket and do conventional upgrade as there're no waiters possible. + If fast upgrade fails, do conventional upgrade. + @retval FALSE Success @retval TRUE Failure (thread was killed) */ @@ -2671,6 +3205,9 @@ MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket, mdl_ticket->get_key()->mdl_namespace() != MDL_key::BACKUP) DBUG_RETURN(FALSE); + if (mdl_ticket->get_lock()->try_fast_upgrade(mdl_ticket, new_type)) + DBUG_RETURN(false); + MDL_REQUEST_INIT_BY_KEY(&mdl_xlock_request, mdl_ticket->get_key(), new_type, MDL_TRANSACTION); diff --git a/sql/mdl.h b/sql/mdl.h index 06bf86105547f..c6bd3d55bdf6b 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -22,6 +22,7 @@ #include #include #include +#include #include "lex_ident.h" class THD; @@ -759,6 +760,9 @@ class MDL_ticket : public MDL_wait_for_subgraph, public ilist_node<> MDL_ticket(MDL_context *ctx_arg, MDL_request *request); ~MDL_ticket(); private: + /** Property of MDL_lock::Fast_road, unauthorized access is prohibited. */ + std::atomic m_fast_lane; + /** Type of metadata lock. Externally accessible. */ enum enum_mdl_type m_type; @@ -1137,6 +1141,7 @@ extern "C" int thd_is_connected(MYSQL_THD thd); to avoid starving out weak, low-prio locks. */ extern "C" ulong max_write_lock_count; +extern uint mdl_instances; typedef int (*mdl_iterator_callback)(MDL_ticket *ticket, void *arg, bool granted); diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index c45559794222f..db61b99c5601d 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -1991,6 +1991,14 @@ static Sys_var_ulong Sys_metadata_locks_hash_instances( BLOCK_SIZE(1), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0), ON_UPDATE(0), DEPRECATED(1105, "")); +static Sys_var_uint Sys_metadata_locks_instances( + "metadata_locks_instances", + "Number of fast lanes to create for metadata locks. Can be used to " + "improve DML scalability by eliminating MDL_lock::rwlock load. " + "Use 1 to disable MDL fast lanes. Supported MDL namespaces: BACKUP", + READ_ONLY GLOBAL_VAR(mdl_instances), CMD_LINE(REQUIRED_ARG), + VALID_RANGE(1, 256), DEFAULT(8), BLOCK_SIZE(1)); + static Sys_var_on_access_session Sys_pseudo_thread_id(