MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:kpettersson Date:July 11 2007 6:47am
Subject:bk commit into 5.0 tree (thek:1.2489) BUG#28249
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 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-07-11 08:46:56+02:00, thek@adventure.(none) +5 -0
  Bug#28249 Query Cache returns wrong result with concurrent insert / certain lock
  
  A race condition in the integration between MyISAM and the query cache code 
  caused the query cache to fail to invalidate itself on concurrently inserted
  data.
  
  This patch fix this problem by using the existing handler interface which, upon
  each statement cache attempt, compare the size of the table as viewed from the 
  cache writing thread and with any snap shot of the global table state. If the
  two sizes are different the global table size is unknown and the current
  statement can't be cached.

  mysql-test/r/query_cache.result@stripped, 2007-07-11 08:46:54+02:00, thek@adventure.(none) +35 -0
    Added test case

  mysql-test/t/query_cache.test@stripped, 2007-07-11 08:46:54+02:00, thek@adventure.(none) +53 -0
    Added test case

  sql/ha_myisam.cc@stripped, 2007-07-11 08:46:54+02:00, thek@adventure.(none) +63 -0
    - Implemented handler interface for ha_myisam class to dermine if the table
     belonging to the currently processed statement can be cached or not.

  sql/ha_myisam.h@stripped, 2007-07-11 08:46:54+02:00, thek@adventure.(none) +7 -0
    - Implemented handler interface for ha_myisam class to dermine if the table
     belonging to the currently processed statement can be cached or not.

  sql/handler.h@stripped, 2007-07-11 08:46:54+02:00, thek@adventure.(none) +39 -6
    - Documented register_query_cache_table method in the handler interface.

# 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/bug28249/my50-bug28249

--- 1.178/sql/ha_myisam.cc	2007-03-28 10:22:20 +02:00
+++ 1.179/sql/ha_myisam.cc	2007-07-11 08:46:54 +02:00
@@ -1922,3 +1922,66 @@ uint ha_myisam::checksum() const
   return (uint)file->state->checksum;
 }
 
+#ifdef HAVE_QUERY_CACHE
+/**
+  @brief Register a named table with a call back function to the query cache.
+
+  @param thd The thread handle
+  @param table_key A pointer to the table name in the table cache
+  @param key_length The length of the table name
+  @param[out] engine_callback The pointer to the storage engine call back
+    function, currently 0
+  @param[out] engine_data Engine data will be set to 0.
+
+  @note Despite the name of this function, it is used to check each statement
+    before it is cached and not to register a table or callback function.
+
+  @see handler::register_query_cache_table
+
+  @return The error code. The engine_data and engine_callback will be set to 0.
+    @retval TRUE Success
+    @retval FALSE An error occured
+*/
+
+my_bool ha_myisam::register_query_cache_table(THD *thd, char *table_name,
+                                              uint table_name_len,
+                                              qc_engine_callback
+                                              *engine_callback,
+                                              ulonglong *engine_data)
+{
+  /*
+    No call back function is needed to determine if a cached statement
+    is valid or not.
+  */
+  *engine_callback= 0;
+
+  /*
+    No engine data is needed.
+  */
+  *engine_data= 0;
+
+  /*
+    If a concurrent INSERT has happened just before the currently processed
+    SELECT statement, the total size of the table is unknown.
+
+    To determine if the table size is known, the current thread's snap shot of
+    the table size with the actual table size are compared.
+
+    If the table size is unknown the SELECT statement can't be cached.
+  */
+  ulonglong actual_data_file_length;
+  ulonglong current_data_file_length;
+
+  actual_data_file_length= file->s->state.state.data_file_length;
+  current_data_file_length= file->save_state.data_file_length;
+
+  if (current_data_file_length != actual_data_file_length)
+  {
+    /* Don't cache current statement. */
+    return FALSE;
+  }
+
+  /* It is ok to try to cache current statement. */
+  return TRUE;
+}
+#endif

--- 1.72/sql/ha_myisam.h	2006-12-30 21:02:06 +01:00
+++ 1.73/sql/ha_myisam.h	2007-07-11 08:46:54 +02:00
@@ -127,4 +127,11 @@ class ha_myisam: public handler
   int dump(THD* thd, int fd);
   int net_read_dump(NET* net);
 #endif
+#ifdef HAVE_QUERY_CACHE
+  my_bool register_query_cache_table(THD *thd, char *table_key,
+                                     uint key_length,
+                                     qc_engine_callback
+                                     *engine_callback,
+                                     ulonglong *engine_data);
+#endif
 };

--- 1.183/sql/handler.h	2007-03-30 10:00:20 +02:00
+++ 1.184/sql/handler.h	2007-07-11 08:46:54 +02:00
@@ -841,16 +841,49 @@ public:
 
   /* Type of table for caching query */
   virtual uint8 table_cache_type() { return HA_CACHE_TBL_NONTRANSACT; }
-  /* ask handler about permission to cache table when query is to be cached */
+
+
+  /**
+    @brief Register a named table with a call back function to the query cache.
+
+    @param thd The thread handle
+    @param table_key A pointer to the table name in the table cache
+    @param key_length The length of the table name
+    @param[out] engine_callback The pointer to the storage engine call back
+      function
+    @param[out] engine_data Storage engine specific data which could be
+      anything
+
+    This method offers the storage engine, the possibility to store a reference
+    to a table name which is going to be used with query cache. 
+    The method is called each time a statement is written to the cache and can
+    be used to verify if a specific statement is cachable. It also offers
+    the possibility to register a generic (but static) call back function which
+    is called each time a statement is matched against the query cache.
+
+    @note If engine_data supplied with this function is different from
+      engine_data supplied with the callback function, and the callback returns
+      FALSE, a table invalidation on the current table will occur.
+
+    @return Upon success the engine_callback will point to the storage engine
+      call back function, if any, and engine_data will point to any storage
+      engine data used in the specific implementation.
+      @retval TRUE Success
+      @retval FALSE The specified table or current statement should not be
+        cached
+  */
+
   virtual my_bool register_query_cache_table(THD *thd, char *table_key,
-					     uint key_length,
-					     qc_engine_callback
-					     *engine_callback,
-					     ulonglong *engine_data)
+                                             uint key_length,
+                                             qc_engine_callback
+                                             *engine_callback,
+                                             ulonglong *engine_data)
   {
     *engine_callback= 0;
-    return 1;
+    return TRUE;
   }
+
+
  /*
   RETURN
     true  Primary key (if there is one) is clustered key covering all fields

--- 1.80/mysql-test/r/query_cache.result	2007-05-08 12:18:33 +02:00
+++ 1.81/mysql-test/r/query_cache.result	2007-07-11 08:46:54 +02:00
@@ -1409,3 +1409,38 @@ 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;
+set GLOBAL query_cache_type=1;
+set GLOBAL query_cache_limit=10000;
+set GLOBAL query_cache_min_res_unit=0;
+set GLOBAL query_cache_size= 100000;
+flush tables;
+drop table if exists t1, t2;
+create table t1 (a int);
+create table t2 (a int);
+insert into t1 values (1),(2),(3);
+lock table t2 write;
+select *, (select count(*) from t2) from t1;;
+insert into t1 values (4);
+unlock tables;
+a	(select count(*) from t2)
+1	0
+2	0
+3	0
+select *, (select count(*) from t2) from t1;
+a	(select count(*) from t2)
+1	0
+2	0
+3	0
+4	0
+reset query cache;
+select *, (select count(*) from t2) from t1;
+a	(select count(*) from t2)
+1	0
+2	0
+3	0
+4	0
+drop table t1,t2;
+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;

--- 1.59/mysql-test/t/query_cache.test	2007-05-08 11:24:05 +02:00
+++ 1.60/mysql-test/t/query_cache.test	2007-07-11 08:46:54 +02:00
@@ -970,4 +970,57 @@ set GLOBAL query_cache_limit=default;
 set GLOBAL query_cache_min_res_unit=default;
 set GLOBAL query_cache_size= default;
 
+#
+# Bug #28249 Query Cache returns wrong result with concurrent insert / certain lock
+#
+connect (user1,localhost,root,,test,,);
+connect (user2,localhost,root,,test,,);
+connect (user3,localhost,root,,test,,);
+
+connection user1;
+
+set GLOBAL query_cache_type=1;
+set GLOBAL query_cache_limit=10000;
+set GLOBAL query_cache_min_res_unit=0;
+set GLOBAL query_cache_size= 100000;
+
+flush tables;
+--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);
+
+connection user2;
+lock table t2 write;
+
+connection user1;
+--send select *, (select count(*) from t2) from t1;
+
+connection user3;
+insert into t1 values (4);
+
+connection user2;
+unlock tables;
+
+connection user1;
+## This should ALL return 4 rows, as the insert is long finished.
+--reap
+select *, (select count(*) from t2) from t1;
+reset query cache;
+select *, (select count(*) from t2) from t1;
+
+drop table t1,t2;
+
+connection default;
+disconnect user1;
+disconnect user2;
+disconnect user3;
+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;
+
 # End of 5.0 tests
+
Thread
bk commit into 5.0 tree (thek:1.2489) BUG#28249kpettersson11 Jul
  • Re: bk commit into 5.0 tree (thek:1.2489) BUG#28249Sergei Golubchik11 Jul