MDEV-30182: Optimize open_tables to take O(N) time

sql/mdl.cc

@@ -1721,9 +1721,7 @@ void MDL_request::init_by_key_with_source(const MDL_key *key_arg,
 
 
 MDL_ticket::MDL_ticket(MDL_context *ctx_arg, MDL_request *mdl_request):
-#ifndef DBUG_OFF
    m_duration(mdl_request->duration),
-#endif
    m_time(0),
    m_fast_lane(nullptr),
    m_type(mdl_request->type),
@@ -2589,8 +2587,9 @@ const LEX_STRING *MDL_ticket::get_type_name(enum_mdl_type type) const
 /**
   Check whether the context already holds a compatible lock ticket
   on an object.
-  Start searching from list of locks for the same duration as lock
-  being requested. If not look at lists for other durations.
+  For the MDL_EXPLICIT duration, look for the tickets with any duration.
+  For other durations, first try to find the ticket with duration other than
+  MDL_EXPLICIT. If not found, check MDL_EXPLICIT duration as well.
 
   @param mdl_request  Lock request object for lock to be acquired
   @param[out] result_duration  Duration of lock which was found.
@@ -2605,30 +2604,31 @@ MDL_ticket *
 MDL_context::find_ticket(MDL_request *mdl_request,
                          enum_mdl_duration *result_duration)
 {
-  MDL_ticket *ticket;
-  int i;
+  const auto &ticket_identical=
+      [&mdl_request](const MDL_ticket *t) {
+        return mdl_request->key.is_equal(t->get_key()) &&
+               t->has_stronger_or_equal_type(mdl_request->type) &&
+               t->m_duration == mdl_request->duration;
+      };
 
-  for (i= 0; i < MDL_DURATION_END; i++)
+  MDL_ticket *found_ticket= ticket_hash.find(mdl_request->key,
+                                             ticket_identical);
+  if (!found_ticket)
   {
-    enum_mdl_duration duration= (enum_mdl_duration)((mdl_request->duration+i) %
-                                                    MDL_DURATION_END);
-    Ticket_iterator it(m_tickets[duration]);
+    const auto &ticket_still_good=
+        [&mdl_request](const MDL_ticket *t) {
+          return mdl_request->key.is_equal(t->get_key()) &&
+                 t->has_stronger_or_equal_type(mdl_request->type);
+        };
 
-    while ((ticket= it++))
-    {
-      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",
-                            get_mdl_lock_name(mdl_request->key.mdl_namespace(),
-                                              mdl_request->type)->str,
-                            ticket->get_type_name()->str));
-        *result_duration= duration;
-        return ticket;
-      }
-    }
+    found_ticket= ticket_hash.find(&mdl_request->key, ticket_still_good);
   }
-  return NULL;
+
+  if (unlikely(!found_ticket))
+    return NULL;
+
+  *result_duration= found_ticket->m_duration;
+  return found_ticket;
 }
 
 
@@ -2741,6 +2741,8 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
   switch (mdl_locks.try_acquire_lock(m_pins, &mdl_request->key, ticket,
                                      out_ticket != nullptr)) {
   case TAL_ACQUIRED:
+    if (!ticket_hash.insert(ticket))
+      return TRUE;
     m_tickets[mdl_request->duration].push_front(ticket);
     mdl_request->ticket= ticket;
     mysql_mdl_set_status(ticket->m_psi, MDL_ticket::GRANTED);
@@ -2803,6 +2805,11 @@ MDL_context::clone_ticket(MDL_request *mdl_request)
 
   ticket->m_lock= mdl_request->ticket->m_lock;
   ticket->m_time= mdl_request->ticket->m_time;
+  if (!ticket_hash.insert(ticket))
+  {
+    delete ticket;
+    return TRUE;
+  }
   mdl_request->ticket= ticket;
 
   ticket->m_lock->add_cloned_ticket(ticket);
@@ -3065,6 +3072,9 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
   m_wait.reset_status();
 
   m_tickets[mdl_request->duration].push_front(ticket);
+  bool success = ticket_hash.insert(ticket);
+  if (unlikely(!success))
+    DBUG_RETURN(TRUE);
 
   mdl_request->ticket= ticket;
 
@@ -3222,13 +3232,19 @@ MDL_context::upgrade_shared_lock(MDL_ticket *mdl_ticket,
   mdl_ticket->m_lock->upgrade(mdl_ticket, new_type,
                     is_new_ticket ? mdl_xlock_request.ticket : nullptr);
 
+  bool success= true;
   if (is_new_ticket)
   {
+    success= ticket_hash.erase(mdl_xlock_request.ticket);
+    DBUG_ASSERT(success || current_thd->is_error()); // only OOM
+    // We continue removing old data on failure.
+    DBUG_ASSERT(ticket_hash.find(mdl_xlock_request.ticket) == NULL);
+
     m_tickets[MDL_TRANSACTION].remove(mdl_xlock_request.ticket);
     delete mdl_xlock_request.ticket;
   }
 
-  DBUG_RETURN(FALSE);
+  DBUG_RETURN(MY_TEST(!success));
 }
 
 
@@ -3492,6 +3508,10 @@ 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)));
 
+  IF_DBUG(bool success=,) ticket_hash.erase(ticket);
+  DBUG_ASSERT(success);
+  DBUG_ASSERT(ticket_hash.find(ticket) == NULL);
+
   lock->release(m_pins, ticket);
 
   m_tickets[duration].remove(ticket);
@@ -3782,9 +3802,7 @@ void MDL_context::set_lock_duration(MDL_ticket *mdl_ticket,
 
   m_tickets[MDL_TRANSACTION].remove(mdl_ticket);
   m_tickets[duration].push_front(mdl_ticket);
-#ifndef DBUG_OFF
   mdl_ticket->m_duration= duration;
-#endif
 }
 
 
@@ -3819,12 +3837,10 @@ void MDL_context::set_explicit_duration_for_all_locks()
     }
   }
 
-#ifndef DBUG_OFF
   Ticket_iterator exp_it(m_tickets[MDL_EXPLICIT]);
 
   while ((ticket= exp_it++))
     ticket->m_duration= MDL_EXPLICIT;
-#endif
 }
 
 
@@ -3857,12 +3873,10 @@ void MDL_context::set_transaction_duration_for_all_locks()
     m_tickets[MDL_TRANSACTION].push_front(ticket);
   }
 
-#ifndef DBUG_OFF
   Ticket_iterator trans_it(m_tickets[MDL_TRANSACTION]);
 
   while ((ticket= trans_it++))
     ticket->m_duration= MDL_TRANSACTION;
-#endif
 }
 
 

sql/mdl.h

@@ -24,6 +24,7 @@
 #include <lf.h>
 #include <atomic>
 #include "lex_ident.h"
+#include "open_address_hash.h"
 
 class THD;
 
@@ -710,14 +711,12 @@ class MDL_ticket : public MDL_wait_for_subgraph, public ilist_node<>
   MDL_ticket *next_in_context;
   MDL_ticket **prev_in_context;
 
-#ifndef DBUG_OFF
   /**
     Duration of lock represented by this ticket.
-    Context public. Debug-only.
+    Context public.
   */
 public:
   enum_mdl_duration m_duration;
-#endif
   ulonglong m_time;
 
 #ifdef WITH_WSREP
@@ -856,6 +855,24 @@ typedef I_P_List<MDL_request, I_P_List_adapter<MDL_request,
                  I_P_List_counter>
         MDL_request_list;
 
+
+template <typename T>
+struct MDL_key_trait
+{
+  using Hash_value_type= decltype(MDL_key().tc_hash_value());
+
+  static const MDL_key *get_key(T *t) { return t->get_key(); }
+  static my_hash_value_type get_hash_value(const MDL_key *key)
+  {
+    return key->tc_hash_value();
+  }
+};
+namespace traits
+{
+template<>
+struct Open_address_hash_key_trait<MDL_key>: public MDL_key_trait<MDL_ticket>{};
+};
+
 /**
   Context of the owner of metadata locks. I.e. each server
   connection has such a context.
@@ -1052,12 +1069,18 @@ class MDL_context
 private:
   MDL_ticket *find_ticket(MDL_request *mdl_req,
                           enum_mdl_duration *duration);
+
   void release_locks_stored_before(enum_mdl_duration duration, MDL_ticket *sentinel);
   void release_lock(enum_mdl_duration duration, MDL_ticket *ticket);
   bool try_acquire_lock_impl(MDL_request *mdl_request,
                              MDL_ticket **out_ticket);
   bool fix_pins();
 
+  /**
+    Ticket hash. Stores only locked tickets.
+  */
+  Open_address_hash<MDL_key, MDL_ticket*> ticket_hash;
+
 public:
   THD *get_thd() const { return m_owner->get_thd(); }
   bool has_explicit_locks();

sql/sp_head.cc

@@ -3741,6 +3741,7 @@ sp_head::add_used_tables_to_table_list(THD *thd,
                                            belong_to_view,
                                            stab->trg_event_map,
                                            query_tables_last_ptr,
+                                           NULL,
                                            stab->for_insert_data);
       tab_buff+= ALIGN_SIZE(sizeof(TABLE_LIST));
       result= TRUE;

sql/sql_base.cc

@@ -5036,23 +5036,17 @@ bool DML_prelocking_strategy::handle_routine(THD *thd,
   return FALSE;
 }
 
-
-/*
-  @note this can be changed to use a hash, instead of scanning the linked
-  list, if the performance of this function will ever become an issue
-*/
-bool table_already_fk_prelocked(TABLE_LIST *tl, LEX_CSTRING *db,
-                                LEX_CSTRING *table, thr_lock_type lock_type)
+TABLE_LIST *find_fk_prelocked_table(const Query_tables_list *prelocking_ctx,
+                                    const MDL_key &key,
+                                    thr_lock_type lock_type)
 {
-  for (; tl; tl= tl->next_global )
-  {
-    if (tl->lock_type >= lock_type &&
-        tl->prelocking_placeholder == TABLE_LIST::PRELOCK_FK &&
-        strcmp(tl->db.str, db->str) == 0 &&
-        strcmp(tl->table_name.str, table->str) == 0)
-      return true;
-  }
-  return false;
+  return prelocking_ctx->fk_table_hash.find(key,
+          [&key, lock_type](const TABLE_LIST *tl) {
+                  return tl->lock_type >= lock_type
+                         && tl->prelocking_placeholder == TABLE_LIST::PRELOCK_FK
+                         && strcmp(tl->table_name.str, key.name()) == 0
+                         && strcmp(tl->db.str, key.db_name()) == 0;
+  });
 }
 
 
@@ -5104,6 +5098,7 @@ add_internal_tables(THD *thd, Query_tables_list *prelocking_ctx,
                                       TABLE_LIST::PRELOCK_NONE,
                                       0, 0,
                                       &prelocking_ctx->query_tables_last,
+                                      &tables->mdl_request.key,
                                       tables->for_insert_data);
     /*
       Store link to the sequences table so that we can in open_table() update
@@ -5199,30 +5194,56 @@ prepare_fk_prelocking_list(THD *thd, Query_tables_list *prelocking_ctx,
     else
       lock_type= TL_READ;
 
-    if (table_already_fk_prelocked(prelocking_ctx->query_tables,
-          fk->foreign_db, fk->foreign_table, lock_type))
-      continue;
+    const char *db= fk->foreign_db->str;
+    const char *table_name= fk->foreign_table->str;
+    MDL_key key(MDL_key::TABLE, db, table_name);
 
-    TABLE_LIST *tl= thd->alloc<TABLE_LIST>(1);
-    tl->init_one_table_for_prelocking(fk->foreign_db, fk->foreign_table,
-        NULL, lock_type, TABLE_LIST::PRELOCK_FK, table_list->belong_to_view,
-        op, &prelocking_ctx->query_tables_last, table_list->for_insert_data,
-        override_fk_ignore_table);
+    TABLE_LIST *tl= NULL;
+    bool success= prelocking_ctx->fk_table_hash.insert(key,
+            [db, table_name, lock_type](const TABLE_LIST *tl)
+            {
+              return tl->lock_type >= lock_type
+                     && strcmp(tl->table_name.str, table_name) == 0
+                     && strcmp(tl->db.str, db) == 0;
+            },
+            [&tl, thd]()
+            {
+              tl= thd->alloc<TABLE_LIST>(1);
+              return tl;
+            }
+    );
 
-#ifdef WITH_WSREP
-    /*
-      Append table level shared key for the referenced/foreign table for:
-        - statement that updates existing rows (UPDATE, multi-update)
-        - statement that deletes existing rows (DELETE, DELETE_MULTI)
-	- statement that inserts new rows (INSERT, REPLACE, LOAD, ALTER TABLE)
-      This is done to avoid potential MDL conflicts with concurrent DDLs.
-    */
-    if (wsrep_foreign_key_append(thd, fk))
+    if (unlikely(!success))
     {
-      error= TRUE;
-      break;
+      DBUG_ASSERT(thd->is_error()); // ER_OUTOFMEMORY is set inside the hash
+      if (arena)
+        thd->restore_active_arena(arena, &backup);
+      DBUG_RETURN(TRUE);
     }
+
+    if (tl != NULL)
+    {
+      tl->init_one_table_for_prelocking(
+              fk->foreign_db, fk->foreign_table, NULL, lock_type,
+              TABLE_LIST::PRELOCK_FK, table_list->belong_to_view, op,
+              &prelocking_ctx->query_tables_last, &key,
+              table_list->for_insert_data,
+              override_fk_ignore_table);
+#ifdef WITH_WSREP
+      /*
+        Append table level shared key for the referenced/foreign table for:
+          - statement that updates existing rows (UPDATE, multi-update)
+          - statement that deletes existing rows (DELETE, DELETE_MULTI)
+          - statement that inserts new rows (INSERT, REPLACE, LOAD, ALTER TABLE)
+        This is done to avoid potential MDL conflicts with concurrent DDLs.
+      */
+      if (wsrep_foreign_key_append(thd, fk))
+      {
+        error= TRUE;
+        break;
+      }
 #endif // WITH_WSREP
+    }
   }
   if (arena)
     thd->restore_active_arena(arena, &backup);
@@ -5289,20 +5310,20 @@ bool DML_prelocking_strategy::handle_table(THD *thd,
 
       if (table->triggers->
           add_tables_and_routines_for_triggers(thd, prelocking_ctx, table_list))
-        return TRUE;
+        DBUG_RETURN(TRUE);
     }
 
     if (prepare_fk_prelocking_list(thd, prelocking_ctx, table_list,
                                    need_prelocking,
                                    table_list->trg_event_map))
-      return TRUE;
+      DBUG_RETURN(TRUE);
   }
   else if (table_list->slave_fk_event_map)
   {
     if (prepare_fk_prelocking_list(thd, prelocking_ctx, table_list,
                                    need_prelocking,
                                    table_list->slave_fk_event_map))
-      return TRUE;
+      DBUG_RETURN(TRUE);
   }
 
   /* Open any tables used by DEFAULT (like sequence tables) */

sql/sql_class.h

@@ -59,6 +59,7 @@
 #include "ha_handler_stats.h"                    // ha_handler_stats */
 #include "sql_basic_types.h"                     // enum class active_dml_stmt
 #include "sql_trigger.h"
+#include "open_address_hash.h"
 
 extern "C"
 void set_thd_stage_info(void *thd,