List:Commits« Previous MessageNext Message »
From:kpettersson Date:June 14 2007 6:07pm
Subject:bk commit into 5.1 tree (thek:1.2541) BUG#21074
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-06-14 18:07:50+02:00, thek@adventure.(none) +8 -0
  Bug#21074 Large query_cache freezes mysql server sporadically under heavy load
  
  Invaldating a subset of a sufficiently large query cache can take a long time.
  During this time the server is efficiently frozen and no other operation can
  be executed. This patch addresses this problem by moving the locks which cause
  the freezing and also by temporarily disable the query cache while the 
  invalidation takes place.

  sql/lock.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +82 -0
    - Added function for acquiring table name exclusive locks.
    - Added function for asserting that table name lock is acquired.

  sql/mysql_priv.h@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +3 -0
    - Added function for acquiring table name exclusive locks.
    - Added function for asserting that table name lock is acquired.

  sql/sql_cache.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +209 -167
    - Changed flush_in_progress-flag into a state and added a function, 
      is_flushing() to reflect on this state. A new state was needed to indicate
      that a partial invalidation was in progress.
    - An unused parameter 'under_guard' was removed.
    - The Query_cache mutex structural_guard was pushed down into one
      invalidate_table function to avoid multiple entry points which makes
      maintainens more difficult.
    - Instead of keeping the structural_guard mutex during the entire invalidation
      we set the query cache status state to TABLE_FLUSH_IN_PROGRESS to
      temporarily disable the cache and avoid locking other threads needing the
      the Query_cache resource.

  sql/sql_cache.h@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +12 -7
    - Changed flush_in_progress-flag into a state and added a function, 
      is_flushing() to reflect on this state. A new state was needed to indicate
      that a partial invalidation was in progress.
    - An unused parameter 'under_guard' was removed.
    - The Query_cache mutex structural_guard was pushed down into one
      invalidate_table function to avoid multiple entry points which makes
      maintainens more difficult.
    - Instead of keeping the structural_guard mutex during the entire invalidation
      we set the query cache status state to TABLE_FLUSH_IN_PROGRESS to
      temporarily disable the cache and avoid locking other threads needing the
      the Query_cache resource.

  sql/sql_parse.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +1 -1
    - Function query_cache_invalidation3 isn't protect by a lock and we have a 
      race condition.
    - Moving this function into mysql_rename_tables and make sure it is protected
      by a exclusive table name lock.

  sql/sql_rename.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +12 -4
    - Function query_cache_invalidation3 isn't protect by a lock and we have a 
      race condition.
    - Moving this function into mysql_rename_tables and make sure it is protected
      by a exclusive table name lock.
    - Instead of using LOCK_open mutex, which excludes all other threads, the lock
      is changed into exclusive table name locks instead. This prevents us from
      locking the server if a query cache invalidation would take a long time to
      complete.

  sql/sql_table.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +12 -12
    - Instead of using LOCK_open mutex, which excludes all other threads, the lock
      is changed into exclusive table name locks instead. This prevents us from
      locking the server if a query cache invalidation would take a long time to
      complete.

  sql/sql_trigger.cc@stripped, 2007-06-14 18:07:47+02:00, thek@adventure.(none) +27 -18
    - Table_triggers don't need to be protexted by LOCK_open mutex. This 
      patch cancel this restriction.
    - Refactored comments to doxygen style.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	thek
# Host:	adventure.(none)
# Root:	/home/thek/Development/cpp/bug21074/my51-bug21074

--- 1.108/sql/lock.cc	2007-05-24 12:24:28 +02:00
+++ 1.109/sql/lock.cc	2007-06-14 18:07:47 +02:00
@@ -1026,6 +1026,88 @@ end:
   return 1;
 }
 
+/**
+  @brief Lock all tables in list with an exclusive table name lock.
+
+  @param thd Thread handle.
+  @param table_list Names of tables to lock.
+
+  @note This function needs to be protected by LOCK_open.
+
+  @see lock_table_names
+  @see unlock_table_names
+
+  @retval TRUE An error occured.
+  @retval FALSE Name lock successfully acquired.
+*/
+bool lock_table_names_exclusively(THD *thd, TABLE_LIST *table_list)
+{
+  if (lock_table_names(thd, table_list))
+    return TRUE;
+
+  /*
+    Upgrade the table name locks from semi-exclusive to exclusive locks.
+  */
+  for (TABLE_LIST *table= table_list; table; table=table->next_global)
+  {
+    if (table->table)
+      table->table->open_placeholder= 1;
+  }
+  return FALSE;
+}
+
+/**
+  @brief Test is 'table' is protected by an exclusive name lock.
+  
+  @param thd The current thread handler.
+  @param table Table container containing the single table to be tested.
+
+  @note Needs to be protected by LOCK_open mutex.
+
+  @retval TRUE Table is protected.
+  @retval FALSE Table is not protected.
+*/
+bool is_table_name_exclusively_locked_by_this_thread(THD *thd, TABLE_LIST *table_list)
+{
+  char  key[MAX_DBKEY_LENGTH];
+  uint  key_length;
+  HASH_SEARCH_STATE state;
+
+  key_length= create_table_def_key(thd, key, table_list, 0);
+
+  return is_table_name_exclusively_locked_by_this_thread(thd, (uchar *)&key[0],
key_length);
+}
+
+/**
+  @brief Test is 'table key' is protected by an exclusive name lock.
+  @see is_table_name_exclusively_locked_by_this_thread(THD *thd, TABLE_LIST *table_list)
+  @param thd The current thread handler.
+  @param table Table container containing the single table to be tested.
+
+  @note Needs to be protected by LOCK_open mutex.
+
+  @retval TRUE Table is protected.
+  @retval FALSE Table is not protected.
+ */
+bool is_table_name_exclusively_locked_by_this_thread(THD *thd, uchar *key, int
key_length)
+{
+  HASH_SEARCH_STATE state;
+  TABLE *table;
+
+  /* Only insert the table if we haven't insert it already */
+  for (table=(TABLE*) hash_first(&open_cache, (uchar*)key,
+       key_length, &state);
+       table ;
+       table = (TABLE*) hash_next(&open_cache,(uchar*) key,
+                key_length, &state))
+  {
+    if (table->in_use == thd && table->open_placeholder == 1
+        && table->s->version == 0)
+      return TRUE;
+  }
+
+  return FALSE;
+}
 
 /*
   Unlock all tables in list with a name lock

--- 1.511/sql/mysql_priv.h	2007-06-01 09:43:52 +02:00
+++ 1.512/sql/mysql_priv.h	2007-06-14 18:07:47 +02:00
@@ -1799,6 +1799,9 @@ bool wait_for_locked_table_names(THD *th
 bool lock_table_names(THD *thd, TABLE_LIST *table_list);
 void unlock_table_names(THD *thd, TABLE_LIST *table_list,
 			TABLE_LIST *last_table);
+bool lock_table_names_exclusively(THD *thd, TABLE_LIST *table_list);
+bool is_table_name_exclusively_locked_by_this_thread(THD *thd, TABLE_LIST *table_list);
+bool is_table_name_exclusively_locked_by_this_thread(THD *thd, uchar *key, int
key_length);
 
 
 /* old unireg functions */

--- 1.112/sql/sql_cache.cc	2007-06-01 10:12:02 +02:00
+++ 1.113/sql/sql_cache.cc	2007-06-14 18:07:47 +02:00
@@ -268,6 +268,33 @@ are stored in one block.
 
 If join_results allocated new block(s) then we need call pack_cache again.
 
+7. Interface
+The query cache interfaces with the rest of the server code through 7 functions:
+ 1. Query_cache::send_result_to_client
+       - Called before parsing and used to match a statement with the stored queries
hash.
+         If a match is found the cached result set is sent through repeated calls to 
+         net_real_write. (note: calling thread doesn't have a registered result set
writer:
+         thd->net.query_cache_query=0)
+ 2. Query_cache::store_query
+       - Called just before handle_select() and is used to register a result set writer
to the
+         statement currently being processed (thd->net.query_cache_query).
+ 3. query_cache_insert
+       - Called from net_real_write to append a result set to a cached query if (and only
if)
+         this query has a registered result set writer (thd->net.query_cache_query).
+ 4. Query_cache::invalidate
+       - Called from various places to invalidate query cache based on database, table
+         and myisam file name. During an on going invalidation the query cache is
temporarily
+         disabled.
+ 5. Query_cache::flush
+       - Used when a RESET QUERY CACHE is issued. This clears the entire cache block by
block.
+ 6. Query_cache::resize
+       - Used to change the available memory used by the query cache. This will also
invalidate
+         the entrie query cache in one free operation.
+ 7. Query_cache::pack
+       - Used when a FLUSH QUERY CACHE is issued. This changes the order of the used
memory blocks
+         in physical memory order and move all available memory to the 'bottom' of the
memory.
+
+
 TODO list:
 
   - Delayed till after-parsing qache answer (for column rights processing)
@@ -616,8 +643,8 @@ void query_cache_insert(NET *net, const 
 
   STRUCT_LOCK(&query_cache.structure_guard_mutex);
 
-  if (unlikely(query_cache.query_cache_size == 0 ||
-               query_cache.flush_in_progress))
+  if (unlikely(query_cache.query_cache_size == 0 
+      || query_cache.m_cache_status == Query_cache::FLUSH_IN_PROGRESS))
   {
     STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
     DBUG_VOID_RETURN;
@@ -627,6 +654,26 @@ void query_cache_insert(NET *net, const 
 				    net->query_cache_query);
   if (query_block)
   {
+    /*
+       If query cache is invalidating at the same time as we
+       are gathering a result set we need to wait for the table
+       flush to finish and then cancel this operation and
+       finally free the query or we might end up with
+       broken result sets in the query cache.
+    */
+    if (query_cache.m_cache_status == Query_cache::TABLE_FLUSH_IN_PROGRESS)
+    {
+      while (query_cache.is_flushing())
+       
pthread_cond_wait(&query_cache.COND_cache_status_changed,&query_cache.structure_guard_mutex);
+      /* If writer was dropped due to invalidation we don't need to continue. */
+      if (net->query_cache_query == 0 )
+      {
+        STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
+        DBUG_VOID_RETURN;
+      }
+      /* We still got the writer so we can continue to write the remaining result set. */
+    }
+
     Query_cache_query *header = query_block->query();
     Query_cache_block *result = header->result();
 
@@ -673,7 +720,7 @@ void query_cache_abort(NET *net)
   STRUCT_LOCK(&query_cache.structure_guard_mutex);
 
   if (unlikely(query_cache.query_cache_size == 0 ||
-               query_cache.flush_in_progress))
+               query_cache.is_flushing()))
   {
     STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
     DBUG_VOID_RETURN;
@@ -714,7 +761,7 @@ void query_cache_end_of_result(THD *thd)
   STRUCT_LOCK(&query_cache.structure_guard_mutex);
 
   if (unlikely(query_cache.query_cache_size == 0 ||
-               query_cache.flush_in_progress))
+               query_cache.is_flushing()))
     goto end;
 
   query_block= ((Query_cache_block*) thd->net.query_cache_query);
@@ -801,9 +848,9 @@ ulong Query_cache::resize(ulong query_ca
   DBUG_ASSERT(initialized);
 
   STRUCT_LOCK(&structure_guard_mutex);
-  while (flush_in_progress)
-    pthread_cond_wait(&COND_flush_finished, &structure_guard_mutex);
-  flush_in_progress= TRUE;
+  while (is_flushing())
+    pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex);
+  m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
   STRUCT_UNLOCK(&structure_guard_mutex);
 
   free_cache();
@@ -814,8 +861,8 @@ ulong Query_cache::resize(ulong query_ca
   DBUG_EXECUTE("check_querycache",check_integrity(0););
 
   STRUCT_LOCK(&structure_guard_mutex);
-  flush_in_progress= FALSE;
-  pthread_cond_signal(&COND_flush_finished);
+  m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+  pthread_cond_signal(&COND_cache_status_changed);
   STRUCT_UNLOCK(&structure_guard_mutex);
 
   DBUG_RETURN(new_query_cache_size);
@@ -910,7 +957,7 @@ def_week_frmt: %lu",                    
     ha_release_temporary_latches(thd);
 
     STRUCT_LOCK(&structure_guard_mutex);
-    if (query_cache_size == 0 || flush_in_progress)
+    if (query_cache_size == 0 || is_flushing())
     {
       STRUCT_UNLOCK(&structure_guard_mutex);
       DBUG_VOID_RETURN;
@@ -954,7 +1001,7 @@ def_week_frmt: %lu",                    
       Query_cache_block *query_block;
       query_block= write_block_data(tot_length, (uchar*) thd->query,
 				    ALIGN_SIZE(sizeof(Query_cache_query)),
-				    Query_cache_block::QUERY, local_tables, 1);
+				    Query_cache_block::QUERY, local_tables);
       if (query_block != 0)
       {
 	DBUG_PRINT("qcache", ("query block 0x%lx allocated, %lu",
@@ -1088,13 +1135,23 @@ Query_cache::send_result_to_client(THD *
   }
 
   STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size == 0 || flush_in_progress)
+
+  if (query_cache_size == 0 )
+  {
+    DBUG_PRINT("qcache",("query cache disable"));
+    goto err_unlock;
+  }
+
+  if (is_flushing())
   {
-    DBUG_PRINT("qcache", ("query cache disabled"));
+    /* Return; Query cache is temporarily disabled while we flush. */
+    DBUG_PRINT("qcache",("query cache disabled"));
     goto err_unlock;
   }
 
-  /* Check that we haven't forgot to reset the query cache variables */
+  /* Check that we haven't forgot to reset the query cache variables;
+     make sure there are no attached query cache writer to this thread.
+   */
   DBUG_ASSERT(thd->net.query_cache_query == 0);
 
   Query_cache_block *query_block;
@@ -1330,32 +1387,26 @@ void Query_cache::invalidate(THD *thd, T
 			     my_bool using_transactions)
 {
   DBUG_ENTER("Query_cache::invalidate (table list)");
-  STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size > 0 && !flush_in_progress)
-  {
-    DUMP(this);
 
-    using_transactions= using_transactions &&
-      (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
-    for (; tables_used; tables_used= tables_used->next_local)
-    {
-      DBUG_ASSERT(!using_transactions || tables_used->table!=0);
-      if (tables_used->derived)
-        continue;
-      if (using_transactions &&
-         (tables_used->table->file->table_cache_type() ==
-          HA_CACHE_TBL_TRANSACT))
-        /*
-           Tables_used->table can't be 0 in transaction.
-           Only 'drop' invalidate not opened table, but 'drop'
-           force transaction finish.
-        */
-        thd->add_changed_table(tables_used->table);
-      else
-        invalidate_table(tables_used);
-    }
+  using_transactions= using_transactions &&
+    (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
+  for (; tables_used; tables_used= tables_used->next_local)
+  {
+    DBUG_ASSERT(!using_transactions || tables_used->table!=0);
+    if (tables_used->derived)
+      continue;
+    if (using_transactions &&
+        (tables_used->table->file->table_cache_type() ==
+        HA_CACHE_TBL_TRANSACT))
+      /*
+          Tables_used->table can't be 0 in transaction.
+          Only 'drop' invalidate not opened table, but 'drop'
+          force transaction finish.
+      */
+      thd->add_changed_table(tables_used->table);
+    else
+      invalidate_table(tables_used);
   }
-  STRUCT_UNLOCK(&structure_guard_mutex);
 
   DBUG_VOID_RETURN;
 }
@@ -1363,21 +1414,12 @@ void Query_cache::invalidate(THD *thd, T
 void Query_cache::invalidate(CHANGED_TABLE_LIST *tables_used)
 {
   DBUG_ENTER("Query_cache::invalidate (changed table list)");
-  if (tables_used)
+  for (; tables_used; tables_used= tables_used->next)
   {
-    STRUCT_LOCK(&structure_guard_mutex);
-    if (query_cache_size > 0 && !flush_in_progress)
-    {
-      DUMP(this);
-      for (; tables_used; tables_used= tables_used->next)
-      {
-	invalidate_table((uchar*) tables_used->key, tables_used->key_length);
-	DBUG_PRINT("qcache", ("db: %s  table: %s", tables_used->key,
-			      tables_used->key+
-			      strlen(tables_used->key)+1));
-      }
-    }
-    STRUCT_UNLOCK(&structure_guard_mutex);
+    invalidate_table((uchar*) tables_used->key, tables_used->key_length);
+    DBUG_PRINT("qcache", ("db: %s  table: %s", tables_used->key,
+                          tables_used->key+
+                          strlen(tables_used->key)+1));
   }
   DBUG_VOID_RETURN;
 }
@@ -1396,20 +1438,11 @@ void Query_cache::invalidate(CHANGED_TAB
 void Query_cache::invalidate_locked_for_write(TABLE_LIST *tables_used)
 {
   DBUG_ENTER("Query_cache::invalidate_locked_for_write");
-  if (tables_used)
+  for (; tables_used; tables_used= tables_used->next_local)
   {
-    STRUCT_LOCK(&structure_guard_mutex);
-    if (query_cache_size > 0 && !flush_in_progress)
-    {
-      DUMP(this);
-      for (; tables_used; tables_used= tables_used->next_local)
-      {
-        if (tables_used->lock_type & (TL_WRITE_LOW_PRIORITY | TL_WRITE) &&
-            tables_used->table)
-	  invalidate_table(tables_used->table);
-      }
-    }
-    STRUCT_UNLOCK(&structure_guard_mutex);
+    if (tables_used->lock_type & (TL_WRITE_LOW_PRIORITY | TL_WRITE) &&
+        tables_used->table)
+      invalidate_table(tables_used->table);
   }
   DBUG_VOID_RETURN;
 }
@@ -1423,18 +1456,14 @@ void Query_cache::invalidate(THD *thd, T
 {
   DBUG_ENTER("Query_cache::invalidate (table)");
   
-  STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size > 0 && !flush_in_progress)
-  {
-    using_transactions= using_transactions &&
-      (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
-    if (using_transactions && 
-        (table->file->table_cache_type() == HA_CACHE_TBL_TRANSACT))
-      thd->add_changed_table(table);
-    else
-      invalidate_table(table);
-  }
-  STRUCT_UNLOCK(&structure_guard_mutex);
+  using_transactions= using_transactions &&
+    (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
+  if (using_transactions && 
+      (table->file->table_cache_type() == HA_CACHE_TBL_TRANSACT))
+    thd->add_changed_table(table);
+  else
+    invalidate_table(table);
+
 
   DBUG_VOID_RETURN;
 }
@@ -1443,18 +1472,13 @@ void Query_cache::invalidate(THD *thd, c
 			     my_bool using_transactions)
 {
   DBUG_ENTER("Query_cache::invalidate (key)");
-  
-  STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size > 0 && !flush_in_progress)
-  {
-    using_transactions= using_transactions &&
-      (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
-    if (using_transactions) // used for innodb => has_transactions() is TRUE
-      thd->add_changed_table(key, key_length);
-    else
-      invalidate_table((uchar*)key, key_length);
-  }
-  STRUCT_UNLOCK(&structure_guard_mutex);  
+
+  using_transactions= using_transactions &&
+    (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN));
+  if (using_transactions) // used for innodb => has_transactions() is TRUE
+    thd->add_changed_table(key, key_length);
+  else
+    invalidate_table((uchar*)key, key_length);
 
   DBUG_VOID_RETURN;
 }
@@ -1467,7 +1491,7 @@ void Query_cache::invalidate(char *db)
 {
   DBUG_ENTER("Query_cache::invalidate (db)");
   STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size > 0 && !flush_in_progress)
+  if (query_cache_size > 0 && m_cache_status ==
Query_cache::NO_FLUSH_IN_PROGRESS)
   {
     DUMP(this);
   restart_search:
@@ -1479,7 +1503,15 @@ void Query_cache::invalidate(char *db)
       {
         next= curr->next;
         if (strcmp(db, (char*)(curr->table()->db())) == 0)
-          invalidate_table(curr);
+        {
+          Query_cache_block_table *list_root= curr->table(0);
+          while (list_root->next != list_root)
+          {
+            Query_cache_block *query_block = list_root->next->block();
+            BLOCK_LOCK_WR(query_block);
+            free_query(query_block);
+          }
+        }
         /*
           invalidate_table can freed block on which point 'next' (if
           table of this block used only in queries which was deleted
@@ -1504,21 +1536,11 @@ void Query_cache::invalidate_by_MyISAM_f
 {
   DBUG_ENTER("Query_cache::invalidate_by_MyISAM_filename");
 
-  STRUCT_LOCK(&structure_guard_mutex);
-  if (query_cache_size > 0 && !flush_in_progress)
-  {
-    /* Calculate the key outside the lock to make the lock shorter */
-    char key[MAX_DBKEY_LENGTH];
-    uint32 db_length;
-    uint key_length= filename_2_table_key(key, filename, &db_length);
-    Query_cache_block *table_block;
-    if ((table_block = (Query_cache_block*) hash_search(&tables,
-                                                        (uchar*) key,
-                                                        key_length)))
-      invalidate_table(table_block);
-  }
-  STRUCT_UNLOCK(&structure_guard_mutex);
-
+  /* Calculate the key outside the lock to make the lock shorter */
+  char key[MAX_DBKEY_LENGTH];
+  uint32 db_length;
+  uint key_length= filename_2_table_key(key, filename, &db_length);
+  invalidate_table((uchar *)key, key_length);
   DBUG_VOID_RETURN;
 }
 
@@ -1568,7 +1590,7 @@ void Query_cache::destroy()
     free_cache();
     STRUCT_UNLOCK(&structure_guard_mutex);
 
-    pthread_cond_destroy(&COND_flush_finished);
+    pthread_cond_destroy(&COND_cache_status_changed);
     pthread_mutex_destroy(&structure_guard_mutex);
     initialized = 0;
   }
@@ -1584,8 +1606,8 @@ void Query_cache::init()
 {
   DBUG_ENTER("Query_cache::init");
   pthread_mutex_init(&structure_guard_mutex,MY_MUTEX_INIT_FAST);
-  pthread_cond_init(&COND_flush_finished, NULL);
-  flush_in_progress= FALSE;
+  pthread_cond_init(&COND_cache_status_changed, NULL);
+  m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
   initialized = 1;
   DBUG_VOID_RETURN;
 }
@@ -1817,7 +1839,7 @@ void Query_cache::free_cache()
   DESCRIPTION
     This function will flush cache contents.  It assumes we have
     'structure_guard_mutex' locked.  The function sets the
-    flush_in_progress flag and releases the lock, so other threads may
+    m_cache_status flag and releases the lock, so other threads may
     proceed skipping the cache as if it is disabled.  Concurrent
     flushes are performed in turn.
 
@@ -1837,15 +1859,15 @@ void Query_cache::flush_cache()
     Query_cache::free_cache()) depends on the fact that after the
     flush the cache is empty.
   */
-  while (flush_in_progress)
-    pthread_cond_wait(&COND_flush_finished, &structure_guard_mutex);
+  while (m_cache_status == Query_cache::FLUSH_IN_PROGRESS || m_cache_status ==
Query_cache::TABLE_FLUSH_IN_PROGRESS)
+    pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex);
 
   /*
-    Setting 'flush_in_progress' will prevent other threads from using
+    Setting 'FLUSH_IN_PROGRESS' will prevent other threads from using
     the cache while we are in the middle of the flush, and we release
     the lock so that other threads won't block.
   */
-  flush_in_progress= TRUE;
+  m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
   STRUCT_UNLOCK(&structure_guard_mutex);
 
   my_hash_reset(&queries);
@@ -1856,8 +1878,8 @@ void Query_cache::flush_cache()
   }
 
   STRUCT_LOCK(&structure_guard_mutex);
-  flush_in_progress= FALSE;
-  pthread_cond_signal(&COND_flush_finished);
+  m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+  pthread_cond_signal(&COND_cache_status_changed);
 }
 
 /*
@@ -2013,8 +2035,7 @@ Query_cache_block *
 Query_cache::write_block_data(ulong data_len, uchar* data,
 			      ulong header_len,
 			      Query_cache_block::block_type type,
-			      TABLE_COUNTER_TYPE ntab,
-			      my_bool under_guard)
+			      TABLE_COUNTER_TYPE ntab)
 {
   ulong all_headers_len = (ALIGN_SIZE(sizeof(Query_cache_block)) +
 			   ALIGN_SIZE(ntab*sizeof(Query_cache_block_table)) +
@@ -2026,7 +2047,7 @@ Query_cache::write_block_data(ulong data
 		      data_len, header_len, all_headers_len));
   Query_cache_block *block = allocate_block(max(align_len, 
 						min_allocation_unit),
-					    1, 0, under_guard);
+					    1, 0);
   if (block != 0)
   {
     block->type = type;
@@ -2243,8 +2264,7 @@ my_bool Query_cache::allocate_data_chain
 
     if (!(new_block= allocate_block(max(min_size, align_len),
 				    min_result_data_size == 0,
-				    all_headers_len + min_result_data_size,
-				    1)))
+				    all_headers_len + min_result_data_size)))
     {
       DBUG_PRINT("warning", ("Can't allocate block for results"));
       DBUG_RETURN(FALSE);
@@ -2294,14 +2314,12 @@ void Query_cache::invalidate_table(TABLE
   {
     char key[MAX_DBKEY_LENGTH];
     uint key_length;
-    Query_cache_block *table_block;
+
     key_length=(uint) (strmov(strmov(key,table_list->db)+1,
 			      table_list->table_name) -key)+ 1;
 
     // We don't store temporary tables => no key_length+=4 ...
-    if ((table_block = (Query_cache_block*)
-	 hash_search(&tables,(uchar*) key,key_length)))
-      invalidate_table(table_block);
+    invalidate_table((uchar *)key, key_length);
   }
 }
 
@@ -2313,23 +2331,56 @@ void Query_cache::invalidate_table(TABLE
 
 void Query_cache::invalidate_table(uchar * key, uint32  key_length)
 {
-  Query_cache_block *table_block;
-  if ((table_block = ((Query_cache_block*)
-		      hash_search(&tables, key, key_length))))
-    invalidate_table(table_block);
-}
+    STRUCT_LOCK(&structure_guard_mutex);
 
-void Query_cache::invalidate_table(Query_cache_block *table_block)
-{
-  Query_cache_block_table *list_root =	table_block->table(0);
-  while (list_root->next != list_root)
-  {
-    Query_cache_block *query_block = list_root->next->block();
-    BLOCK_LOCK_WR(query_block);
-    free_query(query_block);
-  }
-}
+    while (is_flushing())
+    {
+    /*
+      If there already is a full flush in progress query cache isn't enabled
+      and additional flushes are redundant; just return instead.
+    */
+      if (m_cache_status == Query_cache::FLUSH_IN_PROGRESS)
+      {
+        STRUCT_UNLOCK(&structure_guard_mutex);
+        return;
+      }
+    /*
+      If a table flush is in progress; wait on cache status to change.
+    */
+      if (m_cache_status == Query_cache::TABLE_FLUSH_IN_PROGRESS)
+        pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex);
+    }
+
+    /*
+      Setting 'TABLE_FLUSH_IN_PROGRESS' will prevent other threads from using
+      the cache while we are in the middle of the flush, and we release
+      the lock so that other threads won't be blocked.
+    */
+    m_cache_status= Query_cache::TABLE_FLUSH_IN_PROGRESS;
+    STRUCT_UNLOCK(&structure_guard_mutex);
 
+    Query_cache_block *table_block;
+    if ( query_cache_size > 0 && (table_block = ((Query_cache_block*)
+          hash_search(&tables, key, key_length))))
+    {
+      Query_cache_block_table *list_root= table_block->table(0);
+      while (list_root->next != list_root)
+        {
+          Query_cache_block *query_block = list_root->next->block();
+          BLOCK_LOCK_WR(query_block);
+          free_query(query_block);
+          DBUG_EXECUTE_IF("debug_cache_locks", sleep(10););
+        }
+    }
+    STRUCT_LOCK(&structure_guard_mutex);
+    m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+    /*
+      net_real_write might be waiting on a change on the m_cache_status
+      variable.
+    */
+    pthread_cond_signal(&COND_cache_status_changed);
+    STRUCT_UNLOCK(&structure_guard_mutex);
+}
 
 /*
   Register given table list begining with given position in tables table of
@@ -2463,7 +2514,6 @@ my_bool Query_cache::register_all_tables
 
   if (n)
   {
-    DBUG_PRINT("qcache", ("failed at table %d", (int) n));
     /* Unlink the tables we allocated above */
     for (Query_cache_block_table *tmp = block->table(0) ;
 	 tmp != block_table;
@@ -2506,7 +2556,16 @@ Query_cache::insert_table(uint key_len, 
       as far as we delete all queries with this table, table block will be
       deleted, too
     */
-    invalidate_table(table_block);
+    {
+      Query_cache_block_table *list_root= table_block->table(0);
+      while (list_root->next != list_root)
+      {
+        Query_cache_block *query_block= list_root->next->block();
+        BLOCK_LOCK_WR(query_block);
+        free_query(query_block);
+      }
+    }
+
     table_block= 0;
   }
 
@@ -2516,8 +2575,7 @@ Query_cache::insert_table(uint key_len, 
 			(ulong) key, (int) key_len));
     table_block = write_block_data(key_len, (uchar*) key,
 				   ALIGN_SIZE(sizeof(Query_cache_table)),
-				   Query_cache_block::TABLE,
-				   1, 1);
+				   Query_cache_block::TABLE,1);
     if (table_block == 0)
     {
       DBUG_PRINT("qcache", ("Can't write table name to cache"));
@@ -2577,12 +2635,11 @@ void Query_cache::unlink_table(Query_cac
 *****************************************************************************/
 
 Query_cache_block *
-Query_cache::allocate_block(ulong len, my_bool not_less, ulong min,
-			    my_bool under_guard)
+Query_cache::allocate_block(ulong len, my_bool not_less, ulong min)
 {
   DBUG_ENTER("Query_cache::allocate_block");
-  DBUG_PRINT("qcache", ("len %lu, not less %d, min %lu, uder_guard %d",
-		      len, not_less,min,under_guard));
+  DBUG_PRINT("qcache", ("len %lu, not less %d, min %lu",
+		      len, not_less,min));
 
   if (len >= min(query_cache_size, query_cache_limit))
   {
@@ -2591,17 +2648,6 @@ Query_cache::allocate_block(ulong len, m
     DBUG_RETURN(0); // in any case we don't have such piece of memory
   }
 
-  if (!under_guard)
-  {
-    STRUCT_LOCK(&structure_guard_mutex);
-
-    if (unlikely(query_cache.query_cache_size == 0 || flush_in_progress))
-    {
-      STRUCT_UNLOCK(&structure_guard_mutex);
-      DBUG_RETURN(0);
-    }
-  }
-
   /* Free old queries until we have enough memory to store this block */
   Query_cache_block *block;
   do
@@ -2616,8 +2662,6 @@ Query_cache::allocate_block(ulong len, m
       split_block(block,ALIGN_SIZE(len));
   }
 
-  if (!under_guard)
-    STRUCT_UNLOCK(&structure_guard_mutex);
   DBUG_RETURN(block);
 }
 
@@ -2852,9 +2896,7 @@ uint Query_cache::find_bin(ulong size)
   }
   uint bin =  steps[left].idx - 
     (uint)((size - steps[left].size)/steps[left].increment);
-#ifndef DBUG_OFF
-  bins_dump();
-#endif
+
   DBUG_PRINT("qcache", ("bin %u step %u, size %lu step size %lu",
 			bin, left, size, steps[left].size));
   DBUG_RETURN(bin);
@@ -3146,7 +3188,7 @@ void Query_cache::pack_cache()
 
   STRUCT_LOCK(&structure_guard_mutex);
 
-  if (unlikely(query_cache_size == 0 || flush_in_progress))
+  if (unlikely(query_cache_size == 0 || m_cache_status !=
Query_cache::NO_FLUSH_IN_PROGRESS))
   {
     STRUCT_UNLOCK(&structure_guard_mutex);
     DBUG_VOID_RETURN;
@@ -3461,7 +3503,7 @@ my_bool Query_cache::join_results(ulong 
   DBUG_ENTER("Query_cache::join_results");
 
   STRUCT_LOCK(&structure_guard_mutex);
-  if (queries_blocks != 0 && !flush_in_progress)
+  if (queries_blocks != 0 && m_cache_status == Query_cache::NO_FLUSH_IN_PROGRESS)
   {
     DBUG_ASSERT(query_cache_size > 0);
     Query_cache_block *block = queries_blocks;
@@ -3769,7 +3811,7 @@ my_bool Query_cache::check_integrity(boo
   if (!locked)
     STRUCT_LOCK(&structure_guard_mutex);
 
-  if (unlikely(query_cache_size == 0 || flush_in_progress))
+  if (unlikely(query_cache_size == 0 || m_cache_status !=
Query_cache::NO_FLUSH_IN_PROGRESS))
   {
     if (!locked)
       STRUCT_UNLOCK(&query_cache.structure_guard_mutex);

--- 1.675/sql/sql_parse.cc	2007-06-01 09:43:54 +02:00
+++ 1.676/sql/sql_parse.cc	2007-06-14 18:07:47 +02:00
@@ -2445,7 +2445,7 @@ end_with_restore_list:
           check_grant(thd, INSERT_ACL | CREATE_ACL, &new_list, 0, 1, 0)))
         goto error;
     }
-    query_cache_invalidate3(thd, first_table, 0);
+
     if (end_active_trans(thd) || mysql_rename_tables(thd, first_table, 0))
       goto error;
     break;

--- 1.424/sql/sql_table.cc	2007-06-01 11:33:54 +02:00
+++ 1.425/sql/sql_table.cc	2007-06-14 18:07:47 +02:00
@@ -1411,14 +1411,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
     LOCK_open during wait_if_global_read_lock(), other threads could not
     close their tables. This would make a pretty deadlock.
   */
-  thd->mysys_var->current_mutex= &LOCK_open;
-  thd->mysys_var->current_cond= &COND_refresh;
-  VOID(pthread_mutex_lock(&LOCK_open));
-
   error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
-
-  pthread_mutex_unlock(&LOCK_open);
-
   pthread_mutex_lock(&thd->mysys_var->mutex);
   thd->mysys_var->current_mutex= 0;
   thd->mysys_var->current_cond= 0;
@@ -1461,13 +1454,10 @@ int mysql_rm_table_part2_with_lock(THD *
   int error;
   thd->mysys_var->current_mutex= &LOCK_open;
   thd->mysys_var->current_cond= &COND_refresh;
-  VOID(pthread_mutex_lock(&LOCK_open));
 
   error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 1,
 			      dont_log_query);
 
-  pthread_mutex_unlock(&LOCK_open);
-
   pthread_mutex_lock(&thd->mysys_var->mutex);
   thd->mysys_var->current_mutex= 0;
   thd->mysys_var->current_cond= 0;
@@ -1520,6 +1510,8 @@ int mysql_rm_table_part2(THD *thd, TABLE
   String built_query;
   DBUG_ENTER("mysql_rm_table_part2");
 
+  pthread_mutex_lock(&LOCK_open);
+  
   LINT_INIT(alias);
   LINT_INIT(path_length);
   safe_mutex_assert_owner(&LOCK_open);
@@ -1555,8 +1547,13 @@ int mysql_rm_table_part2(THD *thd, TABLE
     }
   }
 
-  if (!drop_temporary && lock_table_names(thd, tables))
+  if (!drop_temporary && lock_table_names_exclusively(thd, tables))
+  {
+    pthread_mutex_unlock(&LOCK_open);
     DBUG_RETURN(1);
+  }
+
+  pthread_mutex_unlock(&LOCK_open);
 
   /* Don't give warnings for not found errors, as we already generate notes */
   thd->no_warnings_for_error= 1;
@@ -1604,9 +1601,11 @@ int mysql_rm_table_part2(THD *thd, TABLE
     {
       TABLE *locked_table;
       abort_locked_tables(thd, db, table->table_name);
+      pthread_mutex_lock(&LOCK_open);
       remove_table_from_cache(thd, db, table->table_name,
 	                      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
@@ -1739,9 +1738,10 @@ int mysql_rm_table_part2(THD *thd, TABLE
       */
     }
   }
-
+  pthread_mutex_lock(&LOCK_open);
   if (!drop_temporary)
     unlock_table_names(thd, tables, (TABLE_LIST*) 0);
+  pthread_mutex_unlock(&LOCK_open);
   thd->no_warnings_for_error= 0;
   DBUG_RETURN(error);
 }

--- 1.94/sql/sql_trigger.cc	2007-05-24 00:39:25 +02:00
+++ 1.95/sql/sql_trigger.cc	2007-06-14 18:07:47 +02:00
@@ -1268,8 +1268,6 @@ bool Table_triggers_list::drop_all_trigg
   bzero(&table, sizeof(table));
   init_alloc_root(&table.mem_root, 8192, 0);
 
-  safe_mutex_assert_owner(&LOCK_open);
-
   if (Table_triggers_list::check_n_load(thd, db, name, &table, 1))
   {
     result= 1;
@@ -1431,26 +1429,24 @@ Table_triggers_list::change_table_name_i
 }
 
 
-/*
-  Update .TRG and .TRN files after renaming triggers' subject table.
+/**
+  @brief Update .TRG and .TRN files after renaming triggers' subject table.
 
-  SYNOPSIS
-    change_table_name()
-      thd        Thread context
-      db         Old database of subject table
-      old_table  Old name of subject table
-      new_db     New database for subject table
-      new_table  New name of subject table
+  @param thd Thread context
+  @param db Old database of subject table
+  @param old_table Old name of subject table
+  @param new_db New database for subject table
+  @param new_table New name of subject table
 
-  NOTE
+  @note
     This method tries to leave trigger related files in consistent state,
     i.e. it either will complete successfully, or will fail leaving files
     in their initial state.
     Also this method assumes that subject table is not renamed to itself.
+    This method needs to be called under an exclusive table name lock.
 
-  RETURN VALUE
-    FALSE  Success
-    TRUE   Error
+  @retval FALSE Success
+  @retval TRUE  Error
 */
 
 bool Table_triggers_list::change_table_name(THD *thd, const char *db,
@@ -1465,9 +1461,22 @@ bool Table_triggers_list::change_table_n
 
   bzero(&table, sizeof(table));
   init_alloc_root(&table.mem_root, 8192, 0);
-
-  safe_mutex_assert_owner(&LOCK_open);
-
+  
+  uchar key[MAX_DBKEY_LENGTH];
+  uint key_length= (uint) (strmov(strmov((char*)&key[0], db)+1,
+                    old_table)-(char*)&key[0])+1;
+  
+  /*
+     This method interfaces the mysql server code protected by
+     either LOCK_open mutex or with an exclusive table name lock.
+     In the future, only an exclusive table name lock will be enough.
+  */
+  bool got_lock_open= (LOCK_open.count > 0 && 
+                       pthread_equal(pthread_self(), LOCK_open.thread));
+  DBUG_ASSERT(is_table_name_exclusively_locked_by_this_thread(thd, 
+                                             (uchar*)&key[0], key_length)
+      || got_lock_open);
+  
   DBUG_ASSERT(my_strcasecmp(table_alias_charset, db, new_db) ||
               my_strcasecmp(table_alias_charset, old_table, new_table));
 

--- 1.43/sql/sql_rename.cc	2006-12-31 01:06:36 +01:00
+++ 1.44/sql/sql_rename.cc	2007-06-14 18:07:47 +02:00
@@ -144,10 +144,14 @@ bool mysql_rename_tables(THD *thd, TABLE
     }
   }
 
-  VOID(pthread_mutex_lock(&LOCK_open));
-  if (lock_table_names(thd, table_list))
+  pthread_mutex_lock(&LOCK_open);
+  if (lock_table_names_exclusively(thd, table_list))
+  {
+    pthread_mutex_unlock(&LOCK_open);
     goto err;
-  
+  }
+  pthread_mutex_unlock(&LOCK_open);
+
   error=0;
   if ((ren_table=rename_tables(thd,table_list,0)))
   {
@@ -183,10 +187,14 @@ bool mysql_rename_tables(THD *thd, TABLE
     send_ok(thd);
   }
 
+  if (!error)
+    query_cache_invalidate3(thd, table_list, 0);
+
+  pthread_mutex_lock(&LOCK_open);
   unlock_table_names(thd, table_list, (TABLE_LIST*) 0);
+  pthread_mutex_unlock(&LOCK_open);
 
 err:
-  pthread_mutex_unlock(&LOCK_open);
   /* enable logging back if needed */
   if (disable_logs)
   {

--- 1.38/sql/sql_cache.h	2007-05-10 11:59:29 +02:00
+++ 1.39/sql/sql_cache.h	2007-06-14 18:07:47 +02:00
@@ -238,8 +238,11 @@ public:
     free_memory_blocks, total_blocks, lowmem_prunes;
 
 private:
-  pthread_cond_t COND_flush_finished;
-  bool flush_in_progress;
+  pthread_cond_t COND_cache_status_changed;
+
+  enum Cache_status { NO_FLUSH_IN_PROGRESS, FLUSH_IN_PROGRESS, TABLE_FLUSH_IN_PROGRESS };
+
+  Cache_status m_cache_status;
 
   void free_query_internal(Query_cache_block *point);
 
@@ -253,7 +256,7 @@ protected:
       2. query block (for operation inside query (query block/results))
 
     Thread doing cache flush releases the mutex once it sets
-    flush_in_progress flag, so other threads may bypass the cache as
+    m_cache_status flag, so other threads may bypass the cache as
     if it is disabled, not waiting for reset to finish.  The exception
     is other threads that were going to do cache flush---they'll wait
     till the end of a flush operation.
@@ -270,6 +273,7 @@ protected:
   /* options */
   ulong min_allocation_unit, min_result_data_size;
   uint def_query_hash_size, def_table_hash_size;
+  
   uint mem_bin_num, mem_bin_steps;		// See at init_cache & find_bin
 
   my_bool initialized;
@@ -347,8 +351,7 @@ protected:
   Query_cache_block *write_block_data(ulong data_len, uchar* data,
 				       ulong header_len,
 				       Query_cache_block::block_type type,
-				       TABLE_COUNTER_TYPE ntab = 0,
-				       my_bool under_guard=0);
+				       TABLE_COUNTER_TYPE ntab = 0);
   my_bool append_result_data(Query_cache_block **result,
 			     ulong data_len, uchar* data,
 			     Query_cache_block *parent);
@@ -360,8 +363,7 @@ protected:
   inline ulong get_min_first_result_data_size();
   inline ulong get_min_append_result_data_size();
   Query_cache_block *allocate_block(ulong len, my_bool not_less,
-				     ulong min,
-				     my_bool under_guard=0);
+				     ulong min);
   /*
     If query is cacheable return number tables in query
     (query without tables not cached)
@@ -423,6 +425,9 @@ protected:
   friend void query_cache_insert(NET *net, const char *packet, ulong length);
   friend void query_cache_end_of_result(THD *thd);
   friend void query_cache_abort(NET *net);
+
+  bool is_flushing(void) { return (m_cache_status == Query_cache::FLUSH_IN_PROGRESS
+                            || m_cache_status == Query_cache::TABLE_FLUSH_IN_PROGRESS); }
 
   /*
     The following functions are only used when debugging
Thread
bk commit into 5.1 tree (thek:1.2541) BUG#21074kpettersson14 Jun