#At file:///Users/malff/BZR_TREE/mysql-trunk-bug59740/ based on revid:marc.alff@stripped
3567 Marc Alff 2011-01-28
BUG#59740 Performance schema test failures on freebsd
Investigation of bug 59740 shows that selects on
table performance_schema.events_waits_summary_by_thread_by_event_name
sometime to not return the expected rows.
While the root cause of this is not identified yet,
the method table_ews_by_thread_by_event_name::rnd_next()
is suspected to be the origin of the failure.
The code of this method can benefit from simplifications already
identified with other tables, see for example
table_setup_instruments::rnd_next().
This fix implements several improvements for the table scan
of table performance_schema.events_waits_summary_by_thread_by_event_name.
In particular:
- code duplication is reduced
- code complexity is reduced
- the number of calls to thread_array[i].m_lock.is_populated()
is reduced by a factor 200+ (the number of instruments)
This is implemented by changing the iteration on threads from
the innermost index (index 3) to the outermost index (index 1).
While is it unclear if calling atomic operations too frequently
can cause end_optimistic_lock() to fail in ::make_row(),
which could be a possible root cause for the bug found,
reducing the number of calls to atomic operations will make the code
more robust and scalable, so implementing this cleanup is required.
This fix also improves the ortho_iter test, for coverage,
since it also checks table events_waits_summary_by_thread_by_event_name.
modified:
mysql-test/suite/perfschema/r/ortho_iter.result
mysql-test/suite/perfschema/t/ortho_iter.test
storage/perfschema/table_ews_by_thread_by_event_name.cc
storage/perfschema/table_ews_by_thread_by_event_name.h
storage/perfschema/table_ews_global_by_event_name.h
=== modified file 'mysql-test/suite/perfschema/r/ortho_iter.result'
--- a/mysql-test/suite/perfschema/r/ortho_iter.result 2011-01-26 09:34:10 +0000
+++ b/mysql-test/suite/perfschema/r/ortho_iter.result 2011-01-28 13:40:57 +0000
@@ -154,6 +154,15 @@ status
Checking table events_waits_summary_global_by_event_name ...
Warnings:
Warning 12000 Done
+call check_instrument("wait/synch/");
+instr_name is_wait is_stage is_statement
+wait/synch/ 1 0 0
+status
+Checking table events_waits_summary_by_thread_by_event_name ...
+status
+Checking table events_waits_summary_global_by_event_name ...
+Warnings:
+Warning 12000 Done
call check_instrument("wait/io/file/");
instr_name is_wait is_stage is_statement
wait/io/file/ 1 0 0
@@ -181,6 +190,15 @@ status
Checking table events_waits_summary_global_by_event_name ...
Warnings:
Warning 12000 Done
+call check_instrument("wait/io/");
+instr_name is_wait is_stage is_statement
+wait/io/ 1 0 0
+status
+Checking table events_waits_summary_by_thread_by_event_name ...
+status
+Checking table events_waits_summary_global_by_event_name ...
+Warnings:
+Warning 12000 Done
call check_instrument("wait/lock/table/");
instr_name is_wait is_stage is_statement
wait/lock/table/ 1 0 0
@@ -190,6 +208,24 @@ status
Checking table events_waits_summary_global_by_event_name ...
Warnings:
Warning 12000 Done
+call check_instrument("wait/lock/");
+instr_name is_wait is_stage is_statement
+wait/lock/ 1 0 0
+status
+Checking table events_waits_summary_by_thread_by_event_name ...
+status
+Checking table events_waits_summary_global_by_event_name ...
+Warnings:
+Warning 12000 Done
+call check_instrument("wait/");
+instr_name is_wait is_stage is_statement
+wait/ 1 0 0
+status
+Checking table events_waits_summary_by_thread_by_event_name ...
+status
+Checking table events_waits_summary_global_by_event_name ...
+Warnings:
+Warning 12000 Done
call check_instrument("stage/");
instr_name is_wait is_stage is_statement
stage/ 0 1 0
@@ -217,4 +253,13 @@ status
Checking table events_waits_summary_global_by_event_name ...
Warnings:
Warning 12000 Done
+call check_instrument("statement/");
+instr_name is_wait is_stage is_statement
+statement/ 0 0 1
+status
+Checking table events_waits_summary_by_thread_by_event_name ...
+status
+Checking table events_waits_summary_global_by_event_name ...
+Warnings:
+Warning 12000 Done
drop procedure check_instrument;
=== modified file 'mysql-test/suite/perfschema/t/ortho_iter.test'
--- a/mysql-test/suite/perfschema/t/ortho_iter.test 2011-01-26 09:34:10 +0000
+++ b/mysql-test/suite/perfschema/t/ortho_iter.test 2011-01-28 13:40:57 +0000
@@ -142,13 +142,18 @@ show status like "performance_schema%";
call check_instrument("wait/synch/mutex/");
call check_instrument("wait/synch/rwlock/");
call check_instrument("wait/synch/cond/");
+call check_instrument("wait/synch/");
call check_instrument("wait/io/file/");
call check_instrument("wait/io/net/");
call check_instrument("wait/io/table/");
+call check_instrument("wait/io/");
call check_instrument("wait/lock/table/");
+call check_instrument("wait/lock/");
+call check_instrument("wait/");
call check_instrument("stage/");
call check_instrument("statement/com/");
call check_instrument("statement/sql/");
+call check_instrument("statement/");
drop procedure check_instrument;
=== modified file 'storage/perfschema/table_ews_by_thread_by_event_name.cc'
--- a/storage/perfschema/table_ews_by_thread_by_event_name.cc 2011-01-07 16:20:19 +0000
+++ b/storage/perfschema/table_ews_by_thread_by_event_name.cc 2011-01-28 13:40:57 +0000
@@ -115,120 +115,54 @@ void table_ews_by_thread_by_event_name::
int table_ews_by_thread_by_event_name::rnd_next(void)
{
PFS_thread *thread;
- PFS_mutex_class *mutex_class;
- PFS_rwlock_class *rwlock_class;
- PFS_cond_class *cond_class;
- PFS_file_class *file_class;
- PFS_instr_class *table_class;
+ PFS_instr_class *instr_class;
for (m_pos.set_at(&m_next_pos);
- m_pos.has_more_view();
- m_pos.next_view())
+ m_pos.has_more_thread();
+ m_pos.next_thread())
{
- switch (m_pos.m_index_1)
- {
- case pos_ews_by_thread_by_event_name::VIEW_MUTEX:
- do
- {
- mutex_class= find_mutex_class(m_pos.m_index_2);
- if (mutex_class)
- {
- for ( ; m_pos.has_more_thread(); m_pos.next_thread())
- {
- thread= &thread_array[m_pos.m_index_3];
- if (thread->m_lock.is_populated())
- {
- make_row(thread, mutex_class);
- m_next_pos.set_after(&m_pos);
- return 0;
- }
- }
- m_pos.next_instrument();
- }
- } while (mutex_class != NULL);
- break;
- case pos_ews_by_thread_by_event_name::VIEW_RWLOCK:
- do
- {
- rwlock_class= find_rwlock_class(m_pos.m_index_2);
- if (rwlock_class)
- {
- for ( ; m_pos.has_more_thread(); m_pos.next_thread())
- {
- thread= &thread_array[m_pos.m_index_3];
- if (thread->m_lock.is_populated())
- {
- make_row(thread, rwlock_class);
- m_next_pos.set_after(&m_pos);
- return 0;
- }
- }
- m_pos.next_instrument();
- }
- } while (rwlock_class != NULL);
- break;
- case pos_ews_by_thread_by_event_name::VIEW_COND:
- do
- {
- cond_class= find_cond_class(m_pos.m_index_2);
- if (cond_class)
- {
- for ( ; m_pos.has_more_thread(); m_pos.next_thread())
- {
- thread= &thread_array[m_pos.m_index_3];
- if (thread->m_lock.is_populated())
- {
- make_row(thread, cond_class);
- m_next_pos.set_after(&m_pos);
- return 0;
- }
- }
- m_pos.next_instrument();
- }
- } while (cond_class != NULL);
- break;
- case pos_ews_by_thread_by_event_name::VIEW_FILE:
- do
+ thread= &thread_array[m_pos.m_index_1];
+
+ /*
+ Important note: the thread scan is the outer loop (index 1),
+ to minimize the number of calls to atomic operations.
+ */
+ if (thread->m_lock.is_populated())
+ {
+ for ( ;
+ m_pos.has_more_view();
+ m_pos.next_view())
{
- file_class= find_file_class(m_pos.m_index_2);
- if (file_class)
+ switch (m_pos.m_index_2)
{
- for ( ; m_pos.has_more_thread(); m_pos.next_thread())
- {
- thread= &thread_array[m_pos.m_index_3];
- if (thread->m_lock.is_populated())
- {
- make_row(thread, file_class);
- m_next_pos.set_after(&m_pos);
- return 0;
- }
- }
- m_pos.next_instrument();
+ case pos_ews_by_thread_by_event_name::VIEW_MUTEX:
+ instr_class= find_mutex_class(m_pos.m_index_3);
+ break;
+ case pos_ews_by_thread_by_event_name::VIEW_RWLOCK:
+ instr_class= find_rwlock_class(m_pos.m_index_3);
+ break;
+ case pos_ews_by_thread_by_event_name::VIEW_COND:
+ instr_class= find_cond_class(m_pos.m_index_3);
+ break;
+ case pos_ews_by_thread_by_event_name::VIEW_FILE:
+ instr_class= find_file_class(m_pos.m_index_3);
+ break;
+ case pos_ews_by_thread_by_event_name::VIEW_TABLE:
+ instr_class= find_table_class(m_pos.m_index_3);
+ break;
+ default:
+ DBUG_ASSERT(false);
+ instr_class= NULL;
+ break;
}
- } while (file_class != NULL);
- break;
- case pos_ews_by_thread_by_event_name::VIEW_TABLE:
- do
- {
- table_class= find_table_class(m_pos.m_index_2);
- if (table_class)
+
+ if (instr_class != NULL)
{
- for ( ; m_pos.has_more_thread(); m_pos.next_thread())
- {
- thread= &thread_array[m_pos.m_index_3];
- if (thread->m_lock.is_populated())
- {
- make_row(thread, table_class);
- m_next_pos.set_after(&m_pos);
- return 0;
- }
- }
- m_pos.next_instrument();
+ make_row(thread, instr_class);
+ m_next_pos.set_after(&m_pos);
+ return 0;
}
- } while (table_class != NULL);
- break;
- default:
- break;
+ }
}
}
@@ -239,63 +173,42 @@ int
table_ews_by_thread_by_event_name::rnd_pos(const void *pos)
{
PFS_thread *thread;
- PFS_mutex_class *mutex_class;
- PFS_rwlock_class *rwlock_class;
- PFS_cond_class *cond_class;
- PFS_file_class *file_class;
- PFS_instr_class *table_class;
+ PFS_instr_class *instr_class;
set_position(pos);
- DBUG_ASSERT(m_pos.m_index_3 < thread_max);
+ DBUG_ASSERT(m_pos.m_index_1 < thread_max);
- thread= &thread_array[m_pos.m_index_3];
+ thread= &thread_array[m_pos.m_index_1];
if (! thread->m_lock.is_populated())
return HA_ERR_RECORD_DELETED;
- switch (m_pos.m_index_1)
+ switch (m_pos.m_index_2)
{
case pos_ews_by_thread_by_event_name::VIEW_MUTEX:
- mutex_class= find_mutex_class(m_pos.m_index_2);
- if (mutex_class)
- {
- make_row(thread, mutex_class);
- return 0;
- }
+ instr_class= find_mutex_class(m_pos.m_index_3);
break;
case pos_ews_by_thread_by_event_name::VIEW_RWLOCK:
- rwlock_class= find_rwlock_class(m_pos.m_index_2);
- if (rwlock_class)
- {
- make_row(thread, rwlock_class);
- return 0;
- }
+ instr_class= find_rwlock_class(m_pos.m_index_3);
break;
case pos_ews_by_thread_by_event_name::VIEW_COND:
- cond_class= find_cond_class(m_pos.m_index_2);
- if (cond_class)
- {
- make_row(thread, cond_class);
- return 0;
- }
+ instr_class= find_cond_class(m_pos.m_index_3);
break;
case pos_ews_by_thread_by_event_name::VIEW_FILE:
- file_class= find_file_class(m_pos.m_index_2);
- if (file_class)
- {
- make_row(thread, file_class);
- return 0;
- }
+ instr_class= find_file_class(m_pos.m_index_3);
break;
case pos_ews_by_thread_by_event_name::VIEW_TABLE:
- table_class= find_table_class(m_pos.m_index_2);
- if (table_class)
- {
- make_row(thread, table_class);
- return 0;
- }
+ instr_class= find_table_class(m_pos.m_index_3);
break;
+ default:
+ DBUG_ASSERT(false);
+ instr_class= NULL;
}
+ if (instr_class)
+ {
+ make_row(thread, instr_class);
+ return 0;
+ }
return HA_ERR_RECORD_DELETED;
}
=== modified file 'storage/perfschema/table_ews_by_thread_by_event_name.h'
--- a/storage/perfschema/table_ews_by_thread_by_event_name.h 2011-01-07 16:20:19 +0000
+++ b/storage/perfschema/table_ews_by_thread_by_event_name.h 2011-01-28 13:40:57 +0000
@@ -49,46 +49,41 @@ struct row_ews_by_thread_by_event_name
/**
Position of a cursor on
PERFORMANCE_SCHEMA.EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME.
- Index 1 on instrument view
- Index 2 on instrument class (1 based)
- Index 3 on thread (0 based)
+ Index 1 on thread (0 based)
+ Index 2 on instrument view
+ Index 3 on instrument class (1 based)
*/
struct pos_ews_by_thread_by_event_name
: public PFS_triple_index, public PFS_instrument_view_constants
{
pos_ews_by_thread_by_event_name()
- : PFS_triple_index(FIRST_VIEW, 1, 0)
+ : PFS_triple_index(0, FIRST_VIEW, 1)
{}
inline void reset(void)
{
- m_index_1= FIRST_VIEW;
- m_index_2= 1;
- m_index_3= 0;
+ m_index_1= 0;
+ m_index_2= FIRST_VIEW;
+ m_index_3= 1;
}
- inline bool has_more_view(void)
- { return (m_index_1 <= LAST_VIEW); }
-
inline bool has_more_thread(void)
- { return (m_index_3 < thread_max); }
+ { return (m_index_1 < thread_max); }
- inline void next_view(void)
+ inline void next_thread(void)
{
m_index_1++;
- m_index_2= 1;
- m_index_3= 0;
+ m_index_2= FIRST_VIEW;
+ m_index_3= 1;
}
- inline void next_instrument(void)
- {
- m_index_2++;
- m_index_3= 0;
- }
+ inline bool has_more_view(void)
+ { return (m_index_2 <= LAST_VIEW); }
- inline void next_thread(void)
+ inline void next_view(void)
{
- m_index_3++;
+ m_index_2++;
+ m_index_3= 1;
}
};
=== modified file 'storage/perfschema/table_ews_global_by_event_name.h'
--- a/storage/perfschema/table_ews_global_by_event_name.h 2011-01-07 16:20:19 +0000
+++ b/storage/perfschema/table_ews_global_by_event_name.h 2011-01-28 13:40:57 +0000
@@ -71,11 +71,6 @@ struct pos_ews_global_by_event_name
m_index_1++;
m_index_2= 1;
}
-
- inline void next_instrument(void)
- {
- m_index_2++;
- }
};
/** Table PERFORMANCE_SCHEMA.EVENTS_WAITS_SUMMARY_GLOBAL_BY_EVENT_NAME. */
Attachment: [text/bzr-bundle] bzr/marc.alff@oracle.com-20110128134057-i3px1zc1e6t8dgat.bundle
| Thread |
|---|
| • bzr commit into mysql-trunk branch (marc.alff:3567) Bug#59740 | Marc Alff | 28 Jan |
