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-10-03 00:49:07+02:00, thek@adventure.(none) +2 -0
Bug #30887 Server crashes on SET GLOBAL query_cache_size=0
Reseting the query cache by issuing a SET GLOBAL query_cache_size=0 would
cause the server to crash if a the server was saving a new result set to the
query cache.
This patch make the invalidation thread wait on the cache result writers to
finish before the underlying memory is released.
sql/sql_cache.cc@stripped, 2007-10-03 00:49:04+02:00, thek@adventure.(none) +210 -84
- Added mutex assertions for extra safety.
- Added Query_cache methods: drop_writer, wait_until_no_active_writers,
add_reader, drop_reader, add_writer, lock_query_cache_exclusively,
unlock_query_cache, wait_until_no_active_readers.
- Refactored query_cache_end_of_result to always terminate result writer if
possible.
- Added reference variable counting active writers and readers.
- Fixed bug for embedded access which doesn't support grant privileges.
- Added guards against wrong locking order.
- Patched bug in memory bin search algorithm.
sql/sql_cache.h@stripped, 2007-10-03 00:49:04+02:00, thek@adventure.(none) +41 -0
- Added mutex assertions for extra safety.
- Added Query_cache methods: drop_writer, wait_until_no_active_writers,
add_reader, drop_reader, add_writer, lock_query_cache_exclusively,
unlock_query_cache, wait_until_no_active_readers.
- Refactored query_cache_end_of_result to always terminate result writer if
possible.
- Added reference variable counting active writers and readers.
- Fixed bug for embedded access which doesn't support grant privileges.
- Added guards against wrong locking order.
- Patched bug in memory bin search algorithm.
diff -Nrup a/sql/sql_cache.cc b/sql/sql_cache.cc
--- a/sql/sql_cache.cc 2007-09-04 17:43:26 +02:00
+++ b/sql/sql_cache.cc 2007-10-03 00:49:04 +02:00
@@ -648,13 +648,6 @@ void query_cache_insert(NET *net, const
DBUG_VOID_RETURN;
STRUCT_LOCK(&query_cache.structure_guard_mutex);
- bool interrupt;
- query_cache.wait_while_table_flush_is_in_progress(&interrupt);
- if (interrupt)
- {
- STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
- return;
- }
Query_cache_block *query_block= (Query_cache_block*)net->query_cache_query;
if (!query_block)
@@ -667,11 +660,12 @@ void query_cache_insert(NET *net, const
DBUG_VOID_RETURN;
}
+ safe_mutex_assert_owner(&query_cache.structure_guard_mutex);
+ BLOCK_LOCK_WR(query_block);
Query_cache_query *header= query_block->query();
Query_cache_block *result= header->result();
DUMP(&query_cache);
- BLOCK_LOCK_WR(query_block);
DBUG_PRINT("qcache", ("insert packet %lu bytes long",length));
/*
@@ -685,6 +679,7 @@ void query_cache_insert(NET *net, const
DBUG_PRINT("warning", ("Can't append data"));
header->result(result);
DBUG_PRINT("qcache", ("free query 0x%lx", (ulong) query_block));
+ query_cache.drop_writer(query_block);
// The following call will remove the lock on query_block
query_cache.free_query(query_block);
// append_result_data no success => we need unlock
@@ -727,7 +722,9 @@ void query_cache_abort(NET *net)
if (query_block)
{
DUMP(&query_cache);
+ safe_mutex_assert_owner(&query_cache.structure_guard_mutex);
BLOCK_LOCK_WR(query_block);
+ query_cache.drop_writer(query_block);
// The following call will remove the lock on query_block
query_cache.free_query(query_block);
net->query_cache_query= 0;
@@ -735,7 +732,6 @@ void query_cache_abort(NET *net)
}
STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
-
DBUG_VOID_RETURN;
}
@@ -762,14 +758,6 @@ void query_cache_end_of_result(THD *thd)
STRUCT_LOCK(&query_cache.structure_guard_mutex);
- bool interrupt;
- query_cache.wait_while_table_flush_is_in_progress(&interrupt);
- if (interrupt)
- {
- STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
- DBUG_VOID_RETURN;
- }
-
query_block= ((Query_cache_block*) thd->net.query_cache_query);
if (query_block)
{
@@ -787,14 +775,16 @@ void query_cache_end_of_result(THD *thd)
if (header->result() == 0)
{
- DBUG_PRINT("error", ("End of data with no result blocks; "
- "Query '%s' removed from cache.", header->query()));
/*
- Extra safety: empty result should not happen in the normal call
- to this function. In the release version that query should be ignored
- and removed from QC.
+ This can happen if query cache is being invalidated under
+ m_cache_status= Query_cache::FLUSH_IN_PROGRESS or
+ Query_cache::TABLE_FLUSH_IN_PROGRESS
*/
- DBUG_ASSERT(0);
+ DBUG_PRINT("error", ("End of data with no result blocks; "
+ "Query '%s' removed from cache.", header->query()));
+ /* Drop the writer. */
+ query_cache.drop_writer(query_block);
+ /* free_query() will unlock the query_block */
query_cache.free_query(query_block);
STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
DBUG_VOID_RETURN;
@@ -810,15 +800,16 @@ void query_cache_end_of_result(THD *thd)
header->result()->type= Query_cache_block::RESULT;
/* Drop the writer. */
- header->writer(0);
- thd->net.query_cache_query= 0;
+ query_cache.drop_writer(query_block);
BLOCK_UNLOCK_WR(query_block);
+
DBUG_EXECUTE("check_querycache",query_cache.check_integrity(1););
}
STRUCT_UNLOCK(&query_cache.structure_guard_mutex);
+
DBUG_VOID_RETURN;
}
@@ -877,23 +868,17 @@ ulong Query_cache::resize(ulong query_ca
query_cache_size_arg));
DBUG_ASSERT(initialized);
- STRUCT_LOCK(&structure_guard_mutex);
- 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);
+ lock_query_cache_exclusively();
free_cache();
query_cache_size= query_cache_size_arg;
new_query_cache_size= init_cache();
- STRUCT_LOCK(&structure_guard_mutex);
- m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
- pthread_cond_signal(&COND_cache_status_changed);
if (new_query_cache_size)
DBUG_EXECUTE("check_querycache",check_integrity(1););
- STRUCT_UNLOCK(&structure_guard_mutex);
+
+ unlock_query_cache();
DBUG_RETURN(new_query_cache_size);
}
@@ -1066,10 +1051,9 @@ def_week_frmt: %lu",
double_linked_list_simple_include(query_block, &queries_blocks);
inserts++;
queries_in_cache++;
- net->query_cache_query= (uchar*) query_block;
- header->writer(net);
- header->tables_type(tables_type);
-
+ header->tables_type(tables_type);
+ /* Register the result writer */
+ add_writer(thd,query_block);
STRUCT_UNLOCK(&structure_guard_mutex);
// init_n_lock make query block locked
@@ -1259,6 +1243,7 @@ def_week_frmt: %lu",
DBUG_PRINT("qcache", ("Query in query hash 0x%lx", (ulong)query_block));
/* Now lock and test that nothing changed while blocks was unlocked */
+ safe_mutex_assert_owner(&structure_guard_mutex);
BLOCK_LOCK_RD(query_block);
query = query_block->query();
@@ -1371,6 +1356,7 @@ def_week_frmt: %lu",
}
move_to_query_list_end(query_block);
hits++;
+ add_reader();
STRUCT_UNLOCK(&structure_guard_mutex);
/*
@@ -1403,8 +1389,10 @@ def_week_frmt: %lu",
thd->limit_found_rows = query->found_rows();
thd->status_var.last_query_cost= 0.0;
-
BLOCK_UNLOCK_RD(query_block);
+ pthread_mutex_lock(&structure_guard_mutex);
+ drop_reader();
+ pthread_mutex_unlock(&structure_guard_mutex);
DBUG_RETURN(1); // Result sent to client
err_unlock:
@@ -1565,6 +1553,83 @@ void Query_cache::wait_while_table_flush
/**
+ @brief Wait on the predicate m_number_of_active_writers==0 and then locks
+ structure_guard_mutex.
+
+ @pre Query_cache::m_status has to be FLUSH_IN_PROGRESS or
+ TABLE_FLUSH_IN_PROGRESS to be able to prevent new writers from being
+ created and structure_guard_mutex has to be locked.
+*/
+
+void Query_cache::wait_until_no_active_writers(void)
+{
+ safe_mutex_assert_owner(&structure_guard_mutex);
+ while (m_number_of_active_writers > 0)
+ pthread_cond_wait(&COND_no_active_writers, &structure_guard_mutex);
+}
+
+/**
+ @brief Wait on the predicate m_number_of_active_readers==0 and then locks
+ structure_guard_mutex.
+
+ @pre Query_cache::m_status has to be FLUSH_IN_PROGRESS or
+ TABLE_FLUSH_IN_PROGRESS to be able to prevent new readers from being
+ created and structure_guard_mutex has to be locked.
+*/
+void Query_cache::wait_until_no_active_readers(void)
+{
+ safe_mutex_assert_owner(&structure_guard_mutex);
+ while (m_number_of_active_readers > 0)
+ pthread_cond_wait(&COND_no_active_readers, &structure_guard_mutex);
+}
+
+/**
+ @brief Assures that no other writer- or reader-threads are accessing
+ query cache data.
+ @see Query_cache::unlock_query_cache(void)
+*/
+
+void Query_cache::lock_query_cache_exclusively(void)
+{
+ pthread_mutex_lock(&structure_guard_mutex);
+ while( is_flushing() )
+ pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex);
+ /* Prevent new queries to enter the cache */
+ m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
+
+ /*
+ Predicate dependencies:
+ m_number_of_active_writers == 0;
+ depends on m_cache_status == FLUSH_IN_PROGRESS
+ m_number_of_active_readers == 0;
+ depends on m_cache_status == FLUSH_IN_PROGRESS
+ */
+ wait_until_no_active_writers();
+ wait_until_no_active_readers();
+
+ pthread_mutex_unlock(&structure_guard_mutex);
+}
+
+/**
+ @brief Allow other threads to access query cache data.
+
+ Other threads locked out by lock_query_cache_exclusively will be queuing
+ on COND_cache_status_changed. This function changes the query cache status
+ to NO_FLUSH_IN_PROGRESS to allow other threads access and signal any threads
+ waiting in queue.
+
+ @see Query_cache::lock_query_cache_exclusively(void)
+*/
+
+void Query_cache::unlock_query_cache(void)
+{
+ pthread_mutex_lock(&structure_guard_mutex);
+ m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+ pthread_cond_broadcast(&COND_cache_status_changed);
+ pthread_mutex_unlock(&structure_guard_mutex);
+}
+
+/**
@brief Remove all cached queries that uses the given database
*/
@@ -1581,6 +1646,7 @@ void Query_cache::invalidate(char *db)
STRUCT_UNLOCK(&structure_guard_mutex);
return;
}
+ m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
THD *thd= current_thd;
@@ -1636,6 +1702,9 @@ void Query_cache::invalidate(char *db)
} while (restart);
} // end if( tables_blocks )
}
+
+ m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+ pthread_cond_broadcast(&COND_cache_status_changed);
STRUCT_UNLOCK(&structure_guard_mutex);
DBUG_VOID_RETURN;
@@ -1660,7 +1729,7 @@ void Query_cache::invalidate_by_MyISAM_f
void Query_cache::flush()
{
DBUG_ENTER("Query_cache::flush");
- STRUCT_LOCK(&structure_guard_mutex);
+ lock_query_cache_exclusively();
if (query_cache_size > 0)
{
DUMP(this);
@@ -1669,7 +1738,7 @@ void Query_cache::flush()
}
DBUG_EXECUTE("check_querycache",query_cache.check_integrity(1););
- STRUCT_UNLOCK(&structure_guard_mutex);
+ unlock_query_cache();
DBUG_VOID_RETURN;
}
@@ -1703,12 +1772,16 @@ void Query_cache::pack(ulong join_limit,
DBUG_VOID_RETURN;
}
+ m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
+
uint i = 0;
do
{
pack_cache();
} while ((++i < iteration_limit) && join_results(join_limit));
+ m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
+ pthread_cond_broadcast(&COND_cache_status_changed);
STRUCT_UNLOCK(&structure_guard_mutex);
DBUG_VOID_RETURN;
}
@@ -1725,10 +1798,13 @@ void Query_cache::destroy()
{
/* Underlying code expects the lock. */
STRUCT_LOCK(&structure_guard_mutex);
+
free_cache();
+
STRUCT_UNLOCK(&structure_guard_mutex);
pthread_cond_destroy(&COND_cache_status_changed);
+ pthread_cond_destroy(&COND_no_active_writers);
pthread_mutex_destroy(&structure_guard_mutex);
initialized = 0;
}
@@ -1746,7 +1822,9 @@ void Query_cache::init()
pthread_mutex_init(&structure_guard_mutex,MY_MUTEX_INIT_FAST);
pthread_cond_init(&COND_cache_status_changed, NULL);
m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
- initialized = 1;
+ initialized= 1;
+ m_number_of_active_writers= 0;
+ m_number_of_active_readers= 0;
DBUG_VOID_RETURN;
}
@@ -1984,34 +2062,12 @@ void Query_cache::free_cache()
void Query_cache::flush_cache()
{
- /*
- If there is flush in progress, wait for it to finish, and then do
- our flush. This is necessary because something could be added to
- the cache before we acquire the lock again, and some code (like
- Query_cache::free_cache()) depends on the fact that after the
- flush the cache is empty.
- */
- while (is_flushing())
- pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex);
-
- /*
- 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.
- */
- m_cache_status= Query_cache::FLUSH_IN_PROGRESS;
- STRUCT_UNLOCK(&structure_guard_mutex);
-
my_hash_reset(&queries);
while (queries_blocks != 0)
{
BLOCK_LOCK_WR(queries_blocks);
free_query_internal(queries_blocks);
}
-
- STRUCT_LOCK(&structure_guard_mutex);
- m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS;
- pthread_cond_signal(&COND_cache_status_changed);
}
/*
@@ -2034,6 +2090,7 @@ my_bool Query_cache::free_old_query()
{
Query_cache_block *block = queries_blocks;
/* Search until we find first query that we can remove */
+ safe_mutex_assert_owner(&structure_guard_mutex);
do
{
Query_cache_query *header = block->query();
@@ -2049,6 +2106,7 @@ my_bool Query_cache::free_old_query()
if (query_block != 0)
{
+ drop_writer(query_block);
free_query(query_block);
lowmem_prunes++;
DBUG_RETURN(0);
@@ -2058,21 +2116,17 @@ my_bool Query_cache::free_old_query()
}
-/*
- free_query_internal() - free query from query cache.
+/**
+ @brief Free query from cache and decreases the cached queries count.
- SYNOPSIS
- free_query_internal()
- query_block Query_cache_block representing the query
+ This function will remove the query from the cache, and place its
+ memory blocks to the list of free blocks. 'query_block' must be
+ locked for writing, this function will release (and destroy) this
+ lock.
- DESCRIPTION
- This function will remove the query from a cache, and place its
- memory blocks to the list of free blocks. 'query_block' must be
- locked for writing, this function will release (and destroy) this
- lock.
+ @param[in,out] query_block Query_cache_block representing the query
- NOTE
- 'query_block' should be removed from 'queries' hash _before_
+ @note 'query_block' should be removed from 'queries' hash _before_
calling this method, as the lock will be destroyed here.
*/
@@ -2087,12 +2141,8 @@ void Query_cache::free_query_internal(Qu
Query_cache_query *query= query_block->query();
- if (query->writer() != 0)
- {
- /* Tell MySQL that this query should not be cached anymore */
- query->writer()->query_cache_query= 0;
- query->writer(0);
- }
+ DBUG_ASSERT (query->writer() == 0);
+
double_linked_list_exclude(query_block, &queries_blocks);
Query_cache_block_table *table= query_block->table(0);
@@ -2490,7 +2540,7 @@ void Query_cache::invalidate_table(THD *
net_real_write might be waiting on a change on the m_cache_status
variable.
*/
- pthread_cond_signal(&COND_cache_status_changed);
+ pthread_cond_broadcast(&COND_cache_status_changed);
STRUCT_UNLOCK(&structure_guard_mutex);
}
@@ -2531,13 +2581,83 @@ void
Query_cache::invalidate_query_block_list(THD *thd,
Query_cache_block_table *list_root)
{
+ int dropped_writers= 0;
while (list_root->next != list_root)
{
Query_cache_block *query_block= list_root->next->block();
BLOCK_LOCK_WR(query_block);
+ /*
+ It is safe to remove writer dependencies under block level lock
+ but we need to remember how many we remove so that we later
+ can send the proper signals.
+ */
+ if (query_block->query()->writer() != 0)
+ {
+ dropped_writers++;
+ query_block->query()->writer(0);
+ }
free_query(query_block);
DBUG_EXECUTE_IF("debug_cache_locks", sleep(10););
}
+
+ /* Lock global structure mutex and update number of active writers */
+ pthread_mutex_lock(&structure_guard_mutex);
+ m_number_of_active_writers -= dropped_writers;
+ /* Did we fulfill a conditional predicate? */
+ if (m_number_of_active_writers == 0)
+ pthread_cond_broadcast(&COND_no_active_writers);
+ pthread_mutex_unlock(&structure_guard_mutex);
+
+}
+
+
+/**
+ @brief Unregister any writer associated with the query block.
+
+ @param block The query cache block which might be associated with an active
+ writer.
+
+ @pre This function should be called under structure_guard_mutex protection.
+*/
+
+void Query_cache::drop_writer(Query_cache_block *block)
+{
+ safe_mutex_assert_owner(&structure_guard_mutex);
+ Query_cache_query *query= block->query();
+ if (query != 0 && query->writer() != 0)
+ {
+ DBUG_ASSERT(m_number_of_active_writers > 0);
+ query->writer()->query_cache_query= 0;
+ query->writer(0);
+ --m_number_of_active_writers;
+ if (m_number_of_active_writers == 0)
+ pthread_cond_broadcast(&COND_no_active_writers);
+ }
+}
+
+void Query_cache::drop_reader(void)
+{
+ --m_number_of_active_readers;
+ if(m_number_of_active_readers == 0)
+ pthread_cond_broadcast(&COND_no_active_readers);
+}
+
+/**
+ @brief Register a result writer and attach it to a query block.
+
+ @param thd The thread session handler.
+ @param header The query block (of query-type) which will be associated with
+ the result writer.
+
+ @pre This function should be called under structure_guard_mutex protection.
+*/
+
+void Query_cache::add_writer(THD *thd, Query_cache_block *block)
+{
+ Query_cache_query *header= block->query();
+ thd->net.query_cache_query= (uchar*) block;
+ ++m_number_of_active_writers;
+ header->writer(&thd->net);
}
/*
@@ -3075,6 +3195,8 @@ void Query_cache::insert_into_free_memor
uint Query_cache::find_bin(ulong size)
{
DBUG_ENTER("Query_cache::find_bin");
+ if (size < min_allocation_unit)
+ DBUG_RETURN(mem_bin_steps);
// Binary search
int left = 0, right = mem_bin_steps;
do
@@ -3240,7 +3362,7 @@ Query_cache::process_and_count_tables(TH
for (; tables_used; tables_used= tables_used->next_global)
{
table_count++;
-#ifdef HAVE_QUERY_CACHE
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
/*
Disable any attempt to store this statement if there are
column level grants on any referenced tables.
@@ -3551,6 +3673,7 @@ my_bool Query_cache::move_by_type(uchar
DBUG_PRINT("qcache", ("block 0x%lx QUERY", (ulong) block));
if (*border == 0)
break;
+ safe_mutex_assert_owner(&structure_guard_mutex);
BLOCK_LOCK_WR(block);
ulong len = block->length, used = block->used;
TABLE_COUNTER_TYPE n_tables = block->n_tables;
@@ -3651,6 +3774,7 @@ my_bool Query_cache::move_by_type(uchar
*next = block->next,
*prev = block->prev;
Query_cache_block::block_type type = block->type;
+ safe_mutex_assert_owner(&structure_guard_mutex);
BLOCK_LOCK_WR(query_block);
ulong len = block->length, used = block->used;
Query_cache_block *pprev = block->pprev,
@@ -3754,6 +3878,7 @@ my_bool Query_cache::join_results(ulong
if (new_result_block->length >
ALIGN_SIZE(new_len) + min_allocation_unit)
split_block(new_result_block, ALIGN_SIZE(new_len));
+ safe_mutex_assert_owner(&structure_guard_mutex);
BLOCK_LOCK_WR(block);
header->result(new_result_block);
new_result_block->type = Query_cache_block::RESULT;
@@ -4176,6 +4301,7 @@ my_bool Query_cache::check_integrity(boo
}
else
{
+ safe_mutex_assert_owner(&structure_guard_mutex);
BLOCK_LOCK_RD(query_block);
if (in_list(queries_blocks, query_block, "query from results"))
result = 1;
diff -Nrup a/sql/sql_cache.h b/sql/sql_cache.h
--- a/sql/sql_cache.h 2007-08-17 17:26:35 +02:00
+++ b/sql/sql_cache.h 2007-10-03 00:49:04 +02:00
@@ -274,13 +274,54 @@ public:
private:
pthread_cond_t COND_cache_status_changed;
+ /* m_number_of_active_writers == 0 */
+ pthread_cond_t COND_no_active_writers;
+
+ /* m_number_of_active_readers == 0 */
+ pthread_cond_t COND_no_active_readers;
+
enum Cache_status { NO_FLUSH_IN_PROGRESS, FLUSH_IN_PROGRESS,
TABLE_FLUSH_IN_PROGRESS };
Cache_status m_cache_status;
+
+ /**
+ The number of active result writers.
+
+ This value is updated in six places:
+ 1) Query_cache::store_query() - When the writer is first registered.
+ 2) query_cache_end_of_result() - When ever the current thread passes
+ this function regardless if there still
+ is an active result writer or not.
+ 3) query_cache_abort - Drop the writer if the query is aborted.
+ 4) query_cache_insert - Drop the writer if an error occurs while
+ inserting
+ 5) Query_cache::free_old_query - When a low memory prune occurs.
+ 6) Query_cache::invalidate_query_block_list - When a query associated
+ with a writer is invalidated.
+ */
+
+ int m_number_of_active_writers;
+
+ /**
+ The number of active readers are counted over a critical region in
+ the send_result_to_client function where net_real_write is called in
+ a loop until the entire cached result set is returned.
+ */
+
+ int m_number_of_active_readers;
+
void free_query_internal(Query_cache_block *point);
void invalidate_table_internal(THD *thd, uchar *key, uint32 key_length);
+ void wait_until_no_active_writers(void);
+ void wait_until_no_active_readers(void);
+ void lock_query_cache_exclusively(void);
+ void unlock_query_cache(void);
+ void drop_writer(Query_cache_block *block);
+ void add_writer(THD *thd, Query_cache_block *block);
+ void add_reader() { ++m_number_of_active_readers; }
+ void drop_reader();
protected:
/*
| Thread |
|---|
| • bk commit into 5.1 tree (thek:1.2589) BUG#30887 | kpettersson | 3 Oct |