From: Marc Alff Date: September 26 2011 2:07pm Subject: bzr push into mysql-trunk branch (marc.alff:3414 to 3415) Bug#12993572 List-Archive: http://lists.mysql.com/commits/141145 X-Bug: 12993572 Message-Id: <201109261408.p8QE8A0k024346@acsmt357.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3415 Marc Alff 2011-09-26 BUG#12993572 - P_S STANDBY MODE CAUSES PERFORMANCE DROP Backport for mysql-5.6.3-m4-release This fix is a performance improvement. Before this fix, the performance schema unbind_table() and rebind_table() api were called from sql/sql_base.cc while holding LOCK_open. As the implementation of these api can involve updating table statistics for the current table handle, these calls did cause significant bottlenecks for LOCK_open. This fix: - moved the call to unbind_table() before adding a table handle to the table definition cache (which uses LOCK_open), so the code is outside the critical section. - moved the call to rebind_table() after a free handle has been reused from the table definition cache. This is now also outside of the LOCK_open critical section. - The implementation of PFS_table::aggregate(), which is part of unbind_table(), has been optimized to be simpler when there are no statistics to update. Given that table lock statistics are collected for 11 different types of lock (see PFS_TL_LOCK_TYPE), and that table io stats are collected for 4 types (fetch, insert, update, delete), per table and per index, this decreases the overhead when the table instrumentation is not used. modified: sql/sql_base.cc storage/perfschema/pfs.cc storage/perfschema/pfs_instr.cc storage/perfschema/pfs_instr.h 3414 Guilhem Bichot 2011-09-23 Fix for Bug#13009176 "USE DATABASE BREAKS OPTIMIZER TRACE IF THERE IS A VIEW IN THE SCHEMA" COM_FIELD_LIST opened a view when opt-trace-specific security checks could not be run, which, as a security defense, caused tracing to be disabled for the entire session. See sql_parse.cc. @ sql/opt_trace2server.cc updating comment and assertions. @ sql/sql_parse.cc See the long comment in opt_trace_disable_if_no_security_context_access(). No testcase, because to provoke the bug we need the 'mysql' command-line client to build its "completion hash". It doesn't do that when 'mysql' is run either with option --execute or when stdin is redirected from a file (is not a terminal) (both cases causes status.batch to be 1, which shortcuts build_completion_hash()). Running 'mysql' in the mtr suite is done with one of these two ways, so we cannot add a test. But I tested manually. modified: sql/opt_trace2server.cc sql/sql_parse.cc === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2011-08-19 13:04:28 +0000 +++ b/sql/sql_base.cc 2011-09-26 14:07:09 +0000 @@ -542,8 +542,6 @@ static void table_def_use_table(THD *thd DBUG_ASSERT(table->db_stat && table->file); /* The children must be detached from the table. */ DBUG_ASSERT(! table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); - - table->file->rebind_psi(); } @@ -560,7 +558,6 @@ static void table_def_unuse_table(TABLE DBUG_ASSERT(! table->s->has_old_version()); table->in_use= 0; - table->file->unbind_psi(); /* Remove table from the list of tables used in this share. */ table->s->used_tables.remove(table); @@ -1655,6 +1652,10 @@ bool close_thread_table(THD *thd, TABLE table->file->ha_reset(); } + /* Do this *before* entering the LOCK_open critical section. */ + if (table->file != NULL) + table->file->unbind_psi(); + mysql_mutex_lock(&LOCK_open); if (table->s->has_old_version() || table->needs_reopen() || @@ -2731,6 +2732,8 @@ bool open_table(THD *thd, TABLE_LIST *ta int error; TABLE_SHARE *share; my_hash_value_type hash_value; + bool recycled_free_table; + DBUG_ENTER("open_table"); /* @@ -3088,6 +3091,7 @@ retry_share: { table= share->free_tables.front(); table_def_use_table(thd, table); + recycled_free_table= true; /* We need to release share as we have EXTRA reference to it in our hands. */ release_table_share(share); } @@ -3099,6 +3103,7 @@ retry_share: mysql_mutex_unlock(&LOCK_open); + recycled_free_table= false; /* make a new table */ if (!(table=(TABLE*) my_malloc(sizeof(*table),MYF(MY_WME)))) goto err_lock; @@ -3140,6 +3145,13 @@ retry_share: mysql_mutex_unlock(&LOCK_open); + /* Call rebind_psi outside of the LOCK_open critical section. */ + if (recycled_free_table) + { + DBUG_ASSERT(table->file != NULL); + table->file->rebind_psi(); + } + table->mdl_ticket= mdl_ticket; table->next= thd->open_tables; /* Link into simple list */ === modified file 'storage/perfschema/pfs.cc' --- a/storage/perfschema/pfs.cc 2011-08-18 16:15:02 +0000 +++ b/storage/perfschema/pfs.cc 2011-09-26 14:07:09 +0000 @@ -3710,6 +3710,8 @@ static void end_table_io_wait_v1(PSI_tab insert_events_waits_history_long(wait); thread->m_events_waits_count--; } + + table->m_has_io_stats= true; } /** @@ -3787,6 +3789,8 @@ static void end_table_lock_wait_v1(PSI_t insert_events_waits_history_long(wait); thread->m_events_waits_count--; } + + table->m_has_lock_stats= true; } static void start_file_wait_v1(PSI_file_locker *locker, === modified file 'storage/perfschema/pfs_instr.cc' --- a/storage/perfschema/pfs_instr.cc 2011-08-18 12:27:21 +0000 +++ b/storage/perfschema/pfs_instr.cc 2011-09-26 14:07:09 +0000 @@ -1306,6 +1306,8 @@ PFS_table* create_table(PFS_table_share pfs->m_lock_enabled= share->m_enabled && flag_global_instrumentation && global_table_lock_class.m_enabled; pfs->m_lock_timed= share->m_timed && global_table_lock_class.m_timed; + pfs->m_has_io_stats= false; + pfs->m_has_lock_stats= false; share->inc_refcount(); pfs->m_table_stat.reset(); pfs->m_thread_owner= opening_thread; @@ -1327,24 +1329,35 @@ void PFS_table::sanitized_aggregate(void */ PFS_table_share *safe_share= sanitize_table_share(m_share); PFS_thread *safe_thread= sanitize_thread(m_thread_owner); - if (safe_share != NULL && safe_thread != NULL) + if ((safe_share != NULL && safe_thread != NULL) && + (m_has_io_stats || m_has_lock_stats)) + { safe_aggregate(& m_table_stat, safe_share, safe_thread); + m_has_io_stats= false; + m_has_lock_stats= false; + } } void PFS_table::sanitized_aggregate_io(void) { PFS_table_share *safe_share= sanitize_table_share(m_share); PFS_thread *safe_thread= sanitize_thread(m_thread_owner); - if (safe_share != NULL && safe_thread != NULL) + if (safe_share != NULL && safe_thread != NULL && m_has_io_stats) + { safe_aggregate_io(& m_table_stat, safe_share, safe_thread); + m_has_io_stats= false; + } } void PFS_table::sanitized_aggregate_lock(void) { PFS_table_share *safe_share= sanitize_table_share(m_share); PFS_thread *safe_thread= sanitize_thread(m_thread_owner); - if (safe_share != NULL && safe_thread != NULL) + if (safe_share != NULL && safe_thread != NULL && m_has_lock_stats) + { safe_aggregate_lock(& m_table_stat, safe_share, safe_thread); + m_has_lock_stats= false; + } } void PFS_table::safe_aggregate(PFS_table_stat *table_stat, @@ -1361,11 +1374,17 @@ void PFS_table::safe_aggregate(PFS_table uint index; event_name_array= thread->m_instr_class_waits_stats; - /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/io/table/sql/handler) */ + /* + Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME + (for wait/io/table/sql/handler) + */ index= global_table_io_class.m_event_name_index; table_stat->sum_io(& event_name_array[index]); - /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/lock/table/sql/handler) */ + /* + Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME + (for wait/lock/table/sql/handler) + */ index= global_table_lock_class.m_event_name_index; table_stat->sum_lock(& event_name_array[index]); } @@ -1389,7 +1408,10 @@ void PFS_table::safe_aggregate_io(PFS_ta uint index; event_name_array= thread->m_instr_class_waits_stats; - /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/io/table/sql/handler) */ + /* + Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME + (for wait/io/table/sql/handler) + */ index= global_table_io_class.m_event_name_index; table_stat->sum_io(& event_name_array[index]); } @@ -1413,7 +1435,10 @@ void PFS_table::safe_aggregate_lock(PFS_ uint index; event_name_array= thread->m_instr_class_waits_stats; - /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/lock/table/sql/handler) */ + /* + Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME + (for wait/lock/table/sql/handler) + */ index= global_table_lock_class.m_event_name_index; table_stat->sum_lock(& event_name_array[index]); } === modified file 'storage/perfschema/pfs_instr.h' --- a/storage/perfschema/pfs_instr.h 2011-08-18 12:27:21 +0000 +++ b/storage/perfschema/pfs_instr.h 2011-09-26 14:07:09 +0000 @@ -180,6 +180,12 @@ struct PFS_table */ bool m_lock_timed; + /** True if table io statistics have been collected. */ + bool m_has_io_stats; + + /** True if table lock statistics have been collected. */ + bool m_has_lock_stats; + public: /** Aggregate this table handle statistics to the parents. @@ -188,8 +194,12 @@ public: */ void aggregate(void) { - if (likely(m_thread_owner != NULL)) + if (likely((m_thread_owner != NULL) && (m_has_io_stats || m_has_lock_stats))) + { safe_aggregate(& m_table_stat, m_share, m_thread_owner); + m_has_io_stats= false; + m_has_lock_stats= false; + } } /** No bundle (reason: useless for push emails).