List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:May 28 2008 8:16am
Subject:commit into mysql-6.0 branch (dlenev:2652) WL#3726
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-6.0-3726-w/

 2652 Dmitry Lenev	2008-05-28
      WL#3726 "DDL locking for all metadata objects".
      
      After review fixes in progress. Removed unused code and
      adjusted names of functions/methods to better reflect
      their current function.
modified:
  sql/mysql_priv.h
  sql/sql_base.cc
  sql/sql_handler.cc
  sql/sql_insert.cc
  sql/sql_partition.cc
  sql/sql_table.cc
  sql/sql_trigger.cc
  sql/table.h

per-file comments:
  sql/mysql_priv.h
    Changed names of close_data_files_and_morph_locks() and
    close_handle_and_leave_table_as_lock() to better reflect
    their current function (locking is now responsibility
    of metadata locking subsystem).
  sql/sql_base.cc
    Changed names of close_data_files_and_morph_locks() and
    close_handle_and_leave_table_as_lock() to better reflect
    their current function (locking is now responsibility
    of metadata locking subsystem). Also adjusted comments
    describing these functions.
    Got rid of TABLE::open_placeholder since it is no longer
    used (its value is never read anywhere).
    TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
    since we no longer use name-locks.
  sql/sql_handler.cc
    TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
    since we no longer use name-locks.
  sql/sql_insert.cc
    TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
    since we no longer use name-locks.
  sql/sql_partition.cc
    Changed name of close_data_files_and_morph_locks() to
    better reflect its current function (locking is now
    responsibility of metadata locking subsystem).
  sql/sql_table.cc
    Changed names of close_data_files_and_morph_locks() and
    close_handle_and_leave_table_as_lock() to better reflect
    their current function (locking is now responsibility
    of metadata locking subsystem).
    Got rid of TABLE::open_placeholder since it is no longer
    used.
  sql/sql_trigger.cc
    Changed name of close_data_files_and_morph_locks() to
    better reflect its current function (locking is now
    responsibility of metadata locking subsystem).
  sql/table.h
    Got rid of TABLE::open_placeholder which is no longer used
    altough its value was set in several places no code reads it).
    Removed unused TABLE::is_name_opened() method.
    Finally TABLE::needs_reopen_or_name_lock() was renamed to
    needs_reopen() since we no longer use name-locks.

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2008-05-28 08:07:30 +0000
+++ b/sql/mysql_priv.h	2008-05-28 08:16:03 +0000
@@ -1335,9 +1335,9 @@
                           TABLE_LIST *new_child_list, TABLE_LIST **new_last);
 bool reopen_table(TABLE *table);
 bool reopen_tables(THD *thd, bool get_locks);
-void close_data_files_and_morph_locks(THD *thd, const char *db,
-                                      const char *table_name);
-void close_handle_and_leave_table_as_lock(TABLE *table);
+void close_data_files_and_leave_as_placeholders(THD *thd, const char *db,
+                                                const char *table_name);
+void close_handle_and_leave_table_as_placeholder(TABLE *table);
 bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias,
                   uint db_stat, uint prgflag,
                   uint ha_open_flags, TABLE *outparam,

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2008-05-28 08:07:30 +0000
+++ b/sql/sql_base.cc	2008-05-28 08:16:03 +0000
@@ -702,25 +702,23 @@
 }
 
 
-/*
-  Close file handle, but leave the table in the table cache
-
-  SYNOPSIS
-    close_handle_and_leave_table_as_lock()
-    table		Table handler
-
-  NOTES
-    By leaving the table in the table cache, it disallows any other thread
-    to open the table
-
-    thd->killed will be set if we run out of memory
-
-    If closing a MERGE child, the calling function has to take care for
-    closing the parent too, if necessary.
+/**
+   Close file handle, but leave the table in THD::open_tables list
+   to allow its future reopening.
+
+   @param table  Table handler
+
+   @note THD::killed will be set if we run out of memory
+
+   @note If closing a MERGE child, the calling function has to
+         take care for closing the parent too, if necessary.
+
+   @todo Get rid of this function once we refactor LOCK TABLES
+         to keep around TABLE_LIST elements used for opening
+         of tables until UNLOCK TABLES.
 */
 
-
-void close_handle_and_leave_table_as_lock(TABLE *table)
+void close_handle_and_leave_table_as_placeholder(TABLE *table)
 {
   TABLE_SHARE *share, *old_share= table->s;
   char *key_buff;
@@ -1024,8 +1022,8 @@
           char dbname[NAME_LEN+1];
           char tname[NAME_LEN+1];
           /*
-            Since close_data_files_and_morph_locks() frees share's memroot
-            we need to make copies of database and table names.
+            Since close_data_files_and_leave_as_placeholders() frees share's
+            memroot we need to make copies of database and table names.
           */
           strmov(dbname, tab->s->db.str);
           strmov(tname, tab->s->table_name.str);
@@ -1035,7 +1033,7 @@
             goto err_with_reopen;
           }
           pthread_mutex_lock(&LOCK_open);
-          close_data_files_and_morph_locks(thd, dbname, tname);
+          close_data_files_and_leave_as_placeholders(thd, dbname, tname);
           pthread_mutex_unlock(&LOCK_open);
         }
       }
@@ -1058,7 +1056,8 @@
             goto err_with_reopen;
           }
           pthread_mutex_lock(&LOCK_open);
-          close_data_files_and_morph_locks(thd, table->db, table->table_name);
+          close_data_files_and_leave_as_placeholders(thd, table->db,
+                                                     table->table_name);
           pthread_mutex_unlock(&LOCK_open);
         }
       }
@@ -1482,7 +1481,7 @@
     detach_merge_children(table, TRUE);
 
   table->mdl_lock= 0;
-  if (table->needs_reopen_or_name_lock() ||
+  if (table->needs_reopen() ||
       thd->version != refresh_version || !table->db_stat)
   {
     free_cache_entry(table);
@@ -1490,12 +1489,6 @@
   }
   else
   {
-    /*
-      Open placeholders have TABLE::db_stat set to 0, so they should be
-      handled by the first alternative.
-    */
-    DBUG_ASSERT(!table->open_placeholder);
-
     /* Assert that MERGE children are not attached in unused_tables. */
     DBUG_ASSERT(!table->is_children_attached());
 
@@ -3195,7 +3188,7 @@
 
 /**
     Close all instances of a table open by this thread and replace
-    them with exclusive name-locks.
+    them with placeholder in THD::open_tables list for future reopening.
 
     @param thd        Thread context
     @param db         Database name for the table to be closed
@@ -3210,11 +3203,11 @@
           the strings are used in a loop even after the share may be freed.
 */
 
-void close_data_files_and_morph_locks(THD *thd, const char *db,
-                                      const char *table_name)
+void close_data_files_and_leave_as_placeholders(THD *thd, const char *db,
+                                               const char *table_name)
 {
   TABLE *table;
-  DBUG_ENTER("close_data_files_and_morph_locks");
+  DBUG_ENTER("close_data_files_and_leave_as_placeholders");
 
   safe_mutex_assert_owner(&LOCK_open);
 
@@ -3228,11 +3221,6 @@
     thd->lock= 0;
   }
 
-  /*
-    Note that open table list may contain a name-lock placeholder
-    for target table name if we process ALTER TABLE ... RENAME.
-    So loop below makes sense even if we are not under LOCK TABLES.
-  */
   for (table=thd->open_tables; table ; table=table->next)
   {
     if (!strcmp(table->s->table_name.str, table_name) &&
@@ -3249,15 +3237,13 @@
             won't have multiple children with the same db.table_name.
           */
           mysql_lock_remove(thd, thd->locked_tables, table->parent, TRUE);
-          table->parent->open_placeholder= 1;
-          close_handle_and_leave_table_as_lock(table->parent);
+          close_handle_and_leave_table_as_placeholder(table->parent);
         }
         else
           mysql_lock_remove(thd, thd->locked_tables, table, TRUE);
       }
-      table->open_placeholder= 1;
       table->s->version= 0;
-      close_handle_and_leave_table_as_lock(table);
+      close_handle_and_leave_table_as_placeholder(table);
     }
   }
   DBUG_VOID_RETURN;

=== modified file 'sql/sql_handler.cc'
--- a/sql/sql_handler.cc	2008-05-27 17:31:53 +0000
+++ b/sql/sql_handler.cc	2008-05-28 08:16:03 +0000
@@ -781,7 +781,7 @@
     if (hash_tables->table &&
         (hash_tables->table->mdl_lock &&
          mdl_has_pending_conflicting_lock(hash_tables->table->mdl_lock) ||
-         hash_tables->table->needs_reopen_or_name_lock()))
+         hash_tables->table->needs_reopen()))
     {
       mysql_ha_close_table(thd, hash_tables);
       /* Mark table as closed, ready for re-open. */

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2008-05-27 17:31:53 +0000
+++ b/sql/sql_insert.cc	2008-05-28 08:16:03 +0000
@@ -2581,7 +2581,7 @@
 
   thd_proc_info(&thd, "insert");
   max_rows= delayed_insert_limit;
-  if (thd.killed || table->needs_reopen_or_name_lock())
+  if (thd.killed || table->needs_reopen())
   {
     thd.killed= THD::KILL_CONNECTION;
     max_rows= ULONG_MAX;                     // Do as much as possible

=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc	2008-05-23 13:54:03 +0000
+++ b/sql/sql_partition.cc	2008-05-28 08:16:03 +0000
@@ -5828,7 +5828,7 @@
     and we set db_stat to zero to ensure we don't close twice.
   */
   pthread_mutex_lock(&LOCK_open);
-  close_data_files_and_morph_locks(thd, db, table_name);
+  close_data_files_and_leave_as_placeholders(thd, db, table_name);
   pthread_mutex_unlock(&LOCK_open);
   DBUG_RETURN(0);
 }

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2008-05-28 08:07:30 +0000
+++ b/sql/sql_table.cc	2008-05-28 08:16:03 +0000
@@ -5778,9 +5778,8 @@
   pthread_mutex_lock(&LOCK_open);
   alter_table_manage_keys(table, table->file->indexes_are_disabled(),
                           keys_onoff);
-  close_data_files_and_morph_locks(thd,
-                                   table->pos_in_table_list->db,
-                                   table->pos_in_table_list->table_name);
+  close_data_files_and_leave_as_placeholders(thd, table->pos_in_table_list->db,
+                                          table->pos_in_table_list->table_name);
   if (mysql_rename_table(NULL,
 			 altered_table->s->db.str,
                          altered_table->s->table_name.str,
@@ -5832,9 +5831,8 @@
       object to make it suitable for reopening.
     */
     DBUG_ASSERT(t_table == table);
-    table->open_placeholder= 1;
     pthread_mutex_lock(&LOCK_open);
-    close_handle_and_leave_table_as_lock(table);
+    close_handle_and_leave_table_as_placeholder(table);
     pthread_mutex_unlock(&LOCK_open);
   }
 
@@ -7005,7 +7003,7 @@
 
   pthread_mutex_lock(&LOCK_open);
 
-  close_data_files_and_morph_locks(thd, db, table_name);
+  close_data_files_and_leave_as_placeholders(thd, db, table_name);
 
   error=0;
   save_old_db_type= old_db_type;

=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc	2008-05-23 13:54:03 +0000
+++ b/sql/sql_trigger.cc	2008-05-28 08:16:03 +0000
@@ -487,7 +487,8 @@
   if (!result && thd->locked_tables)
   {
     /* Make table suitable for reopening */
-    close_data_files_and_morph_locks(thd, tables->db, tables->table_name);
+    close_data_files_and_leave_as_placeholders(thd, tables->db,
+                                               tables->table_name);
     thd->in_lock_tables= 1;
     if (reopen_tables(thd, 1))
     {

=== modified file 'sql/table.h'
--- a/sql/table.h	2008-05-27 12:15:44 +0000
+++ b/sql/table.h	2008-05-28 08:16:03 +0000
@@ -722,24 +722,6 @@
   my_bool force_index;
   my_bool distinct,const_table,no_rows;
   my_bool key_read, no_keyread;
-  /*
-    Placeholder for an open table which prevents other connections
-    from taking name-locks on this table. Typically used with
-    TABLE_SHARE::version member to take an exclusive name-lock on
-    this table name -- a name lock that not only prevents other
-    threads from opening the table, but also blocks other name
-    locks. This is achieved by:
-    - setting open_placeholder to 1 - this will block other name
-      locks, as wait_for_locked_table_name will be forced to wait,
-      see table_is_used for details.
-    - setting version to 0 - this will force other threads to close
-      the instance of this table and wait (this is the same approach
-      as used for usual name locks).
-    An exclusively name-locked table currently can have no handler
-    object associated with it (db_stat is always 0), but please do
-    not rely on that.
-  */
-  my_bool open_placeholder;
   my_bool locked_by_logger;
   my_bool no_replicate;
   my_bool locked_by_name;
@@ -803,12 +785,10 @@
     read_set= &def_read_set;
     write_set= &def_write_set;
   }
-  /* Is table open or should be treated as such by name-locking? */
-  inline bool is_name_opened() { return db_stat || open_placeholder; }
   /*
-    Is this instance of the table should be reopen or represents a name-lock?
+    Is this instance of the table should be reopen?
   */
-  inline bool needs_reopen_or_name_lock()
+  inline bool needs_reopen()
   { return s->version != refresh_version; }
   bool is_children_attached(void);
 };

Thread
commit into mysql-6.0 branch (dlenev:2652) WL#3726Dmitry Lenev28 May