bk commit into 5.1 tree (thek:1.2529) BUG#28211

[kpettersson]

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-05-24 22:07:36+02:00, thek@adventure.(none) +4 -0
  Bug#28211 RENAME DATABASE and query cache don't play nicely together
  - When all table blocks were removed from the query cache the client session
    hung in a tight loop waiting on an impossible condition while consuming a lot
    of CPU.
  - This patch also corrects an error which caused valid tables to sometimes be
    removed from the query cache.
  - Minor refactoring of name: tables_blocks => first_table_block

  mysql-test/r/query_cache.result@stripped, 2007-05-24 22:07:33+02:00, thek@adventure.(none)
+19 -0
    Added test case to make sure server doesn't hang in a tight loop if last
    table block is removed from the cache.

  mysql-test/t/query_cache.test@stripped, 2007-05-24 22:07:33+02:00, thek@adventure.(none) +22
-0
    Added test case to make sure server doesn't hang in a tight loop if last
    table block is removed from the cache.

  sql/sql_cache.cc@stripped, 2007-05-24 22:07:33+02:00, thek@adventure.(none) +49 -33
    Bug#28211 RENAME DATABASE and query cache don't play nicely together
    - When all table blocks were removed from the query cache the client session
      hung in a tight loop waiting on an impossible condition while consuming a lot
      of CPU.
    - This patch also corrects an error which caused valid tables to sometimes be
      removed from the query cache.

  sql/sql_cache.h@stripped, 2007-05-24 22:07:33+02:00, thek@adventure.(none) +1 -1
    Refactored name of 'tables_blocks' to 'first_table_block'. 

# 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/bug28211/my51-bug28211

--- 1.109/sql/sql_cache.cc	2007-05-15 15:44:38 +02:00
+++ 1.110/sql/sql_cache.cc	2007-05-24 22:07:33 +02:00
@@ -23,7 +23,7 @@
 	- list of blocks ordered as they allocated in memory
 (first_block)
 	- list of queries block (queries_blocks)
-	- list of used tables (tables_blocks)
+	- list of used tables (first_table_block)
 
 2. Query cache memory pool (cache) consists of
 	- table of steps of memory bins allocation
@@ -1456,28 +1456,43 @@ void Query_cache::invalidate(char *db)
   if (query_cache_size > 0 && !flush_in_progress)
   {
     DUMP(this);
-  restart_search:
-    if (tables_blocks)
+    if (first_table_block)
     {
-      Query_cache_block *curr= tables_blocks;
-      Query_cache_block *next;
-      do
-      {
-        next= curr->next;
-        if (strcmp(db, (char*)(curr->table()->db())) == 0)
-          invalidate_table(curr);
-        /*
-          invalidate_table can freed block on which point 'next' (if
-          table of this block used only in queries which was deleted
-          by invalidate_table). As far as we do not allocate new blocks
-          and mark all headers of freed blocks as 'FREE' (even if they are
-          merged with other blocks) we can just test type of block
-          to be sure that block is not deleted
-        */
-        if (next->type == Query_cache_block::FREE)
-          goto restart_search;
-        curr= next;
-      } while (curr != tables_blocks);
+     Query_cache_block *table_block = first_table_block;
+     do
+     {
+        bool found= FALSE;
+        do
+        {
+          Query_cache_table *table = table_block->table();
+          if (strcmp(table->db(),db) == 0)
+            found= TRUE;
+          else
+            table_block= table_block->next;
+        } while(!found && table_block != first_table_block);
+
+        if (found)
+        {
+          /*
+            Invalidating this table will also mean that all cached queries using
+            this table also will be invalidated. This will in turn change the
+            list of tables associated with these queries and the linked list of
+            used table will be changed. Because of this we need to restart the search
+            after each invalidated table.
+            Note: If last table block is removed the first_table_block pointer will be 0
+          */
+          Query_cache_block *next= table_block->next;
+          invalidate_table(table_block);
+          /*
+            If the next query cache block is marked as FREE this is an indication
+            that we need to restart the search.
+          */
+          if( next->type == Query_cache_block::FREE )
+            table_block= first_table_block;
+          else
+            table_block= next;
+        }
+     } while( first_table_block != 0 && table_block != first_table_block );
     }
   }
   STRUCT_UNLOCK(&structure_guard_mutex);
@@ -2412,6 +2427,7 @@ Query_cache::register_tables_from_list(T
                   (ulong) tables_used->table,
                   tables_used->table->s->table_cache_key.length,
                   (ulong) tables_used->table->s->table_cache_key.str));
+
       if (!insert_table(tables_used->table->s->table_cache_key.length,
                         tables_used->table->s->table_cache_key.str,
                         block_table,
@@ -2478,7 +2494,7 @@ my_bool Query_cache::register_all_tables
 
   n= register_tables_from_list(tables_used, 0, block_table);
 
-  if (n)
+  if (n==0)
   {
     DBUG_PRINT("qcache", ("failed at table %d", (int) n));
     /* Unlink the tables we allocated above */
@@ -2542,7 +2558,7 @@ Query_cache::insert_table(uint key_len, 
     }
     Query_cache_table *header = table_block->table();
     double_linked_list_simple_include(table_block,
-				      &tables_blocks);
+				      &first_table_block);
     Query_cache_block_table *list_root = table_block->table(0);
     list_root->n = 0;
     list_root->next = list_root->prev = list_root;
@@ -2582,7 +2598,7 @@ void Query_cache::unlink_table(Query_cac
     // list is empty (neighbor is root of list)
     Query_cache_block *table_block = neighbour->block();
     double_linked_list_exclude(table_block,
-			       &tables_blocks);
+			       &first_table_block);
     hash_delete(&tables,(byte *) table_block);
     free_memory_block(table_block);
   }
@@ -3262,8 +3278,8 @@ my_bool Query_cache::move_by_type(byte *
     new_block->n_tables=1;
     memmove((char*) new_block->data(), data, len-new_block->headers_len());
     relink(block, new_block, next, prev, pnext, pprev);
-    if (tables_blocks == block)
-      tables_blocks = new_block;
+    if (first_table_block == block)
+      first_table_block = new_block;
 
     Query_cache_block_table *nlist_root = new_block->table(0);
     nlist_root->n = 0;
@@ -3761,15 +3777,15 @@ void Query_cache::tables_dump()
   DBUG_PRINT("qcache", ("--------------------"));
   DBUG_PRINT("qcache", ("TABLES"));
   DBUG_PRINT("qcache", ("--------------------"));
-  if (tables_blocks != 0)
+  if (first_table_block != 0)
   {
-    Query_cache_block *table_block = tables_blocks;
+    Query_cache_block *table_block = first_table_block;
     do
     {
       Query_cache_table *table = table_block->table();
       DBUG_PRINT("qcache", ("'%s' '%s'", table->db(), table->table()));
       table_block = table_block->next;
-    } while ( table_block != tables_blocks);
+    } while (table_block != first_table_block);
   }
   else
     DBUG_PRINT("qcache", ("no tables in list"));
@@ -3877,7 +3893,7 @@ my_bool Query_cache::check_integrity(boo
       break;
     }
     case Query_cache_block::TABLE:
-      if (in_list(tables_blocks, block, "tables"))
+      if (in_list(first_table_block, block, "tables"))
 	result = 1;
       if (in_table_list(block->table(0),  block->table(0), "table list root"))
 	result = 1;
@@ -3991,7 +4007,7 @@ my_bool Query_cache::check_integrity(boo
   }
 
   DBUG_PRINT("qcache", ("check tables ..."));
-  if ((block = tables_blocks))
+  if ((block = first_table_block))
   {
     do
     {
@@ -4009,7 +4025,7 @@ my_bool Query_cache::check_integrity(boo
       if (in_blocks(block))
 	result = 1;
       block=block->next;
-    } while (block != tables_blocks);
+    } while (block != first_table_block);
   }
 
   DBUG_PRINT("qcache", ("check free blocks"));

--- 1.89/mysql-test/r/query_cache.result	2007-05-10 14:27:52 +02:00
+++ 1.90/mysql-test/r/query_cache.result	2007-05-24 22:07:33 +02:00
@@ -1437,3 +1437,22 @@ set GLOBAL query_cache_type=default;
 set GLOBAL query_cache_limit=default;
 set GLOBAL query_cache_min_res_unit=default;
 set GLOBAL query_cache_size= default;
+drop database if exists db1;
+drop database if exists db2;
+set GLOBAL query_cache_size=15*1024*1024;
+create database db1;
+use db1;
+create table t1(c1 int)engine=myisam;
+insert into t1(c1) values (1);
+select * from db1.t1 f;
+c1
+1
+show status like 'Qcache_queries_in_cache';
+Variable_name	Value
+Qcache_queries_in_cache	1
+rename schema db1 to db2;
+show status like 'Qcache_queries_in_cache';
+Variable_name	Value
+Qcache_queries_in_cache	0
+drop database db2;
+set global query_cache_size=default;

--- 1.69/mysql-test/t/query_cache.test	2007-05-08 11:56:45 +02:00
+++ 1.70/mysql-test/t/query_cache.test	2007-05-24 22:07:33 +02:00
@@ -1000,3 +1000,25 @@ set GLOBAL query_cache_min_res_unit=defa
 set GLOBAL query_cache_size= default;
 
 # End of 5.0 tests
+
+
+#
+# Bug #28211 RENAME DATABASE and query cache don't play nicely together
+#
+--disable_warnings
+drop database if exists db1;
+drop database if exists db2;
+--enable_warnings
+set GLOBAL query_cache_size=15*1024*1024;
+create database db1;
+use db1;
+create table t1(c1 int)engine=myisam;
+insert into t1(c1) values (1);
+select * from db1.t1 f;
+show status like 'Qcache_queries_in_cache';
+rename schema db1 to db2;
+show status like 'Qcache_queries_in_cache';
+drop database db2;
+set global query_cache_size=default;
+
+# End of 5.1 tests

--- 1.37/sql/sql_cache.h	2007-01-24 18:57:00 +01:00
+++ 1.38/sql/sql_cache.h	2007-05-24 22:07:33 +02:00
@@ -264,7 +264,7 @@ protected:
   byte *cache;					// cache memory
   Query_cache_block *first_block;		// physical location block list
   Query_cache_block *queries_blocks;		// query list (LIFO)
-  Query_cache_block *tables_blocks;
+  Query_cache_block *first_table_block;         // list of used tables
 
   Query_cache_memory_bin *bins;			// free block lists
   Query_cache_memory_bin_step *steps;		// bins spacing info