Bug#43758 Query cache can lock up threads in 'freeing items' state

Early patch submitted for discussion.

It is possible for more than one thread to enter the condition
in query_cache_insert(), but the condition predicate is to
signal one thread each time the cache status changes between
the following states: {NO_FLUSH_IN_PROGRESS,FLUSH_IN_PROGRESS,
TABLE_FLUSH_IN_PROGRESS}

Consider three threads THD1, THD2, THD3

   THD2: select ... => Got a writer in ::store_query
   THD3: select ... => Got a writer in ::store_query
   THD1: flush tables => qc status= FLUSH_IN_PROGRESS;
                      new writers are blocked.
   THD2: select ... => Still got a writer and enters cond in
                       query_cache_insert
   THD3: select ... => Still got a writer and enters cond in
                       query_cache_insert
   THD1: flush tables => finished and signal status change.
   THD2: select ... => Wakes up and completes the insert.
   THD3: select ... => Happily waiting for better times. Why hurry?

This patch is a refactoring of this lock system. It introduces four new methods:
   Query_cache::try_lock()
   Query_cache::lock()
   Query_cache::lock_and_suspend()
   Query_cache::unlock()

This change also deprecates wait_while_table_flush_is_in_progress(). All threads are
queued and put on a conditional wait. On each unlock the queue is signalled. This resolve
the issues with left over threads. To assure that no threads are spending unnecessary
time waiting a signal broadcast is issued every time a lock is taken before a full
cache flush.

mysql-test/r/query_cache_debug.result:
  * Added test case for bug43758
mysql-test/t/query_cache_debug.test:
  * Added test case for bug43758
sql/sql_cache.cc:
  * Replaced calls to wait_while_table_flush_is_in_progress() with
    calls to try_lock(), lock_and_suspend() and unlock().
  * Renamed enumeration Cache_status to Cache_lock_status.
  * Renamed enumeration items to UNLOCKED, LOCKED_NO_WAIT and LOCKED.
    If the LOCKED_NO_WAIT lock type is used to lock the query cache, other
    threads using try_lock() will fail to acquire the lock.
    This is useful if the query cache is temporary disabled due to 
    a full table flush.
sql/sql_cache.h:
  * Replaced calls to wait_while_table_flush_is_in_progress() with
    calls to try_lock(), lock_and_suspend() and unlock().
  * Renamed enumeration Cache_status to Cache_lock_status.
  * Renamed enumeration items to UNLOCKED, LOCKED_NO_WAIT and LOCKED.
    If the LOCKED_NO_WAIT lock type is used to lock the query cache, other
    threads using try_lock() will fail to acquire the lock.
    This is useful if the query cache is temporary disabled due to 
    a full table flush.

1 files changed, 10 insertions, 12 deletions

diff --git a/sql/sql_cache.h b/sql/sql_cache.h
index f2c33eff614..777ddd39280 100644
--- a/sql/sql_cache.h
+++ b/sql/sql_cache.h
@@ -272,12 +272,12 @@ public:
 
 
 private:
+#ifndef DBUG_OFF
+  my_thread_id m_cache_lock_thread_id;
+#endif
   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;
+  enum Cache_lock_status { UNLOCKED, LOCKED_NO_WAIT, LOCKED };
+  Cache_lock_status m_cache_lock_status;
 
   void free_query_internal(Query_cache_block *point);
   void invalidate_table_internal(THD *thd, uchar *key, uint32 key_length);
@@ -380,8 +380,6 @@ protected:
 	      Query_cache_block *pprev);
   my_bool join_results(ulong join_limit);
 
-  void wait_while_table_flush_is_in_progress(bool *interrupt);
-
   /*
     Following function control structure_guard_mutex
     by themself or don't need structure_guard_mutex
@@ -469,11 +467,6 @@ protected:
   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::NO_FLUSH_IN_PROGRESS);
-  }
-
   /*
     The following functions are only used when debugging
     We don't protect these with ifndef DBUG_OFF to not have to recompile
@@ -491,6 +484,11 @@ protected:
 			Query_cache_block_table * point,
 			const char *name);
   my_bool in_blocks(Query_cache_block * point);
+
+  bool try_lock(void);
+  void lock(void);
+  void lock_and_suspend(void);
+  void unlock(void);
 };
 
 extern Query_cache query_cache;