A new implementation for the TABLE_SHARE cache in MDL

subsystem. Fix a number of caveates that the previous
implementation suffered from, including unprotected
access to shared data and lax resource accounting
(share->ref_count) that could lead to deadlocks.

The new implementation still suffers from a number
of potential deadlocks in some edge cases, and this is 
still not enabled by default. Especially since performance
testing has shown that it gives only marginable (not even 
exceeding measuring accuracy) improvements.

@todo: 
- Remove calls to close_cached_tables() with REFRESH_FAST,
and have_lock, because they break the MDL cache. 
- rework FLUSH TABLES <list> to not use close_cached_tables()
- make sure that whenever we set TABLE_SHARE::version to
0 we free MDL cache references to it.


sql/mdl.cc:
  We may cache references to TABLE_SHARE objects in 
  MDL_lock objects for tables. Create a separate
  MDL_lock class to represent a table.
sql/mdl.h:
  Adjust the MDL caching API to avoid races.
sql/sql_base.cc:
  Move all caching functionality close together.
  Implement a solution for deadlocks caused by 
  close_cached_tables() when MDL cache is enabled (incomplete).
sql/sql_yacc.yy:
  Adjust FLUSH rule to do the necessary initialization of
  TABLE_LIST elements used in for FLUSH TABLES <list>, and thus
  work OK with flush_mdl_cache() function.

1 files changed, 53 insertions, 29 deletions

diff --git a/sql/mdl.cc b/sql/mdl.cc
index 184b3c6051d..22ad15d2360 100644
--- a/sql/mdl.cc
+++ b/sql/mdl.cc
@@ -284,8 +284,8 @@ public:
 public:
   /** The key of the object (data) being protected. */
   MDL_key key;
-  void   *cached_object;
-  mdl_cached_object_release_hook cached_object_release_hook;
+  /** A cached reference to the TABLE_SHARE. Protected by LOCK_open. */
+  void   *m_cached_object;
   /**
     Read-write lock protecting this lock context.
 
@@ -362,8 +362,7 @@ public:
 
   MDL_lock(const MDL_key *key_arg)
   : key(key_arg),
-    cached_object(NULL),
-    cached_object_release_hook(NULL),
+    m_cached_object(NULL),
     m_ref_usage(0),
     m_ref_release(0),
     m_is_destroyed(FALSE)
@@ -371,6 +370,8 @@ 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);
@@ -458,6 +459,25 @@ 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"
@@ -674,8 +694,7 @@ void MDL_map::remove(MDL_lock *lock)
 {
   uint ref_usage, ref_release;
 
-  if (lock->cached_object)
-    (*lock->cached_object_release_hook)(lock->cached_object);
+  lock->release_cached_object();
 
   /*
     Destroy the MDL_lock object, but ensure that anyone that is
@@ -839,6 +858,8 @@ inline MDL_lock *MDL_lock::create(const MDL_key *mdl_key)
   {
     case MDL_key::GLOBAL:
       return new MDL_global_lock(mdl_key);
+    case MDL_key::TABLE:
+      return new MDL_table_lock(mdl_key);
     default:
       return new MDL_object_lock(mdl_key);
   }
@@ -1181,11 +1202,6 @@ void MDL_lock::reschedule_waiters()
         */
         m_waiting.remove_ticket(ticket);
         m_granted.add_ticket(ticket);
-
-        /* If we are granting an X lock, release the cached object. */
-        if (ticket->get_type() == MDL_EXCLUSIVE && cached_object)
-          (*cached_object_release_hook)(cached_object);
-        cached_object= NULL;
       }
       /*
         If we could not update the wait slot of the waiter,
@@ -1655,14 +1671,13 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
   {
     lock->m_granted.add_ticket(ticket);
 
-    if (mdl_request->type == MDL_EXCLUSIVE && lock->cached_object)
-      (*lock->cached_object_release_hook)(lock->cached_object);
-    lock->cached_object= NULL;
-
     mysql_prlock_unlock(&lock->m_rwlock);
 
     m_tickets.push_front(ticket);
 
+    if (ticket->get_type() == MDL_EXCLUSIVE)
+      ticket->clear_cached_object();
+
     mdl_request->ticket= ticket;
   }
   else
@@ -1864,6 +1879,9 @@ 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;
@@ -2450,7 +2468,7 @@ bool MDL_ticket::has_pending_conflicting_lock() const
 
   This function has the following usage pattern:
     - try to acquire an MDL lock
-    - when done, call for mdl_get_cached_object(). If it returns NULL, our
+    - 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.
@@ -2460,29 +2478,34 @@ bool MDL_ticket::has_pending_conflicting_lock() const
 */
 
 void
-MDL_ticket::set_cached_object(void *cached_object,
-                              mdl_cached_object_release_hook release_hook)
+MDL_ticket::set_cached_object(void *cached_object)
 {
-  DBUG_ENTER("mdl_set_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));
-  /*
-    TODO: This assumption works now since we do get_cached_object()
-          and set_cached_object() in the same critical section. Once
-          this becomes false we will have to call release_hook here and
-          use additional mutex protecting 'cached_object' member.
-  */
-  DBUG_ASSERT(!m_lock->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->cached_object= cached_object;
-  m_lock->cached_object_release_hook= release_hook;
+  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.
@@ -2492,7 +2515,8 @@ MDL_ticket::set_cached_object(void *cached_object,
 
 void *MDL_ticket::get_cached_object()
 {
-  return m_lock->cached_object;
+  mysql_mutex_assert_owner(&LOCK_open);
+  return m_lock->m_cached_object;
 }