MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:kpettersson Date:September 22 2007 10:35am
Subject:bk commit into 5.1 tree (thek:1.2576)
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of thek. When thek does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-09-22 12:35:15+02:00, thek@adventure.(none) +6 -0
  PREVIEW patch for discussion.

  sql/ha_ndbcluster.cc@stripped, 2007-09-22 12:35:12+02:00, thek@adventure.(none) +3 -3
    ndbcluster_drop_database, ndbcluster_alter_tablespace, ndbcluster_log_scema_op
    are all operating on tables which are protected by an exclusive meta data
    lock. This means that they don't need LOCK_open mutex, but still are guarded.

  sql/ha_ndbcluster_binlog.cc@stripped, 2007-09-22 12:35:13+02:00, thek@adventure.(none) +10 -13
    ndbcluster_drop_database, ndbcluster_alter_tablespace, ndbcluster_log_scema
    are all operating on tables which are protected by an exclusive meta data
    lock. This means that they don't need LOCK_open mutex, but still are guarded.
    
    * Removed referenced to LOCK_open.

  sql/lock.cc@stripped, 2007-09-22 12:35:13+02:00, thek@adventure.(none) +11 -14
    * Converted comments into doxygen.

  sql/sql_base.cc@stripped, 2007-09-22 12:35:13+02:00, thek@adventure.(none) +37 -39
    * Converted comments into doxygen.
    * Removed mutex assertion form release_table_share

  sql/sql_rename.cc@stripped, 2007-09-22 12:35:13+02:00, thek@adventure.(none) +1 -11
    * Reduced scope of LOCK_open.

  sql/sql_table.cc@stripped, 2007-09-22 12:35:13+02:00, thek@adventure.(none) +49 -38
    * Reduced scope of LOCK_open by introducing an exclusive lock on the named table object.

diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
--- a/sql/ha_ndbcluster.cc	2007-08-17 10:18:53 +02:00
+++ b/sql/ha_ndbcluster.cc	2007-09-22 12:35:12 +02:00
@@ -6728,7 +6728,7 @@ static void ndbcluster_drop_database(han
   ha_ndbcluster::set_dbname(path, db);
   ndbcluster_log_schema_op(thd, 0,
                            thd->query, thd->query_length,
-                           db, "", 0, 0, SOT_DROP_DB, 0, 0, 0);
+                           db, "", 0, 0, SOT_DROP_DB, 0, 0, 1);
 #endif
   DBUG_VOID_RETURN;
 }
@@ -10067,13 +10067,13 @@ int ndbcluster_alter_tablespace(handlert
                              thd->query, thd->query_length,
                              "", alter_info->tablespace_name,
                              0, 0,
-                             SOT_TABLESPACE, 0, 0, 0);
+                             SOT_TABLESPACE, 0, 0, 1);
   else
     ndbcluster_log_schema_op(thd, 0,
                              thd->query, thd->query_length,
                              "", alter_info->logfile_group_name,
                              0, 0,
-                             SOT_LOGFILE_GROUP, 0, 0, 0);
+                             SOT_LOGFILE_GROUP, 0, 0, 1);
 #endif
   DBUG_RETURN(FALSE);
 
diff -Nrup a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc
--- a/sql/ha_ndbcluster_binlog.cc	2007-08-03 16:54:34 +02:00
+++ b/sql/ha_ndbcluster_binlog.cc	2007-09-22 12:35:13 +02:00
@@ -584,7 +584,7 @@ ndbcluster_binlog_log_query(handlerton *
   {
     ndbcluster_log_schema_op(thd, 0, query, query_length,
                              db, table_name, 0, 0, type,
-                             0, 0, 0);
+                             0, 0, 1);
   }
   DBUG_VOID_RETURN;
 }
@@ -1504,11 +1504,11 @@ end:
 
     int max_timeout= opt_ndb_sync_timeout;
     (void) pthread_mutex_lock(&ndb_schema_object->mutex);
-    if (have_lock_open)
-    {
-      safe_mutex_assert_owner(&LOCK_open);
-      (void) pthread_mutex_unlock(&LOCK_open);
-    }
+    //if (have_lock_open)
+    //{
+    //  safe_mutex_assert_owner(&LOCK_open);
+    //  (void) pthread_mutex_unlock(&LOCK_open);
+    //}
     while (1)
     {
       struct timespec abstime;
@@ -1564,10 +1564,10 @@ end:
                              "distributing", ndb_schema_object->key);
       }
     }
-    if (have_lock_open)
-    {
-      (void) pthread_mutex_lock(&LOCK_open);
-    }
+    //if (have_lock_open)
+    //{
+    //  (void) pthread_mutex_lock(&LOCK_open);
+    //}
     (void) pthread_mutex_unlock(&ndb_schema_object->mutex);
   }
 
@@ -3105,8 +3105,6 @@ ndbcluster_handle_drop_table(Ndb *ndb, c
 #ifdef SYNC_DROP_
   thd->proc_info= "Syncing ndb table schema operation and binlog";
   (void) pthread_mutex_lock(&share->mutex);
-  safe_mutex_assert_owner(&LOCK_open);
-  (void) pthread_mutex_unlock(&LOCK_open);
   int max_timeout= opt_ndb_sync_timeout;
   while (share->op)
   {
@@ -3132,7 +3130,6 @@ ndbcluster_handle_drop_table(Ndb *ndb, c
                            type_str, share->key);
     }
   }
-  (void) pthread_mutex_lock(&LOCK_open);
   (void) pthread_mutex_unlock(&share->mutex);
 #else
   (void) pthread_mutex_lock(&share->mutex);
diff -Nrup a/sql/lock.cc b/sql/lock.cc
--- a/sql/lock.cc	2007-08-21 00:11:42 +02:00
+++ b/sql/lock.cc	2007-09-22 12:35:13 +02:00
@@ -1243,28 +1243,25 @@ is_table_name_exclusively_locked_by_this
   return FALSE;
 }
 
-/*
-  Unlock all tables in list with a name lock
+/**
+  @brief Unlock all tables in list with a name lock.
 
-  SYNOPSIS
-    unlock_table_names()
-    thd			Thread handle
-    table_list		Names of tables to unlock
-    last_table		Don't unlock any tables after this one.
-			(default 0, which will unlock all tables)
+  @param thd Thread handle
+  @param table_list Names of tables to unlock
+  @param last_table Don't unlock any tables after this one
+    (default 0, which will unlock all tables)
 
-  NOTES
-    One must have a lock on LOCK_open when calling this.
+  @note One must have a lock on LOCK_open when calling this.
     This function will broadcast refresh signals to inform other threads
     that the name locks are removed.
 
-  RETURN
-    0	ok
-    1	Fatal error (end of memory ?)
+  @return Status code
+    @retval 0 ok
+    @retval 1 Fatal error (end of memory ?)
 */
 
 void unlock_table_names(THD *thd, TABLE_LIST *table_list,
-			TABLE_LIST *last_table)
+                        TABLE_LIST *last_table)
 {
   DBUG_ENTER("unlock_table_names");
   for (TABLE_LIST *table= table_list;
diff -Nrup a/sql/sql_base.cc b/sql/sql_base.cc
--- a/sql/sql_base.cc	2007-08-21 00:11:42 +02:00
+++ b/sql/sql_base.cc	2007-09-22 12:35:13 +02:00
@@ -533,25 +533,25 @@ static TABLE_SHARE
 }
 
 
-/* 
-   Mark that we are not using table share anymore.
+/**
+   @brief Mark that we are not using table share anymore.
 
-   SYNOPSIS
-     release_table_share()
-     share		Table share
-     release_type	How the release should be done:
-     			RELEASE_NORMAL
-                         - Release without checking
-                        RELEASE_WAIT_FOR_DROP
-                         - Don't return until we get a signal that the
-                           table is deleted or the thread is killed.
-
-   IMPLEMENTATION
-     If ref_count goes to zero and (we have done a refresh or if we have
-     already too many open table shares) then delete the definition.
+   @param share Table share
+   @param release_type How the release should be done
+
+
+   The release_type parameter can be any of:
+    RELEASE_NORMAL
+    - Release without checking
+    RELEASE_WAIT_FOR_DROP
+    - Don't return until we get a signal that the
+    table is deleted or the thread is killed.
+
+    If ref_count goes to zero and (we have done a refresh or if we have
+    already too many open table shares) then delete the definition.
 
-     If type == RELEASE_WAIT_FOR_DROP then don't return until we get a signal
-     that the table is deleted or the thread is killed.
+    If type == RELEASE_WAIT_FOR_DROP then don't return until we get a signal
+    that the table is deleted or the thread is killed.
 */
 
 void release_table_share(TABLE_SHARE *share, enum release_type type)
@@ -563,8 +563,6 @@ void release_table_share(TABLE_SHARE *sh
               (ulong) share, share->db.str, share->table_name.str,
               share->ref_count, share->version));
 
-  safe_mutex_assert_owner(&LOCK_open);
-
   pthread_mutex_lock(&share->mutex);
   if (!--share->ref_count)
   {
@@ -644,17 +642,14 @@ void release_table_share(TABLE_SHARE *sh
 }
 
 
-/*
-  Check if table definition exits in cache
+/**
+  @brief Check if table definition exits in cache.
 
-  SYNOPSIS
-    get_cached_table_share()
-    db			Database name
-    table_name		Table name
+  @param db Database name
+  @param table_name Table name
 
-  RETURN
-    0  Not cached
-    #  TABLE_SHARE for table
+  @return A pointer to the TABLE_SHARE object matching db- and table-name.
+    @retval NULL Table definition is not cached
 */
 
 TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name)
@@ -662,13 +657,12 @@ TABLE_SHARE *get_cached_table_share(cons
   char key[NAME_LEN*2+2];
   TABLE_LIST table_list;
   uint key_length;
-  safe_mutex_assert_owner(&LOCK_open);
 
   table_list.db= (char*) db;
   table_list.table_name= (char*) table_name;
   key_length= create_table_def_key((THD*) 0, key, &table_list, 0);
   return (TABLE_SHARE*) hash_search(&table_def_cache,(uchar*) key, key_length);
-}  
+}
 
 
 /*
@@ -814,7 +808,7 @@ void intern_close_table(TABLE *table)
 
   free_io_cache(table);
   delete table->triggers;
-  if (table->file)                              // Not true if name lock
+  if (table->file) // Not true if name lock
     VOID(closefrm(table, 1));			// close file
   DBUG_VOID_RETURN;
 }
@@ -3154,7 +3148,9 @@ TABLE *drop_locked_tables(THD *thd,const
       else
       {
         /* We already have a name lock, remove copy */
+        pthread_mutex_lock(&LOCK_open);
         VOID(hash_delete(&open_cache,(uchar*) table));
+        pthread_mutex_unlock(&LOCK_open);
       }
     }
     else
@@ -7141,19 +7137,20 @@ void flush_tables()
 }
 
 
-/*
-  Mark all entries with the table as deleted to force an reopen of the table
+/**
+  @brief Mark all entries with the table as deleted to force an reopen of the
+    table
 
   The table will be closed (not stored in cache) by the current thread when
   close_thread_tables() is called.
 
-  PREREQUISITES
-    Lock on LOCK_open()
+  @pre LOCK_open mutex must be locked before calling this function.
 
-  RETURN
-    0  This thread now have exclusive access to this table and no other thread
-       can access the table until close_thread_tables() is called.
-    1  Table is in use by another thread
+  @return Lock status on the target table
+    @retval 0  This thread now have exclusive access to this table and no other
+               thread can access the table until close_thread_tables() is
+               called.
+    @retval 1  Table is in use by another thread.
 */
 
 bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
@@ -7166,6 +7163,7 @@ bool remove_table_from_cache(THD *thd, c
   bool result= 0, signalled= 0;
   DBUG_ENTER("remove_table_from_cache");
   DBUG_PRINT("enter", ("Table: '%s.%s'  flags: %u", db, table_name, flags));
+
 
   key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
   for (;;)
diff -Nrup a/sql/sql_rename.cc b/sql/sql_rename.cc
--- a/sql/sql_rename.cc	2007-07-27 02:26:40 +02:00
+++ b/sql/sql_rename.cc	2007-09-22 12:35:13 +02:00
@@ -137,6 +137,7 @@ bool mysql_rename_tables(THD *thd, TABLE
     pthread_mutex_unlock(&LOCK_open);
     goto err;
   }
+  pthread_mutex_unlock(&LOCK_open);
 
   error=0;
   if ((ren_table=rename_tables(thd,table_list,0)))
@@ -160,17 +161,6 @@ bool mysql_rename_tables(THD *thd, TABLE
 
     error= 1;
   }
-  /*
-    An exclusive lock on table names is satisfactory to ensure
-    no other thread accesses this table.
-    However, NDB assumes that handler::rename_tables is called under
-    LOCK_open. And it indeed is, from ALTER TABLE.
-    TODO: remove this limitation.
-    We still should unlock LOCK_open as early as possible, to provide
-    higher concurrency - query_cache_invalidate can take minutes to
-    complete.
-  */
-  pthread_mutex_unlock(&LOCK_open);
 
   /* Lets hope this doesn't fail as the result will be messy */ 
   if (!silent && !error)
diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc
--- a/sql/sql_table.cc	2007-08-15 23:48:53 +02:00
+++ b/sql/sql_table.cc	2007-09-22 12:35:13 +02:00
@@ -1522,6 +1522,17 @@ int mysql_rm_table_part2(THD *thd, TABLE
   }
 
   pthread_mutex_lock(&LOCK_open);
+  if (!drop_temporary && lock_table_names_exclusively(thd, tables))
+    {
+      pthread_mutex_unlock(&LOCK_open);
+      DBUG_RETURN(1);
+    }
+
+  /* 
+    Tables are now upgraded with an exclusive name lock so we can release
+    the singleton mutex.
+  */
+  pthread_mutex_unlock(&LOCK_open);
 
   /*
     If we have the table in the definition cache, we don't have to check the
@@ -1542,16 +1553,10 @@ int mysql_rm_table_part2(THD *thd, TABLE
                            table->table_name_length, table->table_name, 1))
     {
       my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
-      pthread_mutex_unlock(&LOCK_open);
       DBUG_RETURN(1);
     }
   }
 
-  if (!drop_temporary && lock_table_names_exclusively(thd, tables))
-  {
-    pthread_mutex_unlock(&LOCK_open);
-    DBUG_RETURN(1);
-  }
 
   /* Don't give warnings for not found errors, as we already generate notes */
   thd->no_warnings_for_error= 1;
@@ -1562,7 +1567,13 @@ int mysql_rm_table_part2(THD *thd, TABLE
     handlerton *table_type;
     enum legacy_db_type frm_db_type;
 
-    mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, 1);
+    /* 
+      mysql_ha_flush will aquire LOCK_open mutex because
+      it manipulates handler_tables_hash and because it
+      calls close_thread_table.
+    */
+    mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, 0);
+
     if (!close_temporary_table(thd, table))
     {
       tmp_table_deleted=1;
@@ -1599,9 +1610,16 @@ int mysql_rm_table_part2(THD *thd, TABLE
     {
       TABLE *locked_table;
       abort_locked_tables(thd, db, table->table_name);
+
+      /*
+        We need to manipulate the open_cache container;
+        - Lock the LOCK_open mutex.
+      */
+      pthread_mutex_lock(&LOCK_open);
       remove_table_from_cache(thd, db, table->table_name,
-	                      RTFC_WAIT_OTHER_THREAD_FLAG |
-			      RTFC_CHECK_KILLED_FLAG);
+                              RTFC_WAIT_OTHER_THREAD_FLAG |
+                              RTFC_CHECK_KILLED_FLAG);
+      pthread_mutex_unlock(&LOCK_open);
       /*
         If the table was used in lock tables, remember it so that
         unlock_table_names can free it
@@ -1619,6 +1637,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
       path_length= build_table_filename(path, sizeof(path),
                                         db, alias, reg_ext, 0);
     }
+
     if (drop_temporary ||
         (table_type == NULL &&        
          (access(path, F_OK) &&
@@ -1675,11 +1694,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
       wrong_tables.append(String(table->table_name,system_charset_info));
     }
   }
-  /*
-    It's safe to unlock LOCK_open: we have an exclusive lock
-    on the table name.
-  */
-  pthread_mutex_unlock(&LOCK_open);
+
   thd->thread_specific_used|= tmp_table_deleted;
   error= 0;
   if (wrong_tables.length())
@@ -1739,8 +1754,8 @@ int mysql_rm_table_part2(THD *thd, TABLE
       */
     }
   }
-  pthread_mutex_lock(&LOCK_open);
 err_with_placeholders:
+  pthread_mutex_lock(&LOCK_open);
   unlock_table_names(thd, tables, (TABLE_LIST*) 0);
   pthread_mutex_unlock(&LOCK_open);
   thd->no_warnings_for_error= 0;
@@ -3671,22 +3686,18 @@ mysql_rename_table(handlerton *base, con
 }
 
 
-/*
-  Force all other threads to stop using the table
+/**
+  @brief Force all other threads to stop using the table
+
+  @param thd Thread handler
+  @param table Table to remove from cache
+  @param function HA_EXTRA_PREPARE_FOR_DELETE if table is to be deleted
+    HA_EXTRA_FORCE_REOPEN if table is not be used
 
-  SYNOPSIS
-    wait_while_table_is_used()
-    thd			Thread handler
-    table		Table to remove from cache
-    function		HA_EXTRA_PREPARE_FOR_DELETE if table is to be deleted
-			HA_EXTRA_FORCE_REOPEN if table is not be used
-  NOTES
    When returning, the table will be unusable for other threads until
    the table is closed.
 
-  PREREQUISITES
-    Lock on LOCK_open
-    Win32 clients must also have a WRITE LOCK on the table !
+  @pre Lock on LOCK_open.
 */
 
 static void wait_while_table_is_used(THD *thd,TABLE *table,
@@ -5783,19 +5794,22 @@ bool mysql_alter_table(THD *thd,char *ne
     if (thd->locked_tables || thd->active_transaction())
     {
       my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
-                 ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
+                ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
       DBUG_RETURN(TRUE);
     }
 
     if (wait_if_global_read_lock(thd,0,1))
       DBUG_RETURN(TRUE);
     VOID(pthread_mutex_lock(&LOCK_open));
-    if (lock_table_names(thd, table_list))
+    if (lock_table_names_exclusively(thd, table_list))
     {
       error= 1;
-      goto view_err;
+      pthread_mutex_unlock(&LOCK_open);
+      start_waiting_global_read_lock(thd);
+      DBUG_RETURN(error);
     }
-    
+    pthread_mutex_unlock(&LOCK_open);
+
     if (!do_rename(thd, table_list, new_db, new_name, new_name, 1))
     {
       if (mysql_bin_log.is_open())
@@ -5807,12 +5821,9 @@ bool mysql_alter_table(THD *thd,char *ne
       send_ok(thd);
     }
 
+    pthread_mutex_lock(&LOCK_open);
     unlock_table_names(thd, table_list, (TABLE_LIST*) 0);
-
-view_err:
     pthread_mutex_unlock(&LOCK_open);
-    start_waiting_global_read_lock(thd);
-    DBUG_RETURN(error);
   }
   if (!(table=open_ltable(thd, table_list, TL_WRITE_ALLOW_READ, 0)))
     DBUG_RETURN(TRUE);
@@ -5924,7 +5935,7 @@ view_err:
     my_error(ER_ILLEGAL_HA, MYF(0), table_name);
     goto err;
   }
-  
+
   thd->proc_info="setup";
   if (!(alter_info->flags & ~(ALTER_RENAME | ALTER_KEYS_ONOFF)) &&
       !table->s->tmp_table) // no need to touch frm
@@ -5965,8 +5976,8 @@ view_err:
     {
       error= 0;
       push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
-			  ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA),
-			  table->alias);
+                          ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA),
+                          table->alias);
     }
 
     VOID(pthread_mutex_lock(&LOCK_open));
Thread
bk commit into 5.1 tree (thek:1.2576)kpettersson22 Sep