A pre-requisite for patch fixing bug #52044 "FLUSH TABLES

WITH READ LOCK and FLUSH TABLES <list> WITH READ LOCK are
incompatible", which adds information about waits caused by 
FLUSH TABLES statement to deadlock detector in MDL subsystem.

Remove API supporting caching of pointers to TABLE_SHARE 
object in MDL subsystem and all code related to it. 

The problem was that locking requirements of code 
implementing this API conflicted with locking requirements 
of code which adds information about waits caused by flushes 
to deadlock detector in MDL subsystem (the former needed to
lock LOCK_open or its future equivalent while having 
write-lock on MDL_lock's rwlock, and the latter needs to be 
able to read-lock MDL_lock rwlock while owning LOCK_open or 
its future equivalent).

Since caching of pointers to TABLE_SHARE objects in MDL 
subsystem didn't bring expected performance benefits we 
decided to remove caching API rather than try to come up 
with some complex solution for this problem.

3 files changed, 45 insertions, 338 deletions

diff --git a/sql/mdl.cc b/sql/mdl.cc
index c5b84902823..818b736e597 100644
--- a/sql/mdl.cc
+++ b/sql/mdl.cc
@@ -284,8 +284,6 @@ public:
 public:
   /** The key of the object (data) being protected. */
   MDL_key key;
-  /** A cached reference to the TABLE_SHARE. Protected by LOCK_open. */
-  void   *m_cached_object;
   /**
     Read-write lock protecting this lock context.
 
@@ -362,7 +360,6 @@ public:
 
   MDL_lock(const MDL_key *key_arg)
   : key(key_arg),
-    m_cached_object(NULL),
     m_ref_usage(0),
     m_ref_release(0),
     m_is_destroyed(FALSE)
@@ -370,8 +367,6 @@ public:
     mysql_prlock_init(key_MDL_lock_rwlock, &m_rwlock);
   }
 
-  /* Overridden for TABLE objects, to support TABLE_SHARE cache in MDL. */
-  virtual void release_cached_object() {}
   virtual ~MDL_lock()
   {
     mysql_prlock_destroy(&m_rwlock);
@@ -460,25 +455,6 @@ private:
 };
 
 
-/** 
-  A lock implementation for MDL_key::TABLE.
-*/
-
-class MDL_table_lock: public MDL_object_lock
-{
-public:
-  MDL_table_lock(const MDL_key *key_arg)
-    : MDL_object_lock(key_arg)
-  { }
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-  virtual void release_cached_object()
-  {
-    tdc_release_cached_share(&m_cached_object);
-  }
-#endif
-};
-
-
 static MDL_map mdl_locks;
 
 extern "C"
@@ -695,8 +671,6 @@ void MDL_map::remove(MDL_lock *lock)
 {
   uint ref_usage, ref_release;
 
-  lock->release_cached_object();
-
   /*
     Destroy the MDL_lock object, but ensure that anyone that is
     holding a reference to the object is not remaining, if so he
@@ -860,8 +834,6 @@ inline MDL_lock *MDL_lock::create(const MDL_key *mdl_key)
     case MDL_key::GLOBAL:
     case MDL_key::SCHEMA:
       return new MDL_scoped_lock(mdl_key);
-    case MDL_key::TABLE:
-      return new MDL_table_lock(mdl_key);
     default:
       return new MDL_object_lock(mdl_key);
   }
@@ -1682,9 +1654,6 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
 
     m_tickets.push_front(ticket);
 
-    if (ticket->get_type() == MDL_EXCLUSIVE)
-      ticket->clear_cached_object();
-
     mdl_request->ticket= ticket;
   }
   else
@@ -1886,9 +1855,6 @@ MDL_context::acquire_lock(MDL_request *mdl_request, ulong lock_wait_timeout)
   */
   DBUG_ASSERT(wait_status == MDL_wait::GRANTED);
 
-  if (ticket->get_type() == MDL_EXCLUSIVE)
-    ticket->clear_cached_object();
-
   m_tickets.push_front(ticket);
 
   mdl_request->ticket= ticket;
@@ -2452,75 +2418,6 @@ bool MDL_ticket::has_pending_conflicting_lock() const
 
 
 /**
-  Associate pointer to an opaque object with a lock.
-
-  @param cached_object Pointer to the object
-  @param release_hook  Cleanup function to be called when MDL subsystem
-                       decides to remove lock or associate another object.
-
-  This is used to cache a pointer to TABLE_SHARE in the lock
-  structure. Such caching can save one acquisition of LOCK_open
-  and one table definition cache lookup for every table.
-
-  Since the pointer may be stored only inside an acquired lock,
-  the caching is only effective when there is more than one lock
-  granted on a given table.
-
-  This function has the following usage pattern:
-    - try to acquire an MDL lock
-    - when done, call for get_cached_object(). If it returns NULL, our
-      thread has the only lock on this table.
-    - look up TABLE_SHARE in the table definition cache
-    - call mdl_set_cache_object() to assign the share to the opaque pointer.
-
- The release hook is invoked when the last shared metadata
- lock on this name is released.
-*/
-
-void
-MDL_ticket::set_cached_object(void *cached_object)
-{
-  DBUG_ENTER("MDL_ticket::set_cached_object");
-  DBUG_PRINT("enter", ("db=%s name=%s cached_object=%p",
-                        m_lock->key.db_name(), m_lock->key.name(),
-                        cached_object));
-  mysql_mutex_assert_owner(&LOCK_open);
-  DBUG_ASSERT(m_lock->key.mdl_namespace() == MDL_key::TABLE);
-  DBUG_ASSERT(!m_lock->m_cached_object);
-
-  m_lock->m_cached_object= cached_object;
-
-  DBUG_VOID_RETURN;
-}
-
-
-/**
-  A helper function to flush the table share cached in MDL.
-  @pre The ticket is acquired.
-*/
-
-void MDL_ticket::clear_cached_object()
-{
-  m_lock->release_cached_object();
-}
-
-
-/**
-  Get a pointer to an opaque object that associated with the lock.
-
-  @param ticket  Lock ticket for the lock which the object is associated to.
-
-  @return Pointer to an opaque object associated with the lock.
-*/
-
-void *MDL_ticket::get_cached_object()
-{
-  mysql_mutex_assert_owner(&LOCK_open);
-  return m_lock->m_cached_object;
-}
-
-
-/**
   Releases metadata locks that were acquired after a specific savepoint.
 
   @note Used to release tickets acquired during a savepoint unit.
diff --git a/sql/mdl.h b/sql/mdl.h
index 5c58289aea2..319c16cf6ce 100644
--- a/sql/mdl.h
+++ b/sql/mdl.h
@@ -398,9 +398,6 @@ public:
 public:
   bool has_pending_conflicting_lock() const;
 
-  void *get_cached_object();
-  void set_cached_object(void *cached_object);
-  void clear_cached_object();
   MDL_context *get_ctx() const { return m_ctx; }
   bool is_upgradable_or_exclusive() const
   {
@@ -728,7 +725,6 @@ extern "C" const char *set_thd_proc_info(void *thd_arg, const char *info,
                                          const char *calling_function,
                                          const char *calling_file,
                                          const unsigned int calling_line);
-extern void tdc_release_cached_share(void *ptr);
 #ifndef DBUG_OFF
 extern mysql_mutex_t LOCK_open;
 #endif
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index d57554785b8..f18d5e26f01 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -921,141 +921,6 @@ static void kill_delayed_threads_for_table(TABLE_SHARE *share)
 }
 
 
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-/**
-  Flush MDL cached objects.
-
-  How MDL table share cache works
-  -------------------------------
-  Since we take a table share from the table definition
-  cache only after taking an MDL lock, the MDL lock
-  object is a convenient place to cache a pointer
-  to the table share. However, not all SQL in MySQL
-  takes an MDL lock prior to working with the TDC,
-  various forms of FLUSH TABLES (including SET GLOBAL
-  read_only) being the one and only exception.
-
-  To make FLUSH TABLES work, and avoid having dangling
-  references to TABLE_SHARE objects in MDL subsystem
-  after a flush, we make sure that all references 
-  to table shares are released whenever a flush comes.
-  This is done in this function.
-
-  To sum up, the following invariants are held:
-  - no statement can work with a TABLE_SHARE without
-  a metadata lock. The only exception is FLUSH TABLES.
-  - a metadata lock object can be used to store
-  a cached reference (pointer) to the corresponding
-  TABLE_SHARE, if and only if this TABLE_SHARE is
-  not stale (version == refresh_version). In other words,
-  checking TABLE_SHARE version and setting the reference
-  must happen only in the same critical section protected
-  by LOCK_open.
-  - FLUSH will mark all subject TABLE_SHARE objects
-  as stale, and then will manually release all TABLE_SHARE
-  references in MDL cache. Since marking TABLE_SHARE
-  objects is done inside a critical section protected
-  by LOCK_open and prior to calling flush_mdl_cache(),
-  it's guaranteed that a flush will take place before
-  a new reference to the table share is established
-  in some other connection.
-*/
-
-bool flush_mdl_cache(THD *thd, TABLE_LIST *table_list)
-{
-  MDL_request_list mdl_requests;
-  MDL_request *mdl_request;
-
-  DBUG_ENTER("flush_mdl_cache");
-
-  if (table_list == NULL)
-  {
-    mysql_mutex_lock(&LOCK_open);
-    for (uint idx= 0 ; idx < table_def_cache.records; idx++)
-    {
-      TABLE_SHARE *share=(TABLE_SHARE*) my_hash_element(&table_def_cache,
-                                                        idx);
-      if (share->needs_reopen())
-      {
-        mdl_request= MDL_request::create(MDL_key::TABLE,
-                                         share->db.str,
-                                         share->table_name.str,
-                                         MDL_SHARED_HIGH_PRIO,
-                                         thd->mem_root);
-        if (! mdl_request)
-        {
-          mysql_mutex_unlock(&LOCK_open);
-          DBUG_RETURN(TRUE);
-        }
-        mdl_requests.push_front(mdl_request);
-      }
-    }
-    mysql_mutex_unlock(&LOCK_open);
-  }
-  else
-  {
-    for (TABLE_LIST *tables= table_list; tables; tables= tables->next_global)
-    {
-      DBUG_ASSERT(tables->mdl_request.type == MDL_SHARED_HIGH_PRIO);
-      mdl_requests.push_front(&tables->mdl_request);
-    }
-  }
-
-  for (MDL_request_list::Iterator it(mdl_requests);
-       (mdl_request= it++); )
-  {
-    if (thd->mdl_context.try_acquire_lock(mdl_request))
-      DBUG_RETURN(TRUE);
-    if (mdl_request->ticket)
-    {
-      mdl_request->ticket->clear_cached_object();
-      thd->mdl_context.release_lock(mdl_request->ticket);
-    }
-  }
-  DBUG_RETURN(FALSE);
-}
-
-
-/**
-   @brief Helper function used by MDL subsystem for releasing TABLE_SHARE
-          objects in cases when it no longer wants to cache reference to it.
-*/
-
-void tdc_release_cached_share(void *ptr)
-{
-  TABLE_SHARE **share= (TABLE_SHARE **) ptr;
-  mysql_mutex_lock(&LOCK_open);
-  if (*share)
-  {
-    release_table_share(*share);
-    *share= NULL;
-    broadcast_refresh();
-  }
-  mysql_mutex_unlock(&LOCK_open);
-}
-
-
-/**
-   @brief Mark table share as having one more user (increase its reference
-          count).
-
-   @param share Table share for which reference count should be increased.
-*/
-
-static void tdc_reference_table_share(TABLE_SHARE *share)
-{
-  DBUG_ENTER("tdc_reference_table_share");
-  DBUG_ASSERT(share->ref_count);
-  mysql_mutex_assert_owner(&LOCK_open);
-  share->ref_count++;
-  DBUG_PRINT("exit", ("share: 0x%lx  ref_count: %u",
-                     (ulong) share, share->ref_count));
-  DBUG_VOID_RETURN;
-}
-
-
-#endif // DISABLED_UNTIL_GRL_IS_MADE_PART_OF
-
 /*
   Close all tables which aren't in use by any thread
 
@@ -1167,15 +1032,6 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
   /* Wait until all threads have closed all the tables we are flushing. */
   DBUG_PRINT("info", ("Waiting for other threads to close their open tables"));
 
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF
-  /*
-    @todo We need to do this for fast refresh as well, otherwise
-    deadlocks are possible.
-  */
-  if (flush_mdl_cache(thd, tables))
-    goto err_with_reopen;
-#endif // DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-
   while (found && ! thd->killed)
   {
     found= FALSE;
@@ -2969,97 +2825,61 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
     DBUG_RETURN(FALSE);
 
   mysql_mutex_lock(&LOCK_open);
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-  if (!(share= (TABLE_SHARE *) mdl_ticket->get_cached_object()))
-  {
-#endif // DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-    if (!(share= get_table_share_with_create(thd, table_list, key,
-                                             key_length, OPEN_VIEW,
-                                             &error,
-                                             hash_value)))
-      goto err_unlock2;
-
-    if (share->is_view)
-    {
-      /*
-        If parent_l of the table_list is non null then a merge table
-        has this view as child table, which is not supported.
-      */
-      if (table_list->parent_l)
-      {
-        my_error(ER_WRONG_MRG_TABLE, MYF(0));
-        goto err_unlock;
-      }
-
-      /*
-        This table is a view. Validate its metadata version: in particular,
-        that it was a view when the statement was prepared.
-      */
-      if (check_and_update_table_version(thd, table_list, share))
-        goto err_unlock;
-      if (table_list->i_s_requested_object &  OPEN_TABLE_ONLY)
-        goto err_unlock;
-
-      /* Open view */
-      if (open_new_frm(thd, share, alias,
-                       (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
-                               HA_GET_INDEX | HA_TRY_READ_ONLY),
-                       READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD,
-                       thd->open_options,
-                       0, table_list, mem_root))
-        goto err_unlock;
-
-      /* TODO: Don't free this */
-      release_table_share(share);
-
-      DBUG_ASSERT(table_list->view);
 
-      mysql_mutex_unlock(&LOCK_open);
-      DBUG_RETURN(FALSE);
-    }
-    /*
-      Note that situation when we are trying to open a table for what
-      was a view during previous execution of PS will be handled in by
-      the caller. Here we should simply open our table even if
-      TABLE_LIST::view is true.
-    */
-
-    if (table_list->i_s_requested_object &  OPEN_VIEW_ONLY)
-      goto err_unlock;
+  if (!(share= get_table_share_with_create(thd, table_list, key,
+                                           key_length, OPEN_VIEW,
+                                           &error,
+                                           hash_value)))
+    goto err_unlock2;
 
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
+  if (share->is_view)
+  {
     /*
-      We are going to to store extra reference to the share
-      in MDL-subsystem so we need to increase reference counter.
+      If parent_l of the table_list is non null then a merge table
+      has this view as child table, which is not supported.
     */
-    if (! share->needs_reopen())
-    {
-      mdl_ticket->set_cached_object(share);
-      tdc_reference_table_share(share);
-    }
-  }
-  else
-  {
-    if (table_list->view)
+    if (table_list->parent_l)
     {
-      DBUG_ASSERT(thd->m_reprepare_observer);
-      check_and_update_table_version(thd, table_list, share);
-      /* Always an error. */
-      DBUG_ASSERT(thd->is_error());
-      goto err_unlock2;
+      my_error(ER_WRONG_MRG_TABLE, MYF(0));
+      goto err_unlock;
     }
-    /* When we have cached TABLE_SHARE we know that is not a view. */
-    if (table_list->i_s_requested_object &  OPEN_VIEW_ONLY)
-      goto err_unlock2;
 
     /*
-      We are going to use this share for construction of new TABLE object
-      so reference counter should be increased.
+      This table is a view. Validate its metadata version: in particular,
+      that it was a view when the statement was prepared.
     */
-    tdc_reference_table_share(share);
+    if (check_and_update_table_version(thd, table_list, share))
+      goto err_unlock;
+    if (table_list->i_s_requested_object &  OPEN_TABLE_ONLY)
+      goto err_unlock;
+
+    /* Open view */
+    if (open_new_frm(thd, share, alias,
+                     (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
+                             HA_GET_INDEX | HA_TRY_READ_ONLY),
+                     READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD,
+                     thd->open_options,
+                     0, table_list, mem_root))
+      goto err_unlock;
+
+    /* TODO: Don't free this */
+    release_table_share(share);
+
+    DBUG_ASSERT(table_list->view);
+
+    mysql_mutex_unlock(&LOCK_open);
+    DBUG_RETURN(FALSE);
   }
-#endif // DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
 
+  /*
+    Note that situation when we are trying to open a table for what
+    was a view during previous execution of PS will be handled in by
+    the caller. Here we should simply open our table even if
+    TABLE_LIST::view is true.
+  */
+
+  if (table_list->i_s_requested_object &  OPEN_VIEW_ONLY)
+    goto err_unlock;
 
   /*
     If the version changes while we're opening the tables,
@@ -8860,13 +8680,7 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
         automatically deleted once it is no longer referenced.
       */
       share->version= 0;
-#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
-      /*
-        If lock type is not EXCLUSIVE, we must call
-        MDL_ticket::release_cached_object() here to make sure there
-        is no self-reference left on the share in MDL_lock.
-      */
-#endif
+
       while ((table= it++))
         free_cache_entry(table);
     }