List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:February 12 2009 12:19pm
Subject:bzr commit into mysql-5.1-bugteam branch (davi:2801) Bug#41098
View as plain text  
# At a local mysql-5.1-bugteam repository of davi

 2801 Davi Arnaut	2009-02-12
      Bug#41098: Query Cache returns wrong result with concurrent insert
      
      The problem is that select queries executed concurrently with
      concurrent inserts on a MyISAM table could be cached if the
      select started after the query cache invalidation but before
      the unlock of tables performed by the concurrent insert. This
      race could happen because the concurrent insert was failing
      to prevent cache of select queries happening at the same time.
      
      The solution is to a 'uncacheable' status flag to signal that
      a concurrent insert is being performed on the table and that
      queries executing at the same time shouldn't cache the results.
      modified:
        mysql-test/r/query_cache_debug.result
        mysql-test/t/disabled.def
        mysql-test/t/query_cache_debug.test
        sql/sql_cache.cc
        sql/sql_insert.cc
        storage/myisam/ha_myisam.cc
        storage/myisam/mi_locking.c
        storage/myisam/myisamdef.h

per-file messages:
  mysql-test/r/query_cache_debug.result
    Add test case result for Bug#41098
  mysql-test/t/disabled.def
    Re-enable test case.
  mysql-test/t/query_cache_debug.test
    Add test case for Bug#41098
  sql/sql_cache.cc
    Debug sync point for regression testing purposes.
  sql/sql_insert.cc
    Remove meaningless query cache invalidate. There is already
    a preceding invalidate for queries that started before the
    concurrent insert.
  storage/myisam/ha_myisam.cc
    Check for a active concurrent insert.
  storage/myisam/mi_locking.c
    Signal the start of a concurrent insert. Flag is zeroed once
    the state is updated back.
  storage/myisam/myisamdef.h
    Add flag to signal a active concurrent insert.
=== modified file 'mysql-test/r/query_cache_debug.result'
--- a/mysql-test/r/query_cache_debug.result	2008-04-01 22:43:17 +0000
+++ b/mysql-test/r/query_cache_debug.result	2009-02-12 12:19:01 +0000
@@ -22,3 +22,52 @@ Qcache_queries_in_cache	0
 set global query_cache_size= 0;
 use test;
 drop table t1;
+SET @old_concurrent_insert= @@GLOBAL.concurrent_insert;
+SET @old_query_cache_size= @@GLOBAL.query_cache_size;
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (a INT);
+INSERT INTO t1 VALUES (1),(2),(3);
+SET GLOBAL concurrent_insert= 1;
+SET GLOBAL query_cache_size= 1024*512;
+SET GLOBAL query_cache_type= ON;
+# Switch to connection con1
+SET SESSION debug='+d,wait_after_query_cache_invalidate';
+# Send concurrent insert, will wait in the query cache table invalidate
+INSERT INTO t1 VALUES (4);
+# Switch to connection default
+# Wait for concurrent insert to reach the debug point
+# Switch to connection con2
+# Send SELECT that shouldn't be cached
+SELECT * FROM t1;
+a
+1
+2
+3
+# Switch to connection default
+# Notify the concurrent insert to proceed
+SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST
+WHERE STATE = 'wait_after_query_cache_invalidate' INTO @thread_id;
+KILL QUERY @thread_id;
+# Switch to connection con1
+# Gather insert result
+SHOW STATUS LIKE "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	0
+# Test that it's cacheable
+SELECT * FROM t1;
+a
+1
+2
+3
+4
+SHOW STATUS LIKE "Qcache_queries_in_cache";
+Variable_name	Value
+Qcache_queries_in_cache	1
+# Disconnect
+# Restore defaults
+RESET QUERY CACHE;
+DROP TABLE t1,t2;
+SET GLOBAL concurrent_insert= DEFAULT;
+SET GLOBAL query_cache_size= DEFAULT;
+SET GLOBAL query_cache_type= DEFAULT;

=== modified file 'mysql-test/t/disabled.def'
--- a/mysql-test/t/disabled.def	2009-01-30 14:02:34 +0000
+++ b/mysql-test/t/disabled.def	2009-02-12 12:19:01 +0000
@@ -10,5 +10,4 @@
 #
 ##############################################################################
 kill                     : Bug#37780 2008-12-03 HHunger need some changes to be robust enough for pushbuild.
-query_cache_28249        : Bug#41098 Query Cache returns wrong result with concurrent insert
 innodb_bug39438          : BUG#42383 2009-01-28 lsoares "This fails in embedded and on windows.  Note that this test is not run on windows and on embedded in PB for main trees currently"

=== modified file 'mysql-test/t/query_cache_debug.test'
--- a/mysql-test/t/query_cache_debug.test	2008-04-01 22:43:17 +0000
+++ b/mysql-test/t/query_cache_debug.test	2009-02-12 12:19:01 +0000
@@ -44,3 +44,71 @@ set global query_cache_size= 0;
 use test;
 drop table t1;
 
+#
+# Bug#41098: Query Cache returns wrong result with concurrent insert
+#
+
+SET @old_concurrent_insert= @@GLOBAL.concurrent_insert;
+SET @old_query_cache_size= @@GLOBAL.query_cache_size;
+
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (a INT);
+INSERT INTO t1 VALUES (1),(2),(3);
+
+SET GLOBAL concurrent_insert= 1;
+SET GLOBAL query_cache_size= 1024*512;
+SET GLOBAL query_cache_type= ON;
+
+connect(con1,localhost,root,,test,,);
+connect(con2,localhost,root,,test,,);
+
+connection con1;
+--echo # Switch to connection con1
+SET SESSION debug='+d,wait_after_query_cache_invalidate';
+--echo # Send concurrent insert, will wait in the query cache table invalidate
+--send INSERT INTO t1 VALUES (4)
+
+connection default;
+--echo # Switch to connection default
+--echo # Wait for concurrent insert to reach the debug point
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST
+  WHERE STATE = "wait_after_query_cache_invalidate" AND
+        INFO = "INSERT INTO t1 VALUES (4)";
+--source include/wait_condition.inc
+
+connection con2;
+--echo # Switch to connection con2
+--echo # Send SELECT that shouldn't be cached
+SELECT * FROM t1;
+
+connection default;
+--echo # Switch to connection default
+--echo # Notify the concurrent insert to proceed
+SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST
+  WHERE STATE = 'wait_after_query_cache_invalidate' INTO @thread_id;
+KILL QUERY @thread_id;
+
+connection con1;
+--echo # Switch to connection con1
+--echo # Gather insert result
+--reap
+SHOW STATUS LIKE "Qcache_queries_in_cache";
+--echo # Test that it's cacheable
+SELECT * FROM t1;
+SHOW STATUS LIKE "Qcache_queries_in_cache";
+
+--echo # Disconnect
+disconnect con1;
+disconnect con2;
+
+connection default;
+--echo # Restore defaults
+RESET QUERY CACHE;
+DROP TABLE t1,t2;
+SET GLOBAL concurrent_insert= DEFAULT;
+SET GLOBAL query_cache_size= DEFAULT;
+SET GLOBAL query_cache_type= DEFAULT;

=== modified file 'sql/sql_cache.cc'
--- a/sql/sql_cache.cc	2009-01-22 11:22:26 +0000
+++ b/sql/sql_cache.cc	2009-02-12 12:19:01 +0000
@@ -1518,6 +1518,9 @@ void Query_cache::invalidate(THD *thd, T
       invalidate_table(thd, tables_used);
   }
 
+  DBUG_EXECUTE_IF("wait_after_query_cache_invalidate",
+                  debug_wait_for_kill("wait_after_query_cache_invalidate"););
+
   DBUG_VOID_RETURN;
 }
 

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2009-02-03 17:16:24 +0000
+++ b/sql/sql_insert.cc	2009-02-12 12:19:01 +0000
@@ -904,20 +904,6 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
     }
     DBUG_ASSERT(transactional_table || !changed || 
                 thd->transaction.stmt.modified_non_trans_table);
-
-    if (thd->lock)
-    {
-      /*
-        Invalidate the table in the query cache if something changed
-        after unlocking when changes become fisible.
-        TODO: this is workaround. right way will be move invalidating in
-        the unlock procedure.
-      */
-      if (lock_type ==  TL_WRITE_CONCURRENT_INSERT && changed)
-      {
-        query_cache_invalidate3(thd, table_list, 1);
-      }
-    }
   }
   thd_proc_info(thd, "end");
   /*

=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc	2009-02-05 06:16:00 +0000
+++ b/storage/myisam/ha_myisam.cc	2009-02-12 12:19:01 +0000
@@ -2152,6 +2152,15 @@ my_bool ha_myisam::register_query_cache_
     }
   }
 
+  /*
+    This query execution might have started after the query cache was flushed
+    by a concurrent INSERT. In this case, don't cache this statement as the
+    data file length difference might not be visible yet if the tables haven't
+    been unlocked by the concurrent insert thread.
+  */
+  if (file->state->uncacheable)
+    DBUG_RETURN(FALSE);
+
   /* It is ok to try to cache current statement. */
   DBUG_RETURN(TRUE);
 }

=== modified file 'storage/myisam/mi_locking.c'
--- a/storage/myisam/mi_locking.c	2007-08-13 13:11:25 +0000
+++ b/storage/myisam/mi_locking.c	2009-02-12 12:19:01 +0000
@@ -293,6 +293,8 @@ void mi_get_status(void* param, int conc
   info->save_state=info->s->state.state;
   info->state= &info->save_state;
   info->append_insert_at_end= concurrent_insert;
+  if (concurrent_insert)
+    info->s->state.state.uncacheable= TRUE;
   DBUG_VOID_RETURN;
 }
 

=== modified file 'storage/myisam/myisamdef.h'
--- a/storage/myisam/myisamdef.h	2009-01-26 06:35:15 +0000
+++ b/storage/myisam/myisamdef.h	2009-02-12 12:19:01 +0000
@@ -38,6 +38,7 @@ typedef struct st_mi_status_info
   my_off_t key_file_length;
   my_off_t data_file_length;
   ha_checksum checksum;
+  my_bool uncacheable;                  /* Active concurrent insert */
 } MI_STATUS_INFO;
 
 typedef struct st_mi_state_info

Thread
bzr commit into mysql-5.1-bugteam branch (davi:2801) Bug#41098Davi Arnaut12 Feb
  • Re: bzr commit into mysql-5.1-bugteam branch (davi:2801) Bug#41098Sergei Golubchik19 Feb