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

@@ -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<MDL_request *, 16> 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<MDL_request *, 16> *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<MDL_request *, 16> 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

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<MDL_request *, 16> *sort_buf,
+      MDL_request_list *mdl_requests, bool (*filter_func)(MDL_request *));
+
  public:
   void find_deadlock();
 

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;
 }