From 087071a61ce5c1cf02bec4f83664b4c3c0b23812 Mon Sep 17 00:00:00 2001 From: Rinat Ibragimov Date: Sun, 3 Apr 2022 03:17:59 +0300 Subject: [PATCH] PS-7923 Sort metadata locks acquired during ALTER TABLESPACE https://jira.percona.com/browse/PS-7923 Problem: Metadata locks upgraded during ALTER TABLESPACE are upgraded in no particular order, which present deadlock possibility. Metadata deadlock prevention mechanism kicks in and rolls back an attachable transaction during data dictionary updates, which causes the server to terminate. Fix: Sort metadata lock requests before upgrade procedure. Move metadata lock upgrading routine to MDL_context class to reuse the sorting comparator. --- sql/mdl.cc | 53 +++++++++++++++++++++++++++++++++++-------- sql/mdl.h | 9 ++++++++ sql/sql_tablespace.cc | 15 +++++------- 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 250a3889155c..434e4b987f50 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -58,6 +58,7 @@ extern MYSQL_PLUGIN_IMPORT CHARSET_INFO *system_charset_info; static PSI_memory_key key_memory_MDL_context_acquire_locks; +static PSI_memory_key key_memory_MDL_context_upgrade_shared_locks; #ifdef HAVE_PSI_INTERFACE static PSI_mutex_key key_MDL_wait_LOCK_wait_status; @@ -83,6 +84,9 @@ static PSI_cond_info all_mdl_conds[] = {{&key_MDL_wait_COND_wait_status, static PSI_memory_info all_mdl_memory[] = { {&key_memory_MDL_context_acquire_locks, "MDL_context::acquire_locks", 0, 0, + "Buffer for sorting lock requests."}, + {&key_memory_MDL_context_upgrade_shared_locks, + "MDL_context::upgrade_shared_locks", 0, 0, "Buffer for sorting lock requests."}}; /** @@ -3652,20 +3656,15 @@ bool MDL_context::acquire_locks(MDL_request_list *mdl_requests, any new such locks taken if acquisition fails. */ MDL_ticket *explicit_front = m_ticket_store.front(MDL_EXPLICIT); - const size_t req_count = mdl_requests->elements(); - - if (req_count == 0) return false; /* Sort requests according to MDL_key. */ Prealloced_array sort_buf( key_memory_MDL_context_acquire_locks); - if (sort_buf.reserve(req_count)) return true; - for (size_t ii = 0; ii < req_count; ++ii) { - sort_buf.push_back(it++); - } - - std::sort(sort_buf.begin(), sort_buf.end(), MDL_request_cmp()); + if (filter_and_sort_requests_by_mdl_key( + &sort_buf, mdl_requests, + nullptr /* No filter, process whole array. */)) + return true; size_t num_acquired = 0; for (p_req = sort_buf.begin(); p_req != sort_buf.end(); p_req++) { @@ -3855,6 +3854,42 @@ bool MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket, return false; } +bool MDL_context::filter_and_sort_requests_by_mdl_key( + Prealloced_array *sort_buf, + MDL_request_list *mdl_requests, bool (*filter_func)(MDL_request *)) { + const size_t req_count = mdl_requests->elements(); + if (req_count == 0) return false; + if (sort_buf->reserve(req_count)) return true; + MDL_request_list::Iterator it(*mdl_requests); + for (size_t ii = 0; ii < req_count; ++ii) { + MDL_request *r = it++; + if (filter_func == nullptr || filter_func(r)) sort_buf->push_back(r); + } + std::sort(sort_buf->begin(), sort_buf->end(), MDL_request_cmp()); + return false; +} + +bool MDL_context::upgrade_shared_locks(MDL_request_list *mdl_requests, + enum_mdl_type new_type, + Timeout_type lock_wait_timeout, + bool (*filter_func)(MDL_request *)) { + const size_t req_count = mdl_requests->elements(); + if (req_count == 0) return false; + + /* Sort requests according to MDL_key. */ + Prealloced_array sort_buf( + key_memory_MDL_context_upgrade_shared_locks); + + if (filter_and_sort_requests_by_mdl_key(&sort_buf, mdl_requests, filter_func)) + return true; + + for (MDL_request *r : sort_buf) { + if (upgrade_shared_lock(r->ticket, new_type, lock_wait_timeout)) + return true; + } + return false; +} + /** A fragment of recursive traversal of the wait-for graph in search for deadlocks. Direct the deadlock visitor to all diff --git a/sql/mdl.h b/sql/mdl.h index df45d2a41963..50250a607f6a 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -44,6 +44,7 @@ #include "mysql/components/services/psi_stage_bits.h" #include "mysql/psi/mysql_rwlock.h" #include "mysql_com.h" +#include "prealloced_array.h" #include "sql/sql_plist.h" #include "template_utils.h" @@ -1423,6 +1424,10 @@ class MDL_context { Timeout_type lock_wait_timeout); bool upgrade_shared_lock(MDL_ticket *mdl_ticket, enum_mdl_type new_type, Timeout_type lock_wait_timeout); + bool upgrade_shared_locks(MDL_request_list *mdl_requests, + enum_mdl_type new_type, + Timeout_type lock_wait_timeout, + bool (*filter_func)(MDL_request *) = nullptr); bool clone_ticket(MDL_request *mdl_request); @@ -1668,6 +1673,10 @@ class MDL_context { friend bool mdl_unittest::test_drive_fix_pins(MDL_context *); bool fix_pins(); + bool filter_and_sort_requests_by_mdl_key( + Prealloced_array *sort_buf, + MDL_request_list *mdl_requests, bool (*filter_func)(MDL_request *)); + public: void find_deadlock(); diff --git a/sql/sql_tablespace.cc b/sql/sql_tablespace.cc index 7acca4067398..13f13431c94c 100644 --- a/sql/sql_tablespace.cc +++ b/sql/sql_tablespace.cc @@ -896,15 +896,12 @@ static bool upgrade_lock_for_tables_in_tablespace( DEBUG_SYNC(thd, "upgrade_lock_for_tables_in_tablespace_kill_point"); - MDL_request_list::Iterator it(*table_mdl_reqs); - const size_t req_count = table_mdl_reqs->elements(); - for (size_t i = 0; i < req_count; ++i) { - MDL_request *r = it++; - if (r->key.mdl_namespace() == MDL_key::TABLE && - thd->mdl_context.upgrade_shared_lock(r->ticket, MDL_EXCLUSIVE, - LONG_TIMEOUT)) - return true; - } + if (thd->mdl_context.upgrade_shared_locks( + table_mdl_reqs, MDL_EXCLUSIVE, LONG_TIMEOUT, [](MDL_request *r) { + // Only process MDL_request's for table locks. + return r->key.mdl_namespace() == MDL_key::TABLE; + })) + return true; return false; }