From 289cc26c646f5a45e2c9fc6c0bb75f6f61d301a1 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 2 Jul 2007 19:14:48 +0200 Subject: [PATCH 1/7] Bug#21074 Large query_cache freezes mysql server sporadically under heavy load Invaldating a subset of a sufficiently large query cache can take a long time. During this time the server is efficiently frozen and no other operation can be executed. This patch addresses this problem by moving the locks which cause the freezing and also by temporarily disable the query cache while the invalidation takes place. sql/ha_ndbcluster.cc: - mysql_rm_table_part2 has a new parameter to indicate if OPEN_lock mutex protection is needed. sql/lock.cc: - Added function for acquiring table name exclusive locks. - Added function for asserting that table name lock is acquired. sql/mysql_priv.h: - Added function for acquiring table name exclusive locks. - Added function for asserting that table name lock is acquired. - Added parameter to mysql_rm_table_part2 to indicate whether OPEN_lock mutex protection is needed or not. sql/sql_cache.cc: - Changed flush_in_progress-flag into a state and added a function, is_flushing() to reflect on this state. A new state was needed to indicate that a partial invalidation was in progress. - An unused parameter 'under_guard' was removed. - The Query_cache mutex structural_guard was pushed down into one invalidate_table function to avoid multiple entry points which makes maintainens more difficult. - Instead of keeping the structural_guard mutex during the entire invalidation we set the query cache status state to TABLE_FLUSH_IN_PROGRESS to temporarily disable the cache and avoid locking other threads needing the Query_cache resource. sql/sql_cache.h: - Changed flush_in_progress-flag into a state and added a function, is_flushing() to reflect on this state. A new state was needed to indicate that a partial invalidation was in progress. - An unused parameter 'under_guard' was removed. - The Query_cache mutex structural_guard was pushed down into one invalidate_table function to avoid multiple entry points which makes maintainens more difficult. - Instead of keeping the structural_guard mutex during the entire invalidation we set the query cache status state to TABLE_FLUSH_IN_PROGRESS to temporarily disable the cache and avoid locking other threads needing the the Query_cache resource. sql/sql_db.cc: - mysql_rm_table_part2_with_lock is redundant and replaced with mysql_rm_table_part2. sql/sql_parse.cc: - Function query_cache_invalidate3 isn't protect by a lock and we have a race condition. - Moving this function into mysql_rename_tables and make sure it is protected by a exclusive table name lock. sql/sql_rename.cc: - Function query_cache_invalidation3 isn't protect by a lock and we have a race condition. - Moving this function into mysql_rename_tables and make sure it is protected by a exclusive table name lock. - Instead of using LOCK_open mutex, which excludes all other threads, the lock is changed into exclusive table name locks instead. This prevents us from locking the server if a query cache invalidation would take a long time to complete. sql/sql_table.cc: - Instead of using LOCK_open mutex, which excludes all other threads, the lock is changed into exclusive table name locks instead. This prevents us from locking the server if a query cache invalidation would take a long time to complete. - Added new parameter to mysql_rm_table_part2 to control whether OPEN_lock mutex needs to be aquired or not. This is currently needed by the NDB implemenation. sql/sql_trigger.cc: - Table_triggers don't need to be protexted by LOCK_open mutex. This patch cancel this restriction. - Refactored comments to doxygen style. --- sql/ha_ndbcluster.cc | 13 +- sql/lock.cc | 96 ++++++ sql/mysql_priv.h | 11 +- sql/sql_cache.cc | 715 +++++++++++++++++++++++++++---------------- sql/sql_cache.h | 72 ++++- sql/sql_db.cc | 2 +- sql/sql_parse.cc | 2 +- sql/sql_rename.cc | 16 +- sql/sql_table.cc | 82 ++--- sql/sql_trigger.cc | 40 ++- 10 files changed, 677 insertions(+), 372 deletions(-) diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index aeaea90feb6..8e49ebcbe51 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -6996,7 +6996,6 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, // Lock mutex before deleting and creating frm files pthread_mutex_lock(&LOCK_open); - if (!global_read_lock) { // Delete old files @@ -7010,10 +7009,12 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, table_list.db= (char*) db; table_list.alias= table_list.table_name= (char*)file_name; (void)mysql_rm_table_part2(thd, &table_list, - /* if_exists */ FALSE, - /* drop_temporary */ FALSE, - /* drop_view */ FALSE, - /* dont_log_query*/ TRUE); + FALSE, /* if_exists */ + FALSE, /* drop_temporary */ + FALSE, /* drop_view */ + TRUE, /* dont_log_query*/ + FALSE); /* need lock open */ + /* Clear error message that is returned when table is deleted */ thd->clear_error(); } @@ -7029,7 +7030,7 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, } pthread_mutex_unlock(&LOCK_open); - + hash_free(&ok_tables); hash_free(&ndb_tables); diff --git a/sql/lock.cc b/sql/lock.cc index 6f1dd0669ee..6c1196e29c0 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -1027,6 +1027,102 @@ end: } +/** + @brief Lock all tables in list with an exclusive table name lock. + + @param thd Thread handle. + @param table_list Names of tables to lock. + + @note This function needs to be protected by LOCK_open. If we're + under LOCK TABLES, this function does not work as advertised. Namely, + it does not exclude other threads from using this table and does not + put an exclusive name lock on this table into the table cache. + + @see lock_table_names + @see unlock_table_names + + @retval TRUE An error occured. + @retval FALSE Name lock successfully acquired. +*/ + +bool lock_table_names_exclusively(THD *thd, TABLE_LIST *table_list) +{ + if (lock_table_names(thd, table_list)) + return TRUE; + + /* + Upgrade the table name locks from semi-exclusive to exclusive locks. + */ + for (TABLE_LIST *table= table_list; table; table= table->next_global) + { + if (table->table) + table->table->open_placeholder= 1; + } + return FALSE; +} + + +/** + @brief Test is 'table' is protected by an exclusive name lock. + + @param[in] thd The current thread handler + @param[in] table Table container containing the single table to be tested + + @note Needs to be protected by LOCK_open mutex. + + @return Error status code + @retval TRUE Table is protected + @retval FALSE Table is not protected +*/ + +bool +is_table_name_exclusively_locked_by_this_thread(THD *thd, + TABLE_LIST *table_list) +{ + char key[MAX_DBKEY_LENGTH]; + uint key_length; + + key_length= create_table_def_key(thd, key, table_list, 0); + + return is_table_name_exclusively_locked_by_this_thread(thd, (uchar *)key, + key_length); +} + + +/** + @brief Test is 'table key' is protected by an exclusive name lock. + + @param[in] thd The current thread handler. + @param[in] table Table container containing the single table to be tested. + + @note Needs to be protected by LOCK_open mutex + + @retval TRUE Table is protected + @retval FALSE Table is not protected + */ + +bool +is_table_name_exclusively_locked_by_this_thread(THD *thd, uchar *key, + int key_length) +{ + HASH_SEARCH_STATE state; + TABLE *table; + + for (table= (TABLE*) hash_first(&open_cache, key, + key_length, &state); + table ; + table= (TABLE*) hash_next(&open_cache, key, + key_length, &state)) + { + if (table->in_use == thd && + table->open_placeholder == 1 && + table->s->version == 0) + return TRUE; + } + + return FALSE; +} + /* Unlock all tables in list with a name lock diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index f5d66696cab..3e6bdeb2775 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -820,10 +820,8 @@ void mysql_client_binlog_statement(THD *thd); bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, my_bool drop_temporary); int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, - bool drop_temporary, bool drop_view, bool log_query); -int mysql_rm_table_part2_with_lock(THD *thd, TABLE_LIST *tables, - bool if_exists, bool drop_temporary, - bool log_query); + bool drop_temporary, bool drop_view, bool log_query, + bool need_lock_open); bool quick_rm_table(handlerton *base,const char *db, const char *table_name, uint flags); void close_cached_table(THD *thd, TABLE *table); @@ -1799,6 +1797,11 @@ bool wait_for_locked_table_names(THD *thd, TABLE_LIST *table_list); bool lock_table_names(THD *thd, TABLE_LIST *table_list); void unlock_table_names(THD *thd, TABLE_LIST *table_list, TABLE_LIST *last_table); +bool lock_table_names_exclusively(THD *thd, TABLE_LIST *table_list); +bool is_table_name_exclusively_locked_by_this_thread(THD *thd, + TABLE_LIST *table_list); +bool is_table_name_exclusively_locked_by_this_thread(THD *thd, uchar *key, + int key_length); /* old unireg functions */ diff --git a/sql/sql_cache.cc b/sql/sql_cache.cc index 04f142c5dfb..bcf3c2b550c 100644 --- a/sql/sql_cache.cc +++ b/sql/sql_cache.cc @@ -268,6 +268,39 @@ are stored in one block. If join_results allocated new block(s) then we need call pack_cache again. +7. Interface +The query cache interfaces with the rest of the server code through 7 +functions: + 1. Query_cache::send_result_to_client + - Called before parsing and used to match a statement with the stored + queries hash. + If a match is found the cached result set is sent through repeated + calls to net_real_write. (note: calling thread doesn't have a regis- + tered result set writer: thd->net.query_cache_query=0) + 2. Query_cache::store_query + - Called just before handle_select() and is used to register a result + set writer to the statement currently being processed + (thd->net.query_cache_query). + 3. query_cache_insert + - Called from net_real_write to append a result set to a cached query + if (and only if) this query has a registered result set writer + (thd->net.query_cache_query). + 4. Query_cache::invalidate + - Called from various places to invalidate query cache based on data- + base, table and myisam file name. During an on going invalidation + the query cache is temporarily disabled. + 5. Query_cache::flush + - Used when a RESET QUERY CACHE is issued. This clears the entire + cache block by block. + 6. Query_cache::resize + - Used to change the available memory used by the query cache. This + will also invalidate the entrie query cache in one free operation. + 7. Query_cache::pack + - Used when a FLUSH QUERY CACHE is issued. This changes the order of + the used memory blocks in physical memory order and move all avail- + able memory to the 'bottom' of the memory. + + TODO list: - Delayed till after-parsing qache answer (for column rights processing) @@ -615,49 +648,55 @@ void query_cache_insert(NET *net, const char *packet, ulong length) DBUG_VOID_RETURN; STRUCT_LOCK(&query_cache.structure_guard_mutex); - - if (unlikely(query_cache.query_cache_size == 0 || - query_cache.flush_in_progress)) + 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) + { + /* + We lost the writer and the currently processed query has been + invalidated; there is nothing left to do. + */ STRUCT_UNLOCK(&query_cache.structure_guard_mutex); DBUG_VOID_RETURN; } - Query_cache_block *query_block = ((Query_cache_block*) - net->query_cache_query); - if (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)); + + /* + On success, STRUCT_UNLOCK is done by append_result_data. Otherwise, we + still need structure_guard_mutex to free the query, and therefore unlock + it later in this function. + */ + if (!query_cache.append_result_data(&result, length, (uchar*) packet, + 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)); - - /* - On success STRUCT_UNLOCK(&query_cache.structure_guard_mutex) will be - done by query_cache.append_result_data if success (if not we need - query_cache.structure_guard_mutex locked to free query) - */ - if (!query_cache.append_result_data(&result, length, (uchar*) packet, - query_block)) - { - DBUG_PRINT("warning", ("Can't append data")); - header->result(result); - DBUG_PRINT("qcache", ("free query 0x%lx", (ulong) 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 - STRUCT_UNLOCK(&query_cache.structure_guard_mutex); - DBUG_VOID_RETURN; - } + DBUG_PRINT("warning", ("Can't append data")); header->result(result); - header->last_pkt_nr= net->pkt_nr; - BLOCK_UNLOCK_WR(query_block); - DBUG_EXECUTE("check_querycache",query_cache.check_integrity(0);); - } - else + DBUG_PRINT("qcache", ("free query 0x%lx", (ulong) 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 STRUCT_UNLOCK(&query_cache.structure_guard_mutex); + DBUG_VOID_RETURN; + } + + header->result(result); + header->last_pkt_nr= net->pkt_nr; + BLOCK_UNLOCK_WR(query_block); + DBUG_EXECUTE("check_querycache",query_cache.check_integrity(0);); + DBUG_VOID_RETURN; } @@ -671,17 +710,21 @@ void query_cache_abort(NET *net) DBUG_VOID_RETURN; STRUCT_LOCK(&query_cache.structure_guard_mutex); - - if (unlikely(query_cache.query_cache_size == 0 || - query_cache.flush_in_progress)) + bool interrupt; + query_cache.wait_while_table_flush_is_in_progress(&interrupt); + if (interrupt) { STRUCT_UNLOCK(&query_cache.structure_guard_mutex); DBUG_VOID_RETURN; } + /* + While we were waiting another thread might have changed the status + of the writer. Make sure the writer still exists before continue. + */ Query_cache_block *query_block= ((Query_cache_block*) net->query_cache_query); - if (query_block) // Test if changed by other thread + if (query_block) { DUMP(&query_cache); BLOCK_LOCK_WR(query_block); @@ -713,13 +756,22 @@ void query_cache_end_of_result(THD *thd) STRUCT_LOCK(&query_cache.structure_guard_mutex); - if (unlikely(query_cache.query_cache_size == 0 || - query_cache.flush_in_progress)) - goto end; + 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) { + /* + The writer is still present; finish last result block by chopping it to + suitable size if needed and setting block type. Since this is the last + block, the writer should be dropped. + */ DUMP(&query_cache); BLOCK_LOCK_WR(query_block); Query_cache_query *header= query_block->query(); @@ -746,8 +798,11 @@ void query_cache_end_of_result(THD *thd) #endif header->found_rows(current_thd->limit_found_rows); header->result()->type= Query_cache_block::RESULT; + + /* Drop the writer. */ header->writer(0); thd->net.query_cache_query= 0; + BLOCK_UNLOCK_WR(query_block); DBUG_EXECUTE("check_querycache",query_cache.check_integrity(1);); @@ -801,9 +856,9 @@ ulong Query_cache::resize(ulong query_cache_size_arg) DBUG_ASSERT(initialized); STRUCT_LOCK(&structure_guard_mutex); - while (flush_in_progress) - pthread_cond_wait(&COND_flush_finished, &structure_guard_mutex); - flush_in_progress= TRUE; + 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); free_cache(); @@ -814,8 +869,8 @@ ulong Query_cache::resize(ulong query_cache_size_arg) DBUG_EXECUTE("check_querycache",check_integrity(0);); STRUCT_LOCK(&structure_guard_mutex); - flush_in_progress= FALSE; - pthread_cond_signal(&COND_flush_finished); + m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS; + pthread_cond_signal(&COND_cache_status_changed); STRUCT_UNLOCK(&structure_guard_mutex); DBUG_RETURN(new_query_cache_size); @@ -910,8 +965,13 @@ def_week_frmt: %lu", ha_release_temporary_latches(thd); STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size == 0 || flush_in_progress) + if (query_cache_size == 0 || is_flushing()) { + /* + A table- or a full flush operation can potentially take a long time to + finish. We choose not to wait for them and skip caching statements + instead. + */ STRUCT_UNLOCK(&structure_guard_mutex); DBUG_VOID_RETURN; } @@ -954,7 +1014,7 @@ def_week_frmt: %lu", Query_cache_block *query_block; query_block= write_block_data(tot_length, (uchar*) thd->query, ALIGN_SIZE(sizeof(Query_cache_query)), - Query_cache_block::QUERY, local_tables, 1); + Query_cache_block::QUERY, local_tables); if (query_block != 0) { DBUG_PRINT("qcache", ("query block 0x%lx allocated, %lu", @@ -1088,13 +1148,21 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length) } STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size == 0 || flush_in_progress) + + if (query_cache_size == 0) + goto err_unlock; + + if (is_flushing()) { - DBUG_PRINT("qcache", ("query cache disabled")); + /* Return; Query cache is temporarily disabled while we flush. */ + DBUG_PRINT("qcache",("query cache disabled")); goto err_unlock; } - /* Check that we haven't forgot to reset the query cache variables */ + /* + Check that we haven't forgot to reset the query cache variables; + make sure there are no attached query cache writer to this thread. + */ DBUG_ASSERT(thd->net.query_cache_query == 0); Query_cache_block *query_block; @@ -1267,7 +1335,7 @@ def_week_frmt: %lu", ("Handler require invalidation queries of %s.%s %lu-%lu", table_list.db, table_list.alias, (ulong) engine_data, (ulong) table->engine_data())); - invalidate_table((uchar *) table->db(), table->key_length()); + invalidate_table(thd, (uchar *) table->db(), table->key_length()); } else thd->lex->safe_to_cache_query= 0; // Don't try to cache this @@ -1330,32 +1398,26 @@ void Query_cache::invalidate(THD *thd, TABLE_LIST *tables_used, my_bool using_transactions) { DBUG_ENTER("Query_cache::invalidate (table list)"); - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) - { - DUMP(this); - using_transactions= using_transactions && - (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); - for (; tables_used; tables_used= tables_used->next_local) - { - DBUG_ASSERT(!using_transactions || tables_used->table!=0); - if (tables_used->derived) - continue; - if (using_transactions && - (tables_used->table->file->table_cache_type() == - HA_CACHE_TBL_TRANSACT)) - /* - Tables_used->table can't be 0 in transaction. - Only 'drop' invalidate not opened table, but 'drop' - force transaction finish. - */ - thd->add_changed_table(tables_used->table); - else - invalidate_table(tables_used); - } + using_transactions= using_transactions && + (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); + for (; tables_used; tables_used= tables_used->next_local) + { + DBUG_ASSERT(!using_transactions || tables_used->table!=0); + if (tables_used->derived) + continue; + if (using_transactions && + (tables_used->table->file->table_cache_type() == + HA_CACHE_TBL_TRANSACT)) + /* + tables_used->table can't be 0 in transaction. + Only 'drop' invalidate not opened table, but 'drop' + force transaction finish. + */ + thd->add_changed_table(tables_used->table); + else + invalidate_table(thd, tables_used); } - STRUCT_UNLOCK(&structure_guard_mutex); DBUG_VOID_RETURN; } @@ -1363,21 +1425,13 @@ void Query_cache::invalidate(THD *thd, TABLE_LIST *tables_used, void Query_cache::invalidate(CHANGED_TABLE_LIST *tables_used) { DBUG_ENTER("Query_cache::invalidate (changed table list)"); - if (tables_used) + THD *thd= current_thd; + for (; tables_used; tables_used= tables_used->next) { - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) - { - DUMP(this); - for (; tables_used; tables_used= tables_used->next) - { - invalidate_table((uchar*) tables_used->key, tables_used->key_length); - DBUG_PRINT("qcache", ("db: %s table: %s", tables_used->key, - tables_used->key+ - strlen(tables_used->key)+1)); - } - } - STRUCT_UNLOCK(&structure_guard_mutex); + invalidate_table(thd, (uchar*) tables_used->key, tables_used->key_length); + DBUG_PRINT("qcache", ("db: %s table: %s", tables_used->key, + tables_used->key+ + strlen(tables_used->key)+1)); } DBUG_VOID_RETURN; } @@ -1396,20 +1450,14 @@ void Query_cache::invalidate(CHANGED_TABLE_LIST *tables_used) void Query_cache::invalidate_locked_for_write(TABLE_LIST *tables_used) { DBUG_ENTER("Query_cache::invalidate_locked_for_write"); - if (tables_used) + for (; tables_used; tables_used= tables_used->next_local) { - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) + if (tables_used->lock_type & (TL_WRITE_LOW_PRIORITY | TL_WRITE) && + tables_used->table) { - DUMP(this); - for (; tables_used; tables_used= tables_used->next_local) - { - if (tables_used->lock_type & (TL_WRITE_LOW_PRIORITY | TL_WRITE) && - tables_used->table) - invalidate_table(tables_used->table); - } + THD *thd= current_thd; + invalidate_table(thd, tables_used->table); } - STRUCT_UNLOCK(&structure_guard_mutex); } DBUG_VOID_RETURN; } @@ -1423,18 +1471,14 @@ void Query_cache::invalidate(THD *thd, TABLE *table, { DBUG_ENTER("Query_cache::invalidate (table)"); - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) - { - using_transactions= using_transactions && - (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); - if (using_transactions && - (table->file->table_cache_type() == HA_CACHE_TBL_TRANSACT)) - thd->add_changed_table(table); - else - invalidate_table(table); - } - STRUCT_UNLOCK(&structure_guard_mutex); + using_transactions= using_transactions && + (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); + if (using_transactions && + (table->file->table_cache_type() == HA_CACHE_TBL_TRANSACT)) + thd->add_changed_table(table); + else + invalidate_table(thd, table); + DBUG_VOID_RETURN; } @@ -1443,31 +1487,77 @@ void Query_cache::invalidate(THD *thd, const char *key, uint32 key_length, my_bool using_transactions) { DBUG_ENTER("Query_cache::invalidate (key)"); - - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) - { - using_transactions= using_transactions && - (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); - if (using_transactions) // used for innodb => has_transactions() is TRUE - thd->add_changed_table(key, key_length); - else - invalidate_table((uchar*)key, key_length); - } - STRUCT_UNLOCK(&structure_guard_mutex); + + using_transactions= using_transactions && + (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)); + if (using_transactions) // used for innodb => has_transactions() is TRUE + thd->add_changed_table(key, key_length); + else + invalidate_table(thd, (uchar*)key, key_length); DBUG_VOID_RETURN; } + +/** + @brief Synchronize the thread with any flushing operations. + + This helper function is called whenever a thread needs to operate on the + query cache structure (example: during invalidation). If a table flush is in + progress this function will wait for it to stop. If a full flush is in + progress, the function will set the interrupt parameter to indicate that the + current operation is redundant and should be interrupted. + + @param[out] interrupt This out-parameter will be set to TRUE if the calling + function is redundant and should be interrupted. + + @return If the interrupt-parameter is TRUE then m_cache_status is set to + NO_FLUSH_IN_PROGRESS. If the interrupt-parameter is FALSE then + m_cache_status is set to FLUSH_IN_PROGRESS. + The structure_guard_mutex will in any case be locked. +*/ + +void Query_cache::wait_while_table_flush_is_in_progress(bool *interrupt) +{ + while (is_flushing()) + { + /* + If there already is a full flush in progress query cache isn't enabled + and additional flushes are redundant; just return instead. + */ + if (m_cache_status == Query_cache::FLUSH_IN_PROGRESS) + { + *interrupt= TRUE; + return; + } + /* + If a table flush is in progress; wait on cache status to change. + */ + if (m_cache_status == Query_cache::TABLE_FLUSH_IN_PROGRESS) + pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex); + } + *interrupt= FALSE; +} + /* Remove all cached queries that uses the given database */ - void Query_cache::invalidate(char *db) { DBUG_ENTER("Query_cache::invalidate (db)"); + STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) + bool interrupt; + wait_while_table_flush_is_in_progress(&interrupt); + if (interrupt) + { + STRUCT_UNLOCK(&structure_guard_mutex); + return; + } + + THD *thd= current_thd; + + if (query_cache_size > 0) { DUMP(this); restart_search: @@ -1479,7 +1569,10 @@ void Query_cache::invalidate(char *db) { next= curr->next; if (strcmp(db, (char*)(curr->table()->db())) == 0) - invalidate_table(curr); + { + Query_cache_block_table *list_root= curr->table(0); + invalidate_query_block_list(thd,list_root); + } /* invalidate_table can freed block on which point 'next' (if table of this block used only in queries which was deleted @@ -1491,6 +1584,11 @@ void Query_cache::invalidate(char *db) if (next->type == Query_cache_block::FREE) goto restart_search; curr= next; + /* + The loop will end if the circular list pointer has reached the + point where it started from, or if the current thread was signaled + to die. + */ } while (curr != tables_blocks); } } @@ -1504,21 +1602,12 @@ void Query_cache::invalidate_by_MyISAM_filename(const char *filename) { DBUG_ENTER("Query_cache::invalidate_by_MyISAM_filename"); - STRUCT_LOCK(&structure_guard_mutex); - if (query_cache_size > 0 && !flush_in_progress) - { - /* Calculate the key outside the lock to make the lock shorter */ - char key[MAX_DBKEY_LENGTH]; - uint32 db_length; - uint key_length= filename_2_table_key(key, filename, &db_length); - Query_cache_block *table_block; - if ((table_block = (Query_cache_block*) hash_search(&tables, - (uchar*) key, - key_length))) - invalidate_table(table_block); - } - STRUCT_UNLOCK(&structure_guard_mutex); - + /* Calculate the key outside the lock to make the lock shorter */ + char key[MAX_DBKEY_LENGTH]; + uint32 db_length; + uint key_length= filename_2_table_key(key, filename, &db_length); + THD *thd= current_thd; + invalidate_table(thd,(uchar *)key, key_length); DBUG_VOID_RETURN; } @@ -1540,16 +1629,43 @@ void Query_cache::flush() DBUG_VOID_RETURN; } - /* Join result in cache in 1 block (if result length > join_limit) */ + +/** + @brief Rearrange the memory blocks and join result in cache in 1 block (if + result length > join_limit) + + @param[in] join_limit If the minimum length of a result block to be joined. + @param[in] iteration_limit The maximum number of packing and joining + sequences. + +*/ void Query_cache::pack(ulong join_limit, uint iteration_limit) { DBUG_ENTER("Query_cache::pack"); + + bool interrupt; + STRUCT_LOCK(&structure_guard_mutex); + wait_while_table_flush_is_in_progress(&interrupt); + if (interrupt) + { + STRUCT_UNLOCK(&structure_guard_mutex); + DBUG_VOID_RETURN; + } + + if (query_cache_size == 0) + { + STRUCT_UNLOCK(&structure_guard_mutex); + DBUG_VOID_RETURN; + } + uint i = 0; do { pack_cache(); } while ((++i < iteration_limit) && join_results(join_limit)); + + STRUCT_UNLOCK(&structure_guard_mutex); DBUG_VOID_RETURN; } @@ -1568,7 +1684,7 @@ void Query_cache::destroy() free_cache(); STRUCT_UNLOCK(&structure_guard_mutex); - pthread_cond_destroy(&COND_flush_finished); + pthread_cond_destroy(&COND_cache_status_changed); pthread_mutex_destroy(&structure_guard_mutex); initialized = 0; } @@ -1584,8 +1700,8 @@ void Query_cache::init() { DBUG_ENTER("Query_cache::init"); pthread_mutex_init(&structure_guard_mutex,MY_MUTEX_INIT_FAST); - pthread_cond_init(&COND_flush_finished, NULL); - flush_in_progress= FALSE; + pthread_cond_init(&COND_cache_status_changed, NULL); + m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS; initialized = 1; DBUG_VOID_RETURN; } @@ -1787,9 +1903,10 @@ void Query_cache::make_disabled() /** @class Query_cache @brief Free all resources allocated by the cache. - @details This function frees all resources allocated by the cache. You - have to call init_cache() before using the cache again. This function requires - the structure_guard_mutex to be locked. + + This function frees all resources allocated by the cache. You + have to call init_cache() before using the cache again. This function + requires the structure_guard_mutex to be locked. */ void Query_cache::free_cache() @@ -1808,24 +1925,17 @@ void Query_cache::free_cache() *****************************************************************************/ -/* - flush_cache() - flush the cache. +/** + @brief Flush the cache. - SYNOPSIS - flush_cache() - - DESCRIPTION - This function will flush cache contents. It assumes we have - 'structure_guard_mutex' locked. The function sets the - flush_in_progress flag and releases the lock, so other threads may - proceed skipping the cache as if it is disabled. Concurrent - flushes are performed in turn. - - After flush_cache() call, the cache is flushed, all the freed - memory is accumulated in bin[0], and the 'structure_guard_mutex' - is locked. However, since we could release the mutex during - execution, the rest of the cache state could have been changed, - and should not be relied on. + This function will flush cache contents. It assumes we have + 'structure_guard_mutex' locked. The function sets the m_cache_status flag and + releases the lock, so other threads may proceed skipping the cache as if it + is disabled. Concurrent flushes are performed in turn. + After flush_cache() call, the cache is flushed, all the freed memory is + accumulated in bin[0], and the 'structure_guard_mutex' is locked. However, + since we could release the mutex during execution, the rest of the cache + state could have been changed, and should not be relied on. */ void Query_cache::flush_cache() @@ -1837,15 +1947,15 @@ void Query_cache::flush_cache() Query_cache::free_cache()) depends on the fact that after the flush the cache is empty. */ - while (flush_in_progress) - pthread_cond_wait(&COND_flush_finished, &structure_guard_mutex); + while (is_flushing()) + pthread_cond_wait(&COND_cache_status_changed, &structure_guard_mutex); /* - Setting 'flush_in_progress' will prevent other threads from using + 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. */ - flush_in_progress= TRUE; + m_cache_status= Query_cache::FLUSH_IN_PROGRESS; STRUCT_UNLOCK(&structure_guard_mutex); my_hash_reset(&queries); @@ -1856,8 +1966,8 @@ void Query_cache::flush_cache() } STRUCT_LOCK(&structure_guard_mutex); - flush_in_progress= FALSE; - pthread_cond_signal(&COND_flush_finished); + m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS; + pthread_cond_signal(&COND_cache_status_changed); } /* @@ -1875,7 +1985,7 @@ my_bool Query_cache::free_old_query() sequence is breached. Also we don't need remove locked queries at this point. */ - Query_cache_block *query_block = 0; + Query_cache_block *query_block= 0; if (queries_blocks != 0) { Query_cache_block *block = queries_blocks; @@ -2013,8 +2123,7 @@ Query_cache_block * Query_cache::write_block_data(ulong data_len, uchar* data, ulong header_len, Query_cache_block::block_type type, - TABLE_COUNTER_TYPE ntab, - my_bool under_guard) + TABLE_COUNTER_TYPE ntab) { ulong all_headers_len = (ALIGN_SIZE(sizeof(Query_cache_block)) + ALIGN_SIZE(ntab*sizeof(Query_cache_block_table)) + @@ -2024,9 +2133,8 @@ Query_cache::write_block_data(ulong data_len, uchar* data, DBUG_ENTER("Query_cache::write_block_data"); DBUG_PRINT("qcache", ("data: %ld, header: %ld, all header: %ld", data_len, header_len, all_headers_len)); - Query_cache_block *block = allocate_block(max(align_len, - min_allocation_unit), - 1, 0, under_guard); + Query_cache_block *block= allocate_block(max(align_len, + min_allocation_unit),1, 0); if (block != 0) { block->type = type; @@ -2243,8 +2351,7 @@ my_bool Query_cache::allocate_data_chain(Query_cache_block **result_block, if (!(new_block= allocate_block(max(min_size, align_len), min_result_data_size == 0, - all_headers_len + min_result_data_size, - 1))) + all_headers_len + min_result_data_size))) { DBUG_PRINT("warning", ("Can't allocate block for results")); DBUG_RETURN(FALSE); @@ -2286,51 +2393,94 @@ my_bool Query_cache::allocate_data_chain(Query_cache_block **result_block, Invalidate the first table in the table_list */ -void Query_cache::invalidate_table(TABLE_LIST *table_list) +void Query_cache::invalidate_table(THD *thd, TABLE_LIST *table_list) { if (table_list->table != 0) - invalidate_table(table_list->table); // Table is open + invalidate_table(thd, table_list->table); // Table is open else { char key[MAX_DBKEY_LENGTH]; uint key_length; - Query_cache_block *table_block; + key_length=(uint) (strmov(strmov(key,table_list->db)+1, table_list->table_name) -key)+ 1; // We don't store temporary tables => no key_length+=4 ... - if ((table_block = (Query_cache_block*) - hash_search(&tables,(uchar*) key,key_length))) - invalidate_table(table_block); + invalidate_table(thd, (uchar *)key, key_length); } } -void Query_cache::invalidate_table(TABLE *table) +void Query_cache::invalidate_table(THD *thd, TABLE *table) { - invalidate_table((uchar*) table->s->table_cache_key.str, + invalidate_table(thd, (uchar*) table->s->table_cache_key.str, table->s->table_cache_key.length); } -void Query_cache::invalidate_table(uchar * key, uint32 key_length) +void Query_cache::invalidate_table(THD *thd, uchar * key, uint32 key_length) { - Query_cache_block *table_block; - if ((table_block = ((Query_cache_block*) - hash_search(&tables, key, key_length)))) - invalidate_table(table_block); + bool interrupt; + STRUCT_LOCK(&structure_guard_mutex); + wait_while_table_flush_is_in_progress(&interrupt); + if (interrupt) + { + STRUCT_UNLOCK(&structure_guard_mutex); + return; + } + + /* + Setting 'TABLE_FLUSH_IN_PROGRESS' will temporarily disable the cache + so that structural changes to cache won't block the entire server. + However, threads requesting to change the query cache will still have + to wait for the flush to finish. + */ + m_cache_status= Query_cache::TABLE_FLUSH_IN_PROGRESS; + STRUCT_UNLOCK(&structure_guard_mutex); + + Query_cache_block *table_block= + (Query_cache_block*)hash_search(&tables, key, key_length); + if (query_cache_size > 0 && table_block) + { + Query_cache_block_table *list_root= table_block->table(0); + invalidate_query_block_list(thd, list_root); + } + + STRUCT_LOCK(&structure_guard_mutex); + m_cache_status= Query_cache::NO_FLUSH_IN_PROGRESS; + + /* + net_real_write might be waiting on a change on the m_cache_status + variable. + */ + pthread_cond_signal(&COND_cache_status_changed); + STRUCT_UNLOCK(&structure_guard_mutex); } -void Query_cache::invalidate_table(Query_cache_block *table_block) + +/** + @brief Invalidate a linked list of query cache blocks. + + Each block tries to aquire a block level lock before + free_query is a called. This function will in turn affect + related table- and result-blocks. + + @param[in,out] thd Thread context. + @param[in,out] list_root A pointer to a circular list of query blocks. + +*/ + +void +Query_cache::invalidate_query_block_list(THD *thd, + Query_cache_block_table *list_root) { - Query_cache_block_table *list_root = table_block->table(0); while (list_root->next != list_root) { - Query_cache_block *query_block = list_root->next->block(); + Query_cache_block *query_block= list_root->next->block(); BLOCK_LOCK_WR(query_block); free_query(query_block); + DBUG_EXECUTE_IF("debug_cache_locks", sleep(10);); } } - /* Register given table list begining with given position in tables table of block @@ -2463,7 +2613,6 @@ my_bool Query_cache::register_all_tables(Query_cache_block *block, if (n) { - DBUG_PRINT("qcache", ("failed at table %d", (int) n)); /* Unlink the tables we allocated above */ for (Query_cache_block_table *tmp = block->table(0) ; tmp != block_table; @@ -2473,9 +2622,13 @@ my_bool Query_cache::register_all_tables(Query_cache_block *block, return (n); } -/* - Insert used tablename in cache - Returns 0 on error + +/** + @brief Insert used table name into the cache. + + @return Error status + @retval FALSE On error + @retval TRUE On success */ my_bool @@ -2489,9 +2642,10 @@ Query_cache::insert_table(uint key_len, char *key, DBUG_PRINT("qcache", ("insert table node 0x%lx, len %d", (ulong)node, key_len)); - Query_cache_block *table_block = ((Query_cache_block *) - hash_search(&tables, (uchar*) key, - key_len)); + THD *thd= current_thd; + + Query_cache_block *table_block= + (Query_cache_block *)hash_search(&tables, (uchar*) key, key_len); if (table_block && table_block->table()->engine_data() != engine_data) @@ -2506,7 +2660,11 @@ Query_cache::insert_table(uint key_len, char *key, as far as we delete all queries with this table, table block will be deleted, too */ - invalidate_table(table_block); + { + Query_cache_block_table *list_root= table_block->table(0); + invalidate_query_block_list(thd, list_root); + } + table_block= 0; } @@ -2514,21 +2672,29 @@ Query_cache::insert_table(uint key_len, char *key, { DBUG_PRINT("qcache", ("new table block from 0x%lx (%u)", (ulong) key, (int) key_len)); - table_block = write_block_data(key_len, (uchar*) key, - ALIGN_SIZE(sizeof(Query_cache_table)), - Query_cache_block::TABLE, - 1, 1); + table_block= write_block_data(key_len, (uchar*) key, + ALIGN_SIZE(sizeof(Query_cache_table)), + Query_cache_block::TABLE, 1); if (table_block == 0) { DBUG_PRINT("qcache", ("Can't write table name to cache")); DBUG_RETURN(0); } - Query_cache_table *header = table_block->table(); + Query_cache_table *header= table_block->table(); double_linked_list_simple_include(table_block, - &tables_blocks); - Query_cache_block_table *list_root = table_block->table(0); - list_root->n = 0; - list_root->next = list_root->prev = list_root; + &tables_blocks); + /* + First node in the Query_cache_block_table-chain is the table-type + block. This block will only have one Query_cache_block_table (n=0). + */ + Query_cache_block_table *list_root= table_block->table(0); + list_root->n= 0; + + /* + The node list is circular in nature. + */ + list_root->next= list_root->prev= list_root; + if (my_hash_insert(&tables, (const uchar *) table_block)) { DBUG_PRINT("qcache", ("Can't insert table to hash")); @@ -2536,20 +2702,37 @@ Query_cache::insert_table(uint key_len, char *key, free_memory_block(table_block); DBUG_RETURN(0); } - char *db = header->db(); + char *db= header->db(); header->table(db + db_length + 1); header->key_length(key_len); header->type(cache_type); header->callback(callback); header->engine_data(engine_data); + + /* + We insert this table without the assumption that it isn't refrenenced by + any queries. + */ + header->m_cached_query_count= 0; } - Query_cache_block_table *list_root = table_block->table(0); - node->next = list_root->next; - list_root->next = node; - node->next->prev = node; - node->prev = list_root; - node->parent = table_block->table(); + /* + Table is now in the cache; link the table_block-node associated + with the currently processed query into the chain of queries depending + on the cached table. + */ + Query_cache_block_table *list_root= table_block->table(0); + node->next= list_root->next; + list_root->next= node; + node->next->prev= node; + node->prev= list_root; + node->parent= table_block->table(); + /* + Increase the counter to keep track on how long this chain + of queries is. + */ + Query_cache_table *table_block_data= table_block->table(); + table_block_data->m_cached_query_count++; DBUG_RETURN(1); } @@ -2557,15 +2740,27 @@ Query_cache::insert_table(uint key_len, char *key, void Query_cache::unlink_table(Query_cache_block_table *node) { DBUG_ENTER("Query_cache::unlink_table"); - node->prev->next = node->next; - node->next->prev = node->prev; - Query_cache_block_table *neighbour = node->next; + node->prev->next= node->next; + node->next->prev= node->prev; + Query_cache_block_table *neighbour= node->next; + Query_cache_table *table_block_data= node->parent; + table_block_data->m_cached_query_count--; + + DBUG_ASSERT(table_block_data->m_cached_query_count >= 0); + if (neighbour->next == neighbour) { - // list is empty (neighbor is root of list) - Query_cache_block *table_block = neighbour->block(); + DBUG_ASSERT(table_block_data->m_cached_query_count == 0); + /* + If neighbor is root of list, the list is empty. + The root of the list is always a table-type block + which contain exactly one Query_cache_block_table + node object, thus we can use the block() method + to calculate the Query_cache_block address. + */ + Query_cache_block *table_block= neighbour->block(); double_linked_list_exclude(table_block, - &tables_blocks); + &tables_blocks); hash_delete(&tables,(uchar *) table_block); free_memory_block(table_block); } @@ -2577,12 +2772,11 @@ void Query_cache::unlink_table(Query_cache_block_table *node) *****************************************************************************/ Query_cache_block * -Query_cache::allocate_block(ulong len, my_bool not_less, ulong min, - my_bool under_guard) +Query_cache::allocate_block(ulong len, my_bool not_less, ulong min) { DBUG_ENTER("Query_cache::allocate_block"); - DBUG_PRINT("qcache", ("len %lu, not less %d, min %lu, uder_guard %d", - len, not_less,min,under_guard)); + DBUG_PRINT("qcache", ("len %lu, not less %d, min %lu", + len, not_less,min)); if (len >= min(query_cache_size, query_cache_limit)) { @@ -2591,17 +2785,6 @@ Query_cache::allocate_block(ulong len, my_bool not_less, ulong min, DBUG_RETURN(0); // in any case we don't have such piece of memory } - if (!under_guard) - { - STRUCT_LOCK(&structure_guard_mutex); - - if (unlikely(query_cache.query_cache_size == 0 || flush_in_progress)) - { - STRUCT_UNLOCK(&structure_guard_mutex); - DBUG_RETURN(0); - } - } - /* Free old queries until we have enough memory to store this block */ Query_cache_block *block; do @@ -2616,8 +2799,6 @@ Query_cache::allocate_block(ulong len, my_bool not_less, ulong min, split_block(block,ALIGN_SIZE(len)); } - if (!under_guard) - STRUCT_UNLOCK(&structure_guard_mutex); DBUG_RETURN(block); } @@ -2852,9 +3033,7 @@ uint Query_cache::find_bin(ulong size) } uint bin = steps[left].idx - (uint)((size - steps[left].size)/steps[left].increment); -#ifndef DBUG_OFF - bins_dump(); -#endif + DBUG_PRINT("qcache", ("bin %u step %u, size %lu step size %lu", bin, left, size, steps[left].size)); DBUG_RETURN(bin); @@ -3140,18 +3319,17 @@ my_bool Query_cache::ask_handler_allowance(THD *thd, Packing *****************************************************************************/ + +/** + @brief Rearrange all memory blocks so that free memory joins at the + 'bottom' of the allocated memory block containing all cache data. + @see Query_cache::pack(ulong join_limit, uint iteration_limit) +*/ + void Query_cache::pack_cache() { DBUG_ENTER("Query_cache::pack_cache"); - STRUCT_LOCK(&structure_guard_mutex); - - if (unlikely(query_cache_size == 0 || flush_in_progress)) - { - STRUCT_UNLOCK(&structure_guard_mutex); - DBUG_VOID_RETURN; - } - DBUG_EXECUTE("check_querycache",query_cache.check_integrity(1);); uchar *border = 0; @@ -3185,7 +3363,6 @@ void Query_cache::pack_cache() } DBUG_EXECUTE("check_querycache",query_cache.check_integrity(1);); - STRUCT_UNLOCK(&structure_guard_mutex); DBUG_VOID_RETURN; } @@ -3460,8 +3637,7 @@ my_bool Query_cache::join_results(ulong join_limit) my_bool has_moving = 0; DBUG_ENTER("Query_cache::join_results"); - STRUCT_LOCK(&structure_guard_mutex); - if (queries_blocks != 0 && !flush_in_progress) + if (queries_blocks != 0) { DBUG_ASSERT(query_cache_size > 0); Query_cache_block *block = queries_blocks; @@ -3524,7 +3700,6 @@ my_bool Query_cache::join_results(ulong join_limit) block = block->next; } while ( block != queries_blocks ); } - STRUCT_UNLOCK(&structure_guard_mutex); DBUG_RETURN(has_moving); } @@ -3760,6 +3935,14 @@ void Query_cache::tables_dump() } +/** + @brief Checks integrity of the various linked lists + + @return Error status code + @retval FALSE Query cache is operational. + @retval TRUE Query cache is broken. +*/ + my_bool Query_cache::check_integrity(bool locked) { my_bool result = 0; @@ -3769,14 +3952,8 @@ my_bool Query_cache::check_integrity(bool locked) if (!locked) STRUCT_LOCK(&structure_guard_mutex); - if (unlikely(query_cache_size == 0 || flush_in_progress)) - { - if (!locked) - STRUCT_UNLOCK(&query_cache.structure_guard_mutex); - - DBUG_PRINT("qcache", ("Query Cache not initialized")); - DBUG_RETURN(0); - } + while (is_flushing()) + pthread_cond_wait(&COND_cache_status_changed,&structure_guard_mutex); if (hash_check(&queries)) { diff --git a/sql/sql_cache.h b/sql/sql_cache.h index 3c5d784ce94..cfc52e5d33a 100644 --- a/sql/sql_cache.h +++ b/sql/sql_cache.h @@ -65,17 +65,44 @@ struct Query_cache_query; struct Query_cache_result; class Query_cache; +/** + @brief This class represents a node in the linked chain of queries + belonging to one table. + @note The root of this linked list is not a query-type block, but the table- + type block which all queries has in common. +*/ struct Query_cache_block_table { Query_cache_block_table() {} /* Remove gcc warning */ - TABLE_COUNTER_TYPE n; // numbr in table (from 0) + + /** + This node holds a position in a static table list belonging + to the associated query (base 0). + */ + TABLE_COUNTER_TYPE n; + + /** + Pointers to the next and previous node, linking all queries with + a common table. + */ Query_cache_block_table *next, *prev; + + /** + A pointer to the table-type block which all + linked queries has in common. + */ Query_cache_table *parent; + + /** + A method to calculate the address of the query cache block + owning this node. The purpose of this calculation is to + make it easier to move the query cache block without having + to modify all the pointer addresses. + */ inline Query_cache_block *block(); }; - struct Query_cache_block { Query_cache_block() {} /* Remove gcc warning */ @@ -151,6 +178,11 @@ struct Query_cache_table /* data need by some engines */ ulonglong engine_data_buff; + /** + The number of queries depending of this table. + */ + int32 m_cached_query_count; + inline char *db() { return (char *) data(); } inline char *table() { return tbl; } inline void table(char *table_arg) { tbl= table_arg; } @@ -237,9 +269,14 @@ public: ulong free_memory, queries_in_cache, hits, inserts, refused, free_memory_blocks, total_blocks, lowmem_prunes; + private: - pthread_cond_t COND_flush_finished; - bool flush_in_progress; + 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; void free_query_internal(Query_cache_block *point); @@ -253,7 +290,7 @@ protected: 2. query block (for operation inside query (query block/results)) Thread doing cache flush releases the mutex once it sets - flush_in_progress flag, so other threads may bypass the cache as + m_cache_status flag, so other threads may bypass the cache as if it is disabled, not waiting for reset to finish. The exception is other threads that were going to do cache flush---they'll wait till the end of a flush operation. @@ -270,6 +307,7 @@ protected: /* options */ ulong min_allocation_unit, min_result_data_size; uint def_query_hash_size, def_table_hash_size; + uint mem_bin_num, mem_bin_steps; // See at init_cache & find_bin my_bool initialized; @@ -295,10 +333,13 @@ protected: ulong data_len, Query_cache_block *query_block, my_bool first_block); - void invalidate_table(TABLE_LIST *table); - void invalidate_table(TABLE *table); - void invalidate_table(uchar *key, uint32 key_length); - void invalidate_table(Query_cache_block *table_block); + void invalidate_table(THD *thd, TABLE_LIST *table); + void invalidate_table(THD *thd, TABLE *table); + void invalidate_table(THD *thd, uchar *key, uint32 key_length); + void invalidate_table(THD *thd, Query_cache_block *table_block); + void invalidate_query_block_list(THD *thd, + Query_cache_block_table *list_root); + TABLE_COUNTER_TYPE register_tables_from_list(TABLE_LIST *tables_used, TABLE_COUNTER_TYPE counter, @@ -337,6 +378,8 @@ 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 @@ -347,8 +390,7 @@ protected: Query_cache_block *write_block_data(ulong data_len, uchar* data, ulong header_len, Query_cache_block::block_type type, - TABLE_COUNTER_TYPE ntab = 0, - my_bool under_guard=0); + TABLE_COUNTER_TYPE ntab = 0); my_bool append_result_data(Query_cache_block **result, ulong data_len, uchar* data, Query_cache_block *parent); @@ -360,8 +402,7 @@ protected: inline ulong get_min_first_result_data_size(); inline ulong get_min_append_result_data_size(); Query_cache_block *allocate_block(ulong len, my_bool not_less, - ulong min, - my_bool under_guard=0); + ulong min); /* If query is cacheable return number tables in query (query without tables not cached) @@ -424,6 +465,11 @@ 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 diff --git a/sql/sql_db.cc b/sql/sql_db.cc index 82938184245..49a9620d555 100644 --- a/sql/sql_db.cc +++ b/sql/sql_db.cc @@ -1084,7 +1084,7 @@ static long mysql_rm_known_files(THD *thd, MY_DIR *dirp, const char *db, } } if (thd->killed || - (tot_list && mysql_rm_table_part2_with_lock(thd, tot_list, 1, 0, 1))) + (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1, 1))) goto err; /* Remove RAID directories */ diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 8e55610df36..485648d1fb1 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2445,7 +2445,7 @@ end_with_restore_list: check_grant(thd, INSERT_ACL | CREATE_ACL, &new_list, 0, 1, 0))) goto error; } - query_cache_invalidate3(thd, first_table, 0); + if (end_active_trans(thd) || mysql_rename_tables(thd, first_table, 0)) goto error; break; diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index f34ec83b29c..b87c74dc179 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -144,10 +144,14 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent) } } - VOID(pthread_mutex_lock(&LOCK_open)); - if (lock_table_names(thd, table_list)) + pthread_mutex_lock(&LOCK_open); + if (lock_table_names_exclusively(thd, table_list)) + { + pthread_mutex_unlock(&LOCK_open); goto err; - + } + pthread_mutex_unlock(&LOCK_open); + error=0; if ((ren_table=rename_tables(thd,table_list,0))) { @@ -183,10 +187,14 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent) send_ok(thd); } + if (!error) + query_cache_invalidate3(thd, table_list, 0); + + pthread_mutex_lock(&LOCK_open); unlock_table_names(thd, table_list, (TABLE_LIST*) 0); + pthread_mutex_unlock(&LOCK_open); err: - pthread_mutex_unlock(&LOCK_open); /* enable logging back if needed */ if (disable_logs) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index bb0f7649272..ba89578ec31 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1411,14 +1411,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, LOCK_open during wait_if_global_read_lock(), other threads could not close their tables. This would make a pretty deadlock. */ - thd->mysys_var->current_mutex= &LOCK_open; - thd->mysys_var->current_cond= &COND_refresh; - VOID(pthread_mutex_lock(&LOCK_open)); - - error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0); - - pthread_mutex_unlock(&LOCK_open); - + error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0, 1); pthread_mutex_lock(&thd->mysys_var->mutex); thd->mysys_var->current_mutex= 0; thd->mysys_var->current_cond= 0; @@ -1433,49 +1426,6 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, DBUG_RETURN(FALSE); } - -/* - delete (drop) tables. - - SYNOPSIS - mysql_rm_table_part2_with_lock() - thd Thread handle - tables List of tables to delete - if_exists If 1, don't give error if one table doesn't exists - dont_log_query Don't write query to log files. This will also not - generate warnings if the handler files doesn't exists - - NOTES - Works like documented in mysql_rm_table(), but don't check - global_read_lock and don't send_ok packet to server. - - RETURN - 0 ok - 1 error -*/ - -int mysql_rm_table_part2_with_lock(THD *thd, - TABLE_LIST *tables, bool if_exists, - bool drop_temporary, bool dont_log_query) -{ - int error; - thd->mysys_var->current_mutex= &LOCK_open; - thd->mysys_var->current_cond= &COND_refresh; - VOID(pthread_mutex_lock(&LOCK_open)); - - error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 1, - dont_log_query); - - pthread_mutex_unlock(&LOCK_open); - - pthread_mutex_lock(&thd->mysys_var->mutex); - thd->mysys_var->current_mutex= 0; - thd->mysys_var->current_cond= 0; - pthread_mutex_unlock(&thd->mysys_var->mutex); - return error; -} - - /* Execute the drop of a normal or temporary table @@ -1508,7 +1458,7 @@ int mysql_rm_table_part2_with_lock(THD *thd, int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, bool drop_temporary, bool drop_view, - bool dont_log_query) + bool dont_log_query, bool need_lock_open) { TABLE_LIST *table; char path[FN_REFLEN], *alias; @@ -1520,9 +1470,11 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, String built_query; DBUG_ENTER("mysql_rm_table_part2"); + if (need_lock_open) + pthread_mutex_lock(&LOCK_open); + LINT_INIT(alias); LINT_INIT(path_length); - safe_mutex_assert_owner(&LOCK_open); if (thd->current_stmt_binlog_row_based && !dont_log_query) { @@ -1555,8 +1507,15 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, } } - if (!drop_temporary && lock_table_names(thd, tables)) + if (!drop_temporary && lock_table_names_exclusively(thd, tables)) + { + if (need_lock_open) + pthread_mutex_unlock(&LOCK_open); DBUG_RETURN(1); + } + + if (need_lock_open) + pthread_mutex_unlock(&LOCK_open); /* Don't give warnings for not found errors, as we already generate notes */ thd->no_warnings_for_error= 1; @@ -1567,7 +1526,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, handlerton *table_type; enum legacy_db_type frm_db_type; - mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, TRUE); + mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, !need_lock_open); if (!close_temporary_table(thd, table)) { tmp_table_deleted=1; @@ -1604,6 +1563,8 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, { TABLE *locked_table; abort_locked_tables(thd, db, table->table_name); + if (need_lock_open) + pthread_mutex_lock(&LOCK_open); remove_table_from_cache(thd, db, table->table_name, RTFC_WAIT_OTHER_THREAD_FLAG | RTFC_CHECK_KILLED_FLAG); @@ -1614,6 +1575,9 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, if ((locked_table= drop_locked_tables(thd, db, table->table_name))) table->table= locked_table; + if (need_lock_open) + pthread_mutex_unlock(&LOCK_open); + if (thd->killed) { thd->no_warnings_for_error= 0; @@ -1739,9 +1703,11 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, */ } } - - if (!drop_temporary) - unlock_table_names(thd, tables, (TABLE_LIST*) 0); + if (need_lock_open) + pthread_mutex_lock(&LOCK_open); + unlock_table_names(thd, tables, (TABLE_LIST*) 0); + if (need_lock_open) + pthread_mutex_unlock(&LOCK_open); thd->no_warnings_for_error= 0; DBUG_RETURN(error); } diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index e15003ab243..16159150e66 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -1268,8 +1268,6 @@ bool Table_triggers_list::drop_all_triggers(THD *thd, char *db, char *name) bzero(&table, sizeof(table)); init_alloc_root(&table.mem_root, 8192, 0); - safe_mutex_assert_owner(&LOCK_open); - if (Table_triggers_list::check_n_load(thd, db, name, &table, 1)) { result= 1; @@ -1431,26 +1429,24 @@ Table_triggers_list::change_table_name_in_trignames(const char *db_name, } -/* - Update .TRG and .TRN files after renaming triggers' subject table. +/** + @brief Update .TRG and .TRN files after renaming triggers' subject table. - SYNOPSIS - change_table_name() - thd Thread context - db Old database of subject table - old_table Old name of subject table - new_db New database for subject table - new_table New name of subject table + @param[in,out] thd Thread context + @param[in] db Old database of subject table + @param[in] old_table Old name of subject table + @param[in] new_db New database for subject table + @param[in] new_table New name of subject table - NOTE + @note This method tries to leave trigger related files in consistent state, i.e. it either will complete successfully, or will fail leaving files in their initial state. Also this method assumes that subject table is not renamed to itself. + This method needs to be called under an exclusive table name lock. - RETURN VALUE - FALSE Success - TRUE Error + @retval FALSE Success + @retval TRUE Error */ bool Table_triggers_list::change_table_name(THD *thd, const char *db, @@ -1466,7 +1462,19 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db, bzero(&table, sizeof(table)); init_alloc_root(&table.mem_root, 8192, 0); - safe_mutex_assert_owner(&LOCK_open); + uchar key[MAX_DBKEY_LENGTH]; + uint key_length= (uint) (strmov(strmov((char*)&key[0], db)+1, + old_table)-(char*)&key[0])+1; + + /* + This method interfaces the mysql server code protected by + either LOCK_open mutex or with an exclusive table name lock. + In the future, only an exclusive table name lock will be enough. + */ +#ifndef DBUG_OFF + if (!is_table_name_exclusively_locked_by_this_thread(thd, key, key_length)) + safe_mutex_assert_owner(&LOCK_open); +#endif DBUG_ASSERT(my_strcasecmp(table_alias_charset, db, new_db) || my_strcasecmp(table_alias_charset, old_table, new_table)); From 6ba23b0ac9a4548de36386b4f892af9c7d471e97 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 12 Jul 2007 12:49:39 +0400 Subject: [PATCH 2/7] Fix for 5.1 for BUG#10491: Server returns data as charset binary SHOW CREATE TABLE or SELECT FROM I_S. This is the last patch for this bug, which depends on the big CS patch and was pending. The problem was that SHOW CREATE statements returned original queries in the binary character set. That could cause the query to be unreadable. The fix is to use original character_set_client when sending the original query to the client. In order to preserve the query in mysqldump, 'binary' character set results should be set when issuing SHOW CREATE statement. If either source or destination character set is 'binary' , no conversion is performed. The idea is that since the source character set is no longer 'binary', we fix the destination character set to still produce valid dumps. client/mysqldump.c: Switch character_set_results of mysqldump-connection before calling SHOW CREATE statements for the objects. mysql-test/r/show_check.result: Result file. mysql-test/t/show_check.test: Add test case for the part of BUG#10491. sql/events.cc: Send original query in the original character set. sql/sp_head.cc: Send original query in the original character set. sql/sql_show.cc: Send original query in the original character set. --- client/mysqldump.c | 43 +++++++++++++++++++++++ mysql-test/r/show_check.result | 39 +++++++++++++++++++++ mysql-test/t/show_check.test | 64 ++++++++++++++++++++++++++++++++++ sql/events.cc | 3 +- sql/sp_head.cc | 3 +- sql/sql_show.cc | 12 +++++-- 6 files changed, 160 insertions(+), 4 deletions(-) diff --git a/client/mysqldump.c b/client/mysqldump.c index 812a158048e..11be212ec3b 100644 --- a/client/mysqldump.c +++ b/client/mysqldump.c @@ -1201,6 +1201,20 @@ static void restore_time_zone(FILE *sql_file, (const char *) delimiter); } + +static int switch_character_set_results(MYSQL *mysql, const char *cs_name) +{ + char query_buffer[QUERY_LENGTH]; + size_t query_length; + + query_length= my_snprintf(query_buffer, + sizeof (query_buffer), + "SET SESSION character_set_results = '%s'", + (const char *) cs_name); + + return mysql_real_query(mysql, query_buffer, query_length); +} + /* Open a new .sql file to dump the table or view into @@ -1706,6 +1720,9 @@ static uint dump_events_for_db(char *db) if (fetch_db_collation(db_name_buff, db_cl_name, sizeof (db_cl_name))) DBUG_RETURN(1); + if (switch_character_set_results(mysql, "binary")) + DBUG_RETURN(1); + while ((event_list_row= mysql_fetch_row(event_list_res)) != NULL) { event_name= quote_name(event_list_row[1], name_buff, 0); @@ -1774,6 +1791,9 @@ static uint dump_events_for_db(char *db) } /* end of list of events */ fprintf(sql_file, "DELIMITER ;\n"); fprintf(sql_file, "/*!50106 SET TIME_ZONE= @save_time_zone */ ;\n"); + + if (switch_character_set_results(mysql, default_charset)) + DBUG_RETURN(1); } mysql_free_result(event_list_res); @@ -1853,6 +1873,9 @@ static uint dump_routines_for_db(char *db) if (fetch_db_collation(db_name_buff, db_cl_name, sizeof (db_cl_name))) DBUG_RETURN(1); + if (switch_character_set_results(mysql, "binary")) + DBUG_RETURN(1); + /* 0, retrieve and dump functions, 1, procedures */ for (i= 0; i <= 1; i++) { @@ -1990,6 +2013,9 @@ static uint dump_routines_for_db(char *db) mysql_free_result(routine_list_res); } /* end of for i (0 .. 1) */ + if (switch_character_set_results(mysql, default_charset)) + DBUG_RETURN(1); + if (lock_tables) VOID(mysql_query_with_error_report(mysql, 0, "UNLOCK TABLES")); DBUG_RETURN(0); @@ -2542,6 +2568,9 @@ static void dump_triggers_for_table(char *table, char *db_name) if (fetch_db_collation(db_name, db_cl_name, sizeof (db_cl_name))) DBUG_VOID_RETURN; + if (switch_character_set_results(mysql, "binary")) + DBUG_VOID_RETURN; + /* Dump triggers. */ while ((row= mysql_fetch_row(result))) @@ -2637,6 +2666,9 @@ static void dump_triggers_for_table(char *table, char *db_name) mysql_free_result(result); + if (switch_character_set_results(mysql, default_charset)) + DBUG_VOID_RETURN; + /* make sure to set back opt_compatible mode to original value @@ -4390,14 +4422,22 @@ static my_bool get_view_structure(char *table, char* db) result_table= quote_name(table, table_buff, 1); opt_quoted_table= quote_name(table, table_buff2, 0); + if (switch_character_set_results(mysql, "binary")) + DBUG_RETURN(1); + my_snprintf(query, sizeof(query), "SHOW CREATE TABLE %s", result_table); + if (mysql_query_with_error_report(mysql, &table_res, query)) + { + switch_character_set_results(mysql, default_charset); DBUG_RETURN(0); + } /* Check if this is a view */ field= mysql_fetch_field_direct(table_res, 0); if (strcmp(field->name, "View") != 0) { + switch_character_set_results(mysql, default_charset); verbose_msg("-- It's base table, skipped\n"); DBUG_RETURN(0); } @@ -4540,6 +4580,9 @@ static my_bool get_view_structure(char *table, char* db) dynstr_free(&ds_view); } + if (switch_character_set_results(mysql, default_charset)) + DBUG_RETURN(1); + /* If a separate .sql file was opened, close it now */ if (sql_file != md_result_file) { diff --git a/mysql-test/r/show_check.result b/mysql-test/r/show_check.result index 9ef10865cd7..c89f386623b 100644 --- a/mysql-test/r/show_check.result +++ b/mysql-test/r/show_check.result @@ -1282,4 +1282,43 @@ t1_bi CREATE DEFINER=`root`@`localhost` TRIGGER t1_bi BEFORE INSERT ON t1 FOR E DROP TABLE t1; DROP PROCEDURE p1; DEALLOCATE PREPARE stmt1; +set names koi8r; +DROP VIEW IF EXISTS v1; +DROP PROCEDURE IF EXISTS p1; +DROP FUNCTION IF EXISTS f1; +DROP TABLE IF EXISTS t1; +DROP EVENT IF EXISTS ev1; +CREATE VIEW v1 AS SELECT 'ΤΕΣΤ' AS test; +CREATE PROCEDURE p1() SELECT 'ΤΕΣΤ' AS test; +CREATE FUNCTION f1() RETURNS CHAR(10) RETURN 'ΤΕΣΤ'; +CREATE TABLE t1(c1 CHAR(10)); +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 +FOR EACH ROW +SET NEW.c1 = 'ΤΕΣΤ'; +CREATE EVENT ev1 ON SCHEDULE AT '2030-01-01 00:00:00' DO SELECT 'ΤΕΣΤ' AS test; +set names utf8; +SHOW CREATE VIEW v1; +View Create View character_set_client collation_connection +v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select _koi8r'тСст' AS `test` koi8r koi8r_general_ci +SHOW CREATE PROCEDURE p1; +Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation +p1 CREATE DEFINER=`root`@`localhost` PROCEDURE `p1`() +SELECT 'тСст' AS test koi8r koi8r_general_ci latin1_swedish_ci +SHOW CREATE FUNCTION f1; +Function sql_mode Create Function character_set_client collation_connection Database Collation +f1 CREATE DEFINER=`root`@`localhost` FUNCTION `f1`() RETURNS char(10) CHARSET latin1 +RETURN 'тСст' koi8r koi8r_general_ci latin1_swedish_ci +SHOW CREATE TRIGGER t1_bi; +Trigger sql_mode SQL Original Statement character_set_client collation_connection Database Collation +t1_bi CREATE DEFINER=`root`@`localhost` TRIGGER t1_bi BEFORE INSERT ON t1 +FOR EACH ROW +SET NEW.c1 = 'тСст' koi8r koi8r_general_ci latin1_swedish_ci +SHOW CREATE EVENT ev1; +Event sql_mode time_zone Create Event character_set_client collation_connection Database Collation +ev1 SYSTEM CREATE EVENT `ev1` ON SCHEDULE AT '2030-01-01 00:00:00' ON COMPLETION NOT PRESERVE ENABLE DO SELECT 'тСст' AS test koi8r koi8r_general_ci latin1_swedish_ci +DROP VIEW v1; +DROP PROCEDURE p1; +DROP FUNCTION f1; +DROP TABLE t1; +DROP EVENT ev1; End of 5.1 tests diff --git a/mysql-test/t/show_check.test b/mysql-test/t/show_check.test index 341c9039390..2f474d0335b 100644 --- a/mysql-test/t/show_check.test +++ b/mysql-test/t/show_check.test @@ -897,4 +897,68 @@ DROP TABLE t1; DROP PROCEDURE p1; DEALLOCATE PREPARE stmt1; +# +# BUG#10491: Server returns data as charset binary SHOW CREATE TABLE or SELECT +# FROM INFORMATION_SCHEMA. +# +# Before the change performed to fix the bug, the metadata of the output of +# SHOW CREATE statements would always describe the result as 'binary'. That +# would ensure that the result is never converted to character_set_client +# (which was essential to mysqldump). Now we return to the client the actual +# character set of the object -- which is character_set_client of the +# connection that issues the CREATE statement, and this triggers an automatic +# conversion to character_set_results of the connection that issues SHOW CREATE +# statement. +# +# This test demonstrates that this conversion indeed is taking place. +# + +# Prepare: create objects in a one character set. + +set names koi8r; + +--disable_warnings +DROP VIEW IF EXISTS v1; +DROP PROCEDURE IF EXISTS p1; +DROP FUNCTION IF EXISTS f1; +DROP TABLE IF EXISTS t1; +DROP EVENT IF EXISTS ev1; +--enable_warnings + +CREATE VIEW v1 AS SELECT 'ΤΕΣΤ' AS test; + +CREATE PROCEDURE p1() SELECT 'ΤΕΣΤ' AS test; + +CREATE FUNCTION f1() RETURNS CHAR(10) RETURN 'ΤΕΣΤ'; + +CREATE TABLE t1(c1 CHAR(10)); +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 + FOR EACH ROW + SET NEW.c1 = 'ΤΕΣΤ'; + +CREATE EVENT ev1 ON SCHEDULE AT '2030-01-01 00:00:00' DO SELECT 'ΤΕΣΤ' AS test; + +# Test: switch the character set and show that SHOW CREATE output is +# automatically converted to the new character_set_client. + +set names utf8; + +SHOW CREATE VIEW v1; + +SHOW CREATE PROCEDURE p1; + +SHOW CREATE FUNCTION f1; + +SHOW CREATE TRIGGER t1_bi; + +SHOW CREATE EVENT ev1; + +# Cleanup. + +DROP VIEW v1; +DROP PROCEDURE p1; +DROP FUNCTION f1; +DROP TABLE t1; +DROP EVENT ev1; + --echo End of 5.1 tests diff --git a/sql/events.cc b/sql/events.cc index e48daeca63d..8d32580816f 100644 --- a/sql/events.cc +++ b/sql/events.cc @@ -717,7 +717,8 @@ send_show_create_event(THD *thd, Event_timed *et, Protocol *protocol) protocol->store(et->name.str, et->name.length, system_charset_info); protocol->store(sql_mode.str, sql_mode.length, system_charset_info); protocol->store(tz_name->ptr(), tz_name->length(), system_charset_info); - protocol->store(show_str.c_ptr(), show_str.length(), &my_charset_bin); + protocol->store(show_str.c_ptr(), show_str.length(), + et->creation_ctx->get_client_cs()); protocol->store(et->creation_ctx->get_client_cs()->csname, strlen(et->creation_ctx->get_client_cs()->csname), system_charset_info); diff --git a/sql/sp_head.cc b/sql/sp_head.cc index f0cc5204749..d3a9787ee2b 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -2260,7 +2260,8 @@ sp_head::show_create_routine(THD *thd, int type) protocol->store(sql_mode.str, sql_mode.length, system_charset_info); if (full_access) - protocol->store(m_defstr.str, m_defstr.length, &my_charset_bin); + protocol->store(m_defstr.str, m_defstr.length, + m_creation_ctx->get_client_cs()); else protocol->store_null(); diff --git a/sql/sql_show.cc b/sql/sql_show.cc index f66897df671..04374c1d1c1 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -639,7 +639,8 @@ mysqld_show_create(THD *thd, TABLE_LIST *table_list) if (table_list->view) { - protocol->store(buffer.ptr(), buffer.length(), &my_charset_bin); + protocol->store(buffer.ptr(), buffer.length(), + table_list->view_creation_ctx->get_client_cs()); protocol->store(table_list->view_creation_ctx->get_client_cs()->csname, system_charset_info); @@ -5983,6 +5984,8 @@ static bool show_create_trigger_impl(THD *thd, LEX_STRING trg_connection_cl_name; LEX_STRING trg_db_cl_name; + CHARSET_INFO *trg_client_cs; + /* TODO: Check privileges here. This functionality will be added by implementation of the following WL items: @@ -6008,6 +6011,11 @@ static bool show_create_trigger_impl(THD *thd, trg_sql_mode, &trg_sql_mode_str); + /* Resolve trigger client character set. */ + + if (resolve_charset(trg_client_cs_name.str, NULL, &trg_client_cs)) + return TRUE; + /* Send header. */ fields.push_back(new Item_empty_string("Trigger", NAME_LEN)); @@ -6054,7 +6062,7 @@ static bool show_create_trigger_impl(THD *thd, p->store(trg_sql_original_stmt.str, trg_sql_original_stmt.length, - &my_charset_bin); + trg_client_cs); p->store(trg_client_cs_name.str, trg_client_cs_name.length, From 31ea7d042ba48bd34a457bba138d2189f844731d Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 14 Jul 2007 02:04:48 +0400 Subject: [PATCH 3/7] A follow up after the patch for Bug#21074 - even though we now have exclusive name lock on the table name in mysql_rm_table_part2, we still should keep LOCK_open - some storage engines are not ready for locking scope change and assume that LOCK_open is kept. Still, the binary logging and query cache invalidation calls moved out of LOCK_open scope. Fixes some of the broken 5.1-runtime tests (tests break on asserts). sql/ha_ndbcluster.cc: Do not lock LOCK_open for mysql_rm_table_part2 - it does that for us now. sql/mysql_priv.h: Remove an unused flag. sql/sql_class.h: Fix an unrelated compiler warning. sql/sql_db.cc: Adjust to the changed signature. sql/sql_table.cc: mysql_rm_table_part2: we need to keep LOCK_open while calling storage engine functions, even though now we have an exclusive lock on the table name. Some of them assume that it's kept and attempt to unlock it. --- sql/ha_ndbcluster.cc | 6 ++---- sql/mysql_priv.h | 3 +-- sql/sql_class.h | 2 +- sql/sql_db.cc | 2 +- sql/sql_table.cc | 44 ++++++++++++++++++-------------------------- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index 393c2356404..24ce9946086 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -7019,8 +7019,6 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, } } - // Lock mutex before deleting and creating frm files - pthread_mutex_lock(&LOCK_open); if (!global_read_lock) { // Delete old files @@ -7037,14 +7035,14 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, FALSE, /* if_exists */ FALSE, /* drop_temporary */ FALSE, /* drop_view */ - TRUE, /* dont_log_query*/ - FALSE); /* need lock open */ + TRUE /* dont_log_query*/); /* Clear error message that is returned when table is deleted */ thd->clear_error(); } } + pthread_mutex_lock(&LOCK_open); // Create new files List_iterator_fast it2(create_list); while ((file_name=it2++)) diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index e0de89a5b74..8b00cfb5d24 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -901,8 +901,7 @@ void mysql_client_binlog_statement(THD *thd); bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, my_bool drop_temporary); int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, - bool drop_temporary, bool drop_view, bool log_query, - bool need_lock_open); + bool drop_temporary, bool drop_view, bool log_query); bool quick_rm_table(handlerton *base,const char *db, const char *table_name, uint flags); void close_cached_table(THD *thd, TABLE *table); diff --git a/sql/sql_class.h b/sql/sql_class.h index a8d62d93b21..0dad4966623 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1999,8 +1999,8 @@ class select_insert :public select_result_interceptor { class select_create: public select_insert { ORDER *group; TABLE_LIST *create_table; - TABLE_LIST *select_tables; HA_CREATE_INFO *create_info; + TABLE_LIST *select_tables; Alter_info *alter_info; Field **field; public: diff --git a/sql/sql_db.cc b/sql/sql_db.cc index a0c6f8a8e9d..8b0e371be43 100644 --- a/sql/sql_db.cc +++ b/sql/sql_db.cc @@ -1113,7 +1113,7 @@ static long mysql_rm_known_files(THD *thd, MY_DIR *dirp, const char *db, } } if (thd->killed || - (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1, 1))) + (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1))) goto err; /* Remove RAID directories */ diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 31120f8020e..c603f1ad77f 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1430,11 +1430,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, LOCK_open during wait_if_global_read_lock(), other threads could not close their tables. This would make a pretty deadlock. */ - error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0, 1); - pthread_mutex_lock(&thd->mysys_var->mutex); - thd->mysys_var->current_mutex= 0; - thd->mysys_var->current_cond= 0; - pthread_mutex_unlock(&thd->mysys_var->mutex); + error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0); if (need_start_waiters) start_waiting_global_read_lock(thd); @@ -1477,7 +1473,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, bool drop_temporary, bool drop_view, - bool dont_log_query, bool need_lock_open) + bool dont_log_query) { TABLE_LIST *table; char path[FN_REFLEN], *alias; @@ -1489,9 +1485,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, String built_query; DBUG_ENTER("mysql_rm_table_part2"); - if (need_lock_open) - pthread_mutex_lock(&LOCK_open); - LINT_INIT(alias); LINT_INIT(path_length); @@ -1503,6 +1496,9 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, else built_query.append("DROP TABLE "); } + + pthread_mutex_lock(&LOCK_open); + /* If we have the table in the definition cache, we don't have to check the .frm file to find if the table is a normal table (not view) and what @@ -1522,20 +1518,17 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, table->table_name_length, table->table_name, 1)) { my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP"); + pthread_mutex_unlock(&LOCK_open); DBUG_RETURN(1); } } if (!drop_temporary && lock_table_names_exclusively(thd, tables)) { - if (need_lock_open) - pthread_mutex_unlock(&LOCK_open); + pthread_mutex_unlock(&LOCK_open); DBUG_RETURN(1); } - if (need_lock_open) - pthread_mutex_unlock(&LOCK_open); - /* Don't give warnings for not found errors, as we already generate notes */ thd->no_warnings_for_error= 1; @@ -1545,7 +1538,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, handlerton *table_type; enum legacy_db_type frm_db_type; - mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, !need_lock_open); + mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, 1); if (!close_temporary_table(thd, table)) { tmp_table_deleted=1; @@ -1582,8 +1575,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, { TABLE *locked_table; abort_locked_tables(thd, db, table->table_name); - if (need_lock_open) - pthread_mutex_lock(&LOCK_open); remove_table_from_cache(thd, db, table->table_name, RTFC_WAIT_OTHER_THREAD_FLAG | RTFC_CHECK_KILLED_FLAG); @@ -1594,13 +1585,10 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, if ((locked_table= drop_locked_tables(thd, db, table->table_name))) table->table= locked_table; - if (need_lock_open) - pthread_mutex_unlock(&LOCK_open); - if (thd->killed) { - thd->no_warnings_for_error= 0; - DBUG_RETURN(-1); + error= -1; + goto err_with_placeholders; } alias= (lower_case_table_names == 2) ? table->alias : table->table_name; /* remove .frm file and engine files */ @@ -1663,6 +1651,11 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, wrong_tables.append(String(table->table_name,system_charset_info)); } } + /* + It's safe to unlock LOCK_open: we have an exclusive lock + on the table name. + */ + pthread_mutex_unlock(&LOCK_open); thd->tmp_table_used= tmp_table_deleted; error= 0; if (wrong_tables.length()) @@ -1722,11 +1715,10 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, */ } } - if (need_lock_open) - pthread_mutex_lock(&LOCK_open); + pthread_mutex_lock(&LOCK_open); +err_with_placeholders: unlock_table_names(thd, tables, (TABLE_LIST*) 0); - if (need_lock_open) - pthread_mutex_unlock(&LOCK_open); + pthread_mutex_unlock(&LOCK_open); thd->no_warnings_for_error= 0; DBUG_RETURN(error); } From 9a52e13ccb92d6f30bbd4e92e73771b2a2662021 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 14 Jul 2007 04:38:21 +0400 Subject: [PATCH 4/7] Fix a warning in a non-debug build. --- sql/sql_cache.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/sql_cache.cc b/sql/sql_cache.cc index 69efbdf006a..0e4404b87fc 100644 --- a/sql/sql_cache.cc +++ b/sql/sql_cache.cc @@ -793,7 +793,8 @@ void query_cache_end_of_result(THD *thd) query_cache.wreck() switched query cache off but left content untouched for investigation (it is debugging method). */ - goto end; + STRUCT_UNLOCK(&query_cache.structure_guard_mutex); + DBUG_VOID_RETURN; } #endif header->found_rows(current_thd->limit_found_rows); @@ -808,7 +809,6 @@ void query_cache_end_of_result(THD *thd) } -end: STRUCT_UNLOCK(&query_cache.structure_guard_mutex); DBUG_VOID_RETURN; } From 393e0f654172bd90c2b079d79bc7ae381df7540a Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 14 Jul 2007 05:22:24 +0400 Subject: [PATCH 5/7] Fix a compiler warning. --- sql/sql_trigger.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index b5165a29ab4..55eae2f5ea5 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -1805,16 +1805,16 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db, bzero(&table, sizeof(table)); init_alloc_root(&table.mem_root, 8192, 0); - uchar key[MAX_DBKEY_LENGTH]; - uint key_length= (uint) (strmov(strmov((char*)&key[0], db)+1, - old_table)-(char*)&key[0])+1; - /* This method interfaces the mysql server code protected by either LOCK_open mutex or with an exclusive table name lock. In the future, only an exclusive table name lock will be enough. */ #ifndef DBUG_OFF + uchar key[MAX_DBKEY_LENGTH]; + uint key_length= (uint) (strmov(strmov((char*)&key[0], db)+1, + old_table)-(char*)&key[0])+1; + if (!is_table_name_exclusively_locked_by_this_thread(thd, key, key_length)) safe_mutex_assert_owner(&LOCK_open); #endif From 7416224c60a876c5cde518e41469911d1b00b3ce Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 15 Jul 2007 13:25:38 +0400 Subject: [PATCH 6/7] A fix for Bug#27406 Events: failure only causes a warning. Update test results. When executing a CREATE EVENT statement with ON COMPLETION NOT PRESERVE clause (explicit or implicit) and completion date in the past, we do not create the event. Or, put it differently, we create it and then drop immediately. A warning is issued in this case, not an error -- we want to load successfully old database dumps, and such dumps may contain events that are no longer valid. Update the warning text to not imply an erroneous condition. mysql-test/r/events_bugs.result: Update the test results (Bug#27406) sql/share/errmsg.txt: Fix Bug#27406 "Events: failure only causes a warning" -- update the error message to not imply that there was a failure. --- mysql-test/r/events_bugs.result | 14 +++++++------- sql/share/errmsg.txt | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mysql-test/r/events_bugs.result b/mysql-test/r/events_bugs.result index a01f098affb..14728bdcc2e 100644 --- a/mysql-test/r/events_bugs.result +++ b/mysql-test/r/events_bugs.result @@ -31,7 +31,7 @@ create event e_55 on schedule at 10000101000000 do drop table t; ERROR HY000: Incorrect AT value: '10000101000000' create event e_55 on schedule at 20000101000000 do drop table t; Warnings: -Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created +Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. show events; Db Name Definer Time zone Type Execute at Interval value Interval field Starts Ends Status Originator character_set_client collation_connection Database Collation create event e_55 on schedule at 20200101000000 starts 10000101000000 do drop table t; @@ -447,32 +447,32 @@ e3 +00:00 CREATE EVENT `e3` ON SCHEDULE EVERY 1 DAY STARTS '2006-01-01 00:00:00 The following should fail, and nothing should be altered. ALTER EVENT e1 ON SCHEDULE EVERY 1 HOUR STARTS '1999-01-01 00:00:00' ENDS '1999-01-02 00:00:00'; -ERROR HY000: Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been altered +ERROR HY000: Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. ALTER EVENT e1 ON SCHEDULE EVERY 1 HOUR STARTS '1999-01-01 00:00:00' ENDS '1999-01-02 00:00:00' DISABLE; -ERROR HY000: Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been altered +ERROR HY000: Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. The following should give warnings, and nothing should be created. CREATE EVENT e4 ON SCHEDULE EVERY 1 HOUR STARTS '1999-01-01 00:00:00' ENDS '1999-01-02 00:00:00' DO SELECT 1; Warnings: -Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created +Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. CREATE EVENT e4 ON SCHEDULE EVERY 1 HOUR STARTS '1999-01-01 00:00:00' ENDS '1999-01-02 00:00:00' DISABLE DO SELECT 1; Warnings: -Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created +Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. CREATE EVENT e4 ON SCHEDULE AT '1999-01-01 00:00:00' DO SELECT 1; Warnings: -Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created +Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. CREATE EVENT e4 ON SCHEDULE AT '1999-01-01 00:00:00' DISABLE DO SELECT 1; Warnings: -Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created +Note 1584 Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation. SHOW EVENTS; Db Name Definer Time zone Type Execute at Interval value Interval field Starts Ends Status Originator character_set_client collation_connection Database Collation events_test e1 root@localhost +05:00 RECURRING NULL 1 DAY 2006-01-01 00:00:00 NULL ENABLED 1 latin1 latin1_swedish_ci latin1_swedish_ci diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index 0e3544415d1..0b26fa52e32 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -6052,9 +6052,9 @@ ER_BINLOG_PURGE_EMFILE eng "Too many files opened, please execute the command again" ger "Zu viele offene Dateien, bitte fόhren Sie den Befehl noch einmal aus" ER_EVENT_CANNOT_CREATE_IN_THE_PAST - eng "Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been created" + eng "Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation." ER_EVENT_CANNOT_ALTER_IN_THE_PAST - eng "Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. Event has not been altered" + eng "Event execution time is in the past and ON COMPLETION NOT PRESERVE is set. The event was dropped immediately after creation." ER_SLAVE_INCIDENT eng "The incident %s occured on the master. Message: %-.64s" ER_NO_PARTITION_FOR_GIVEN_VALUE_SILENT From 02a832df2e9743fbe132fe84f906761fc00c1279 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 16 Jul 2007 15:57:20 +0400 Subject: [PATCH 7/7] A follow up after the fix for Bug#21074 - fix NDB tests breaking on asserts. The patch for Bug#21074 replaces acquisition of the global LOCK_open lock with exclusive locks on table names in such operations ad DROP TABLE and RENAME TABLE. Unfortunately, NDB internally assumes that LOCK_open is acquired and tries to release it. This dependency should be fixed by a separate (and significant in size) patch. For now we just satisfy it - after all, the original goal of the patch for Bug#21074 was to move query_cache_invalidate outside of the scope of LOCK_open, and we still can do that. This fixes some failing NDB tests in the runtime tree. sql/sql_rename.cc: Move release of LOCK_open after ha_ndbcluster::rename_tables to satisfy an assert in ndb_log_schema_op. --- sql/sql_rename.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index e68a360f2d4..f5e1b8988f3 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -150,7 +150,6 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent) pthread_mutex_unlock(&LOCK_open); goto err; } - pthread_mutex_unlock(&LOCK_open); error=0; if ((ren_table=rename_tables(thd,table_list,0))) @@ -174,6 +173,17 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent) error= 1; } + /* + An exclusive lock on table names is satisfactory to ensure + no other thread accesses this table. + However, NDB assumes that handler::rename_tables is called under + LOCK_open. And it indeed is, from ALTER TABLE. + TODO: remove this limitation. + We still should unlock LOCK_open as early as possible, to provide + higher concurrency - query_cache_invalidate can take minutes to + complete. + */ + pthread_mutex_unlock(&LOCK_open); /* Lets hope this doesn't fail as the result will be messy */ if (!silent && !error)