Backport of:

------------------------------------------------------------
revno: 2630.9.3
committer: Dmitry Lenev <dlenev@mysql.com>
branch nick: mysql-6.0-3726-w3
timestamp: Wed 2008-06-11 08:33:36 +0400
message:
  WL#3726 "DDL locking for all metadata objects".

  After review fixes in progress.

  Changed close_cached_tables() not to flush all unused TABLE
  instances when flushing individual table.
  Renamed expel_table_from_cache() to tdc_remove_table() and
  added enum parameter to be able more explicitly specify type
  of removal, rewrote its code to be more efficient.

******
Backport of:
------------------------------------------------------------
revno: 2630.9.4
committer: Dmitry Lenev <dlenev@mysql.com>
branch nick: mysql-6.0-3726-w3
timestamp: Wed 2008-06-11 15:53:53 +0400
message:
  WL#3726 "DDL locking for all metadata objects".

  After-review fixes in progress.

  Minor changes in order to improve code readability
  and simplify debugging.


mysql-test/r/ps_ddl.result:
  Restore the original (correct) behaviour,
  now that the patch that fixes the regression
  introduced by the original patch for WL#3726 is fixed.
mysql-test/t/ps_ddl.test:
  Restore the original (correct) behaviour,
  now that the patch that fixes the regression
  introduced by the original patch for WL#3726 is fixed
sql/mysql_priv.h:
  Renamed expel_table_from_cache() to tdc_remove_table() 
  and added enum parameter to be able more explicitly specify 
  type of removal.
sql/sql_base.cc:
  Changed close_cached_tables() not to flush all unused TABLE
  instances when flushing individual table.
  Renamed expel_table_from_cache() to tdc_remove_table() and added
  enum parameter to be able more explicitly specify type of
  removal, rewrote its code to be more efficient. As result removed
  relink_unused() function which is no longer used.
  ******
  Improved code in close_cached_tables() according to reviewer's
  comments.
sql/sql_delete.cc:
  Renamed expel_table_from_cache() to tdc_remove_table() 
  and added enum parameter to be able more explicitly 
  specify type of removal.
sql/sql_rename.cc:
  Renamed expel_table_from_cache() to tdc_remove_table() 
  and added enum parameter to be able more explicitly 
  specify type of removal.
sql/sql_show.cc:
  Moved acquisition of high-prio shared metadata lock in which
  happens in fill_schema_table_from_frm() to separate function.
sql/sql_table.cc:
  Renamed expel_table_from_cache() to tdc_remove_table() 
  and added enum parameter to be able more explicitly 
  specify type of removal.
sql/sql_trigger.cc:
  Renamed expel_table_from_cache() to tdc_remove_table() 
  and added enum parameter to be able more explicitly 
  specify type of removal.

7 files changed, 153 insertions, 97 deletions

diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 2d043d44afa..b650af8017d 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1543,8 +1543,11 @@ char *generate_partition_syntax(partition_info *part_info,
 #endif
 
 bool notify_thread_having_shared_lock(THD *thd, THD *in_use);
-void expel_table_from_cache(THD *leave_thd, const char *db,
-                            const char *table_name);
+
+enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN,
+                                 TDC_RT_REMOVE_UNUSED};
+void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
+                      const char *db, const char *table_name);
 
 #define NORMAL_PART_NAME 0
 #define TEMP_PART_NAME 1
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 633184cde79..f7ac1df8b32 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -921,12 +921,17 @@ void free_io_cache(TABLE *table)
    particular table identified by its share.
 
    @param share Table share.
+
+   @pre Caller should have LOCK_open mutex acquired.
 */
 
 static void kill_delayed_threads_for_table(TABLE_SHARE *share)
 {
   I_P_List_iterator<TABLE, TABLE_share> it(share->used_tables);
   TABLE *tab;
+
+  safe_mutex_assert_owner(&LOCK_open);
+
   while ((tab= it++))
   {
     THD *in_use= tab->in_use;
@@ -983,6 +988,15 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
     DBUG_PRINT("tcache", ("incremented global refresh_version to: %lu",
                           refresh_version));
     kill_delayed_threads();
+    /*
+      Get rid of all unused TABLE and TABLE_SHARE instances. By doing
+      this we automatically close all tables which were marked as "old".
+    */
+    while (unused_tables)
+      free_cache_entry(unused_tables);
+    /* Free table shares which were not freed implicitly by loop above. */
+    while (oldest_unused_share->next)
+      (void) my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
   }
   else
   {
@@ -993,8 +1007,10 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
 
       if (share)
       {
-        share->version= 0;
         kill_delayed_threads_for_table(share);
+        /* tdc_remove_table() also sets TABLE_SHARE::version to 0. */
+        tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, table->db,
+                         table->table_name);
 	found=1;
       }
     }
@@ -1002,28 +1018,14 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
       wait_for_refresh=0;			// Nothing to wait for
   }
 
-  /*
-    Get rid of all unused TABLE and TABLE_SHARE instances. By doing
-    this we automatically close all tables which were marked as "old".
-
-    FIXME: Do not close all unused TABLE instances when flushing
-           particular table.
-  */
-  while (unused_tables)
-    free_cache_entry(unused_tables);
-  /* Free table shares */
-  while (oldest_unused_share->next)
-    (void) my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
+  if (!have_lock)
+    pthread_mutex_unlock(&LOCK_open);
 
   if (!wait_for_refresh)
-  {
-    if (!have_lock)
-      pthread_mutex_unlock(&LOCK_open);
     DBUG_RETURN(result);
-  }
 
+  /* Code below assume that LOCK_open is released. */
   DBUG_ASSERT(!have_lock);
-  pthread_mutex_unlock(&LOCK_open);
 
   if (thd->locked_tables_mode)
   {
@@ -2109,27 +2111,6 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db,
 }
 
 
-	/* move table first in unused links */
-
-static void relink_unused(TABLE *table)
-{
-  /* Assert that MERGE children are not attached in unused_tables. */
-  DBUG_ASSERT(!table->is_children_attached());
-
-  if (table != unused_tables)
-  {
-    table->prev->next=table->next;		/* Remove from unused list */
-    table->next->prev=table->prev;
-    table->next=unused_tables;			/* Link in unused tables */
-    table->prev=unused_tables->prev;
-    unused_tables->prev->next=table;
-    unused_tables->prev=table;
-    unused_tables=table;
-    check_unused();
-  }
-}
-
-
 /**
   Prepare an open merge table for close.
 
@@ -2241,7 +2222,8 @@ bool wait_while_table_is_used(THD *thd, TABLE *table,
   }
 
   pthread_mutex_lock(&LOCK_open);
-  expel_table_from_cache(thd, table->s->db.str, table->s->table_name.str);
+  tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN,
+                   table->s->db.str, table->s->table_name.str);
   pthread_mutex_unlock(&LOCK_open);
   DBUG_RETURN(FALSE);
 }
@@ -4075,7 +4057,7 @@ recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *table,
       if (mdl_acquire_exclusive_locks(&thd->mdl_context))
         return TRUE;
       pthread_mutex_lock(&LOCK_open);
-      expel_table_from_cache(0, table->db, table->table_name);
+      tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name);
       ha_create_table_from_engine(thd, table->db, table->table_name);
       pthread_mutex_unlock(&LOCK_open);
 
@@ -4089,7 +4071,7 @@ recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *table,
       if (mdl_acquire_exclusive_locks(&thd->mdl_context))
         return TRUE;
       pthread_mutex_lock(&LOCK_open);
-      expel_table_from_cache(0, table->db, table->table_name);
+      tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name);
       pthread_mutex_unlock(&LOCK_open);
 
       result= auto_repair_table(thd, table);
@@ -8549,50 +8531,82 @@ bool notify_thread_having_shared_lock(THD *thd, THD *in_use)
 
 
 /**
-   Remove all instances of the table from cache assuming that current thread
-   has exclusive meta-data lock on it (optionally leave instances belonging
-   to the current thread in cache).
-
-   @param  leave_thd  0      If we should remove all instances
-                      non-0  Pointer to current thread context if we should
-                      leave instances belonging to this thread.
-   @param  db         Name of database
-   @param  table_name Name of table
+   Remove all or some (depending on parameter) instances of TABLE and
+   TABLE_SHARE from the table definition cache.
+
+   @param  thd          Thread context
+   @param  remove_type  Type of removal:
+                        TDC_RT_REMOVE_ALL     - remove all TABLE instances and
+                                                TABLE_SHARE instance. There
+                                                should be no used TABLE objects
+                                                and caller should have exclusive
+                                                metadata lock on the table.
+                        TDC_RT_REMOVE_NOT_OWN - remove all TABLE instances
+                                                except those that belong to
+                                                this thread. There should be
+                                                no TABLE objects used by other
+                                                threads and caller should have
+                                                exclusive metadata lock on the
+                                                table.
+                        TDC_RT_REMOVE_UNUSED  - remove all unused TABLE
+                                                instances (if there are no
+                                                used instances will also
+                                                remove TABLE_SHARE).
+   @param  db           Name of database
+   @param  table_name   Name of table
 
    @note Unlike remove_table_from_cache() it assumes that table instances
          are already not used by any (other) thread (this should be achieved
          by using meta-data locks).
 */
 
-void expel_table_from_cache(THD *leave_thd, const char *db, const char *table_name)
+void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
+                      const char *db, const char *table_name)
 {
   char key[MAX_DBKEY_LENGTH];
   uint key_length;
   TABLE *table;
   TABLE_SHARE *share;
 
-  key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
-
-  if ((share= (TABLE_SHARE*) my_hash_search(&table_def_cache,(uchar*) key,
-                                            key_length)))
-  {
-    I_P_List_iterator<TABLE, TABLE_share> it(share->free_tables);
-    share->version= 0;
+  safe_mutex_assert_owner(&LOCK_open);
 
-    while ((table= it++))
-      relink_unused(table);
-  }
+  DBUG_ASSERT(remove_type == TDC_RT_REMOVE_UNUSED ||
+              mdl_is_exclusive_lock_owner(&thd->mdl_context, 0,
+                                          db, table_name));
 
-  /* This may destroy share so we have to do new look-up later. */
-  while (unused_tables && !unused_tables->s->version)
-    free_cache_entry(unused_tables);
+  key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
 
   if ((share= (TABLE_SHARE*) my_hash_search(&table_def_cache,(uchar*) key,
                                             key_length)))
   {
-    DBUG_ASSERT(leave_thd || share->ref_count == 0);
-    if (share->ref_count == 0)
-      my_hash_delete(&table_def_cache, (uchar*) share);
+    if (share->ref_count)
+    {
+      I_P_List_iterator<TABLE, TABLE_share> it(share->free_tables);
+#ifndef DBUG_OFF
+      if (remove_type == TDC_RT_REMOVE_ALL)
+      {
+        DBUG_ASSERT(share->used_tables.is_empty());
+      }
+      else if (remove_type == TDC_RT_REMOVE_NOT_OWN)
+      {
+        I_P_List_iterator<TABLE, TABLE_share> it2(share->used_tables);
+        while ((table= it2++))
+          if (table->in_use != thd)
+          {
+            DBUG_ASSERT(0);
+          }
+      }
+#endif
+      /*
+        Set share's version to zero in order to ensure that it gets
+        automatically deleted once it is no longer referenced.
+      */
+      share->version= 0;
+      while ((table= it++))
+        free_cache_entry(table);
+    }
+    else
+      (void) my_hash_delete(&table_def_cache, (uchar*) share);
   }
 }
 
@@ -8793,7 +8807,7 @@ int abort_and_upgrade_lock(ALTER_PARTITION_PARAM_TYPE *lpt)
     DBUG_RETURN(1);
   }
   pthread_mutex_lock(&LOCK_open);
-  expel_table_from_cache(lpt->thd, lpt->db, lpt->table_name);
+  tdc_remove_table(lpt->thd, TDC_RT_REMOVE_NOT_OWN, lpt->db, lpt->table_name);
   pthread_mutex_unlock(&LOCK_open);
   DBUG_RETURN(0);
 }
diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
index 612f9d1954d..630cf73076c 100644
--- a/sql/sql_delete.cc
+++ b/sql/sql_delete.cc
@@ -1172,7 +1172,8 @@ bool mysql_truncate(THD *thd, TABLE_LIST *table_list, bool dont_send_ok)
     if (mdl_acquire_exclusive_locks(&thd->mdl_context))
       DBUG_RETURN(TRUE);
     pthread_mutex_lock(&LOCK_open);
-    expel_table_from_cache(0, table_list->db, table_list->table_name);
+    tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table_list->db,
+                     table_list->table_name);
     pthread_mutex_unlock(&LOCK_open);
   }
 
diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc
index fdf2f5ac155..ca1543b32d8 100644
--- a/sql/sql_rename.cc
+++ b/sql/sql_rename.cc
@@ -139,7 +139,8 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent)
   pthread_mutex_lock(&LOCK_open);
 
   for (ren_table= table_list; ren_table; ren_table= ren_table->next_local)
-    expel_table_from_cache(0, ren_table->db, ren_table->table_name);
+    tdc_remove_table(thd, TDC_RT_REMOVE_ALL, ren_table->db,
+                     ren_table->table_name);
 
   error=0;
   if ((ren_table=rename_tables(thd,table_list,0)))
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index 9df31c7c2ad..f608e233349 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -3076,6 +3076,56 @@ uint get_table_open_method(TABLE_LIST *tables,
 
 
 /**
+   Acquire high priority share metadata lock on a table.
+
+   @param thd            Thread context.
+   @param mdl_lock_data  Pointer to memory to be used for MDL_LOCK_DATA
+                         object for a lock request.
+   @param mdlkey         Pointer to the buffer for key for the lock request
+                         (should be at least strlen(db) + strlen(name) + 2
+                         bytes, or, if the lengths are not known,
+                         MAX_DBNAME_LENGTH)
+   @param table          Table list element for the table
+
+   @note This is an auxiliary function to be used in cases when we want to
+         access table's description by looking up info in TABLE_SHARE without
+         going through full-blown table open.
+   @note This function assumes that there are no other metadata lock requests
+         in the current metadata locking context.
+
+   @retval FALSE  Success
+   @retval TRUE   Some error occured (probably thread was killed).
+*/
+
+static bool
+acquire_high_prio_shared_mdl_lock(THD *thd, MDL_LOCK_DATA *mdl_lock_data,
+                                  char *mdlkey, TABLE_LIST *table)
+{
+  bool retry;
+
+  mdl_init_lock(mdl_lock_data, mdlkey, 0, table->db, table->table_name);
+  table->mdl_lock_data= mdl_lock_data;
+  mdl_add_lock(&thd->mdl_context, mdl_lock_data);
+  mdl_set_lock_type(mdl_lock_data, MDL_SHARED_HIGH_PRIO);
+
+  while (1)
+  {
+    if (mdl_acquire_shared_lock(&thd->mdl_context, mdl_lock_data, &retry))
+    {
+      if (!retry || mdl_wait_for_locks(&thd->mdl_context))
+      {
+        mdl_remove_all_locks(&thd->mdl_context);
+        return TRUE;
+      }
+      continue;
+    }
+    break;
+  }
+  return FALSE;
+}
+
+
+/**
   @brief          Fill I_S table with data from FRM file only
 
   @param[in]      thd                      thread handler
@@ -3108,7 +3158,6 @@ static int fill_schema_table_from_frm(THD *thd,TABLE *table,
   char db_name_buff[NAME_LEN + 1], table_name_buff[NAME_LEN + 1];
   MDL_LOCK_DATA mdl_lock_data;
   char mdlkey[MAX_DBKEY_LENGTH];
-  bool retry;
 
   bzero((char*) &table_list, sizeof(TABLE_LIST));
   bzero((char*) &tbl, sizeof(TABLE));
@@ -3133,32 +3182,20 @@ static int fill_schema_table_from_frm(THD *thd,TABLE *table,
     table_list.db= db_name->str;
   }
 
-  mdl_init_lock(&mdl_lock_data, mdlkey, 0, db_name->str, table_name->str);
-  table_list.mdl_lock_data= &mdl_lock_data;
-  mdl_add_lock(&thd->mdl_context, &mdl_lock_data);
-  mdl_set_lock_type(&mdl_lock_data, MDL_SHARED_HIGH_PRIO);
-
   /*
     TODO: investigate if in this particular situation we can get by
           simply obtaining internal lock of data-dictionary (ATM it
           is LOCK_open) instead of obtaning full-blown metadata lock.
   */
-  while (1)
+  if (acquire_high_prio_shared_mdl_lock(thd, &mdl_lock_data, mdlkey,
+                                        &table_list))
   {
-    if (mdl_acquire_shared_lock(&thd->mdl_context, &mdl_lock_data, &retry))
-    {
-      if (!retry || mdl_wait_for_locks(&thd->mdl_context))
-      {
-        /*
-          Some error occured or we have been killed while waiting
-          for conflicting locks to go away, let the caller to handle
-          the situation.
-        */
-        return 1;
-      }
-      continue;
-    }
-    break;
+    /*
+      Some error occured (most probably we have been killed while
+      waiting for conflicting locks to go away), let the caller to
+      handle the situation.
+    */
+    return 1;
   }
 
   key_length= create_table_def_key(thd, key, &table_list, 0);
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index dcfc9b811b6..2c91ac69f9f 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -1897,7 +1897,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
         DBUG_RETURN(1);
       pthread_mutex_lock(&LOCK_open);
       for (table= tables; table; table= table->next_local)
-        expel_table_from_cache(0, table->db, table->table_name);
+        tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name);
       pthread_mutex_unlock(&LOCK_open);
     }
     else
@@ -4345,7 +4345,7 @@ static int prepare_for_restore(THD* thd, TABLE_LIST* table,
     if (mdl_acquire_exclusive_locks(&thd->mdl_context))
       DBUG_RETURN(TRUE);
     pthread_mutex_lock(&LOCK_open);
-    expel_table_from_cache(0, table->db, table->table_name);
+    tdc_remove_table(0, TDC_RT_REMOVE_UNUSED, table->db, table->table_name);
     pthread_mutex_unlock(&LOCK_open);
 
     if (my_copy(src_path, dst_path, MYF(MY_WME)))
diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
index 9d47b637886..c8f01a56a72 100644
--- a/sql/sql_trigger.cc
+++ b/sql/sql_trigger.cc
@@ -469,7 +469,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
       goto end;
 
     pthread_mutex_lock(&LOCK_open);
-    expel_table_from_cache(0, tables->db, tables->table_name);
+    tdc_remove_table(thd, TDC_RT_REMOVE_ALL, tables->db, tables->table_name);
 
     if (reopen_name_locked_table(thd, tables))
       goto end_unlock;