Merge into 5.5 : 5.5-ps-blueprint-scalability-metrics : Code : Percona Server moved to https://jira.percona.com/projects/PS

Status:	Merged
Approved by:	Alexey Kopytov on 2014-03-16
Approved revision:	461
Merged at revision:	638
Proposed branch:	lp:~sergei.glushchenko/percona-server/5.5-ps-blueprint-scalability-metrics
Merge into:	lp:percona-server/5.5
Diff against target:	688 lines (+639/-0) 8 files modified mysql-test/include/have_scalability_metrics_plugin.inc (+21/-0) mysql-test/include/plugin.defs (+1/-0) mysql-test/r/scalability_metrics.result (+65/-0) mysql-test/t/scalability_metrics-master.opt (+1/-0) mysql-test/t/scalability_metrics.test (+81/-0) plugin/scalability_metrics/CMakeLists.txt (+17/-0) plugin/scalability_metrics/scalability_metrics.c (+451/-0) sql/item_func.cc (+2/-0)
To merge this branch:	bzr merge lp:~sergei.glushchenko/percona-server/5.5-ps-blueprint-scalability-metrics
Related bugs:	Link a bug report
Related blueprints:	Metrics for scalability measurement (High)

Reviewer	Review Type	Date Requested	Status
Alexey Kopytov (community)		2013-12-09	Approve on 2014-03-16
Review via email: mp+198219@code.launchpad.net

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2013-12-09:

#

Jenkins build
http://jenkins.percona.com/job/percona-server-5.5-param/905/

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2013-12-16:

#

Stable test case should be constructed, current test case has issues.

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2013-12-16:

#

http://jenkins.percona.com/job/percona-server-5.5-param/915/

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2013-12-18:

#

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/918/

it is finally stable, however I didn't find a way to avoid sleep after kill. I need to wait until killed thread invokes audit_notify, but I don't have any good sign that query was killed. the->killed doesn't work at the place needed because killed SELECT SLEEP() doesn't actually treated like a killed of terminated with error, but rather as successfully executed query.

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2013-12-19:

#

Download full text (3.9 KiB)

Sergei,

This looks good in general. I suspect that implementing per-thread
accounting via TLS / my_pthread_getspecific() would be more elegant and
a bit more efficient. But the current approach looks acceptable.

Some comments on the patch:

- could you also document scalability_metrics_ctl and the status vars
in the blueprint?

- let’s use CLOCK_MONOTONIC_RAW when available?

- you can use HAVE_CLOCK_GETTIME instead of “(defined _POSIX_TIMERS) &&
(_POSIX_TIMERS > 0)”

  - the copyright notice should be “Percona LLC and/or its affiliates”
    rather than “Percona Ireland Ltd”. Same for the author field in
    mysql_declare_plugin

  - string.h / time.h / unistd.h – all should be included by
    my_global.h. You can either wrap them into HAVE_... macros, or just
    rely on my_global.h to include them

- do you really need to include CoreServices/CoreServices.h and libkern/OSAtomic.h?

  - stdint.h is incompatible with Windows. And the server code expects
    my_global.h types anyway (i.e. longlong, etc.). I understand that
    stdint.h types are more explicit, but since we have to use server
    types anyway, and longlong types are guaranteed to be 64-bit on all
    platforms, let’s use them instead?

- the __attribute__ portability code is redundant (taken care of in
my_global.h)

  - use my_micro_time() if clock_gettime() is not available? That gives
    us less OSX-specific code and more supported platforms at the cost
    of slightly less efficient code on OSX. Which is not a priority
    anyway.

- thd_list_mutex is supposed to protect thd_list, but in sm_reset() it
is used to reset other global variables? What was the intention?

- would scalability_metrics_control be a better name for scalability_metrics_ctl?

- it looks like sm_thd_data_t::end is not really needed (can be an
auto variable in sm_query_finished)

- the timestamp global variable is also not really needed

- there is a race condition in sm_query_finished() around the
concurrency check:

    suppose that multiple threads first execute
    __sync_bool_compare_and_swap(&concurrency, 1, 0) (which would return
    false), and then all proceed to decrement concurrency. If it becomes
    0 as the result, busytime will not be updated.

- memory leak in sm_thd_data_release(): list_delete() does not free
LIST structure allocated by list_push()

  - sequential ‘if’ statements in sm_notify() in case event_class ==
    MYSQL_AUDIT_GENERAL_CLASS should be “else if”. Otherwise we will be
    checking all conditions even in the common case (i.e. Query events)

  - shouldn’t the check for (sm_ctl == CTL_ON) be done regardless of
    event_class? Otherwise we will be doing unnecessary bookkeeping for
    MYSQL_AUDIT_CONNECTION_CLASS events even if the plugin functionality
    is turned off

- shouldn’t setting sm_ctl to CTL_OFF also release all previously
allocated memory and destroy thd_list?

  - I don’t quite understand the current logic wrt sm_reset() and
    query_series. So if we reset the stats, setting starttime to the
    current time looks correct. Setting concurrency to 0 does not (there
    may still be running queries)....

Sergei,

This looks good in general. I suspect that implementing per-thread
accounting via TLS / my_pthread_getspecific() would be more elegant and
a bit more efficient. But the current approach looks acceptable.

Some comments on the patch:

- could you also document scalability_metrics_ctl and the status vars
    in the blueprint?

- let’s use CLOCK_MONOTONIC_RAW when available?

- you can use HAVE_CLOCK_GETTIME instead of “(defined _POSIX_TIMERS) &&
    (_POSIX_TIMERS > 0)”

- the copyright notice should be “Percona LLC and/or its affiliates”
    rather than “Percona Ireland Ltd”. Same for the author field in
    mysql_declare_plugin

- string.h / time.h / unistd.h – all should be included by
    my_global.h. You can either wrap them into HAVE_... macros, or just
    rely on my_global.h to include them

- do you really need to include CoreServices/CoreServices.h and libkern/OSAtomic.h?

- stdint.h is incompatible with Windows. And the server code expects
    my_global.h types anyway (i.e. longlong, etc.). I understand that
    stdint.h types are more explicit, but since we have to use server
    types anyway, and longlong types are guaranteed to be 64-bit on all
    platforms, let’s use them instead?

- the __attribute__ portability code is redundant (taken care of in
    my_global.h)

- use my_micro_time() if clock_gettime() is not available?  That gives
    us less OSX-specific code and more supported platforms at the cost
    of slightly less efficient code on OSX. Which is not a priority
    anyway.

- thd_list_mutex is supposed to protect thd_list, but in sm_reset() it
    is used to reset other global variables? What was the intention?

- would scalability_metrics_control be a better name for scalability_metrics_ctl?

- it looks like sm_thd_data_t::end is not really needed (can be an
    auto variable in sm_query_finished)

- the timestamp global variable is also not really needed

- there is a race condition in sm_query_finished() around the
    concurrency check:

suppose that multiple threads first execute
    __sync_bool_compare_and_swap(&concurrency, 1, 0) (which would return
    false), and then all proceed to decrement concurrency. If it becomes
    0 as the result, busytime will not be updated.

- memory leak in sm_thd_data_release(): list_delete() does not free
    LIST structure allocated by list_push()

- sequential ‘if’ statements in sm_notify() in case event_class ==
    MYSQL_AUDIT_GENERAL_CLASS should be “else if”. Otherwise we will be
    checking all conditions even in the common case (i.e. Query events)

- shouldn’t the check for (sm_ctl == CTL_ON) be done regardless of
    event_class? Otherwise we will be doing unnecessary bookkeeping for
    MYSQL_AUDIT_CONNECTION_CLASS events even if the plugin functionality
    is turned off

- shouldn’t setting sm_ctl to CTL_OFF also release all previously
    allocated memory and destroy thd_list?

- I don’t quite understand the current logic wrt sm_reset() and
    query_series. So if we reset the stats, setting starttime to the
    current time looks correct. Setting concurrency to 0 does not (there
    may still be running queries). concurrency and busytime will get
    skewed as a result (which can also be observed in the test case
    results).  Shouldn’t you also set busystart to the current
    time depending on the current concurrency value?

- query_series. I understand that it has been implemented to prevent
    queries that started before a reset from accounting. But why do we
    want to do that? Suppose we have 100 queries in progress at the time
    we issue a reset. Those queries will complete sometime later, but
    will not be reflected in the stats? This is also visible in the test
    case.

- Jenkins: note failures on centos5 (use ULONGLONG_MAX instead of
    ULLONG_MAX) and debug build failure on centos6

- testcase: you can avoid “sleep 3” with the
    count_sessions/wait_until_count_sessions combo.

review: Needs Fixing

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2013-12-19:

#

Forgot to add this:

- any reasons to use GCC atomics directly instead of the my_atomic.h ones?

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2014-01-06:

#

Download full text (5.3 KiB)

Alexey,

>
> This looks good in general. I suspect that implementing per-thread
> accounting via TLS / my_pthread_getspecific() would be more elegant and
> a bit more efficient. But the current approach looks acceptable.
>

the very first issue with TLS is that THD will float between OS threads in
case of threadpool

> Some comments on the patch:
>
> - could you also document scalability_metrics_ctl and the status vars
> in the blueprint?
>

done.

> - let’s use CLOCK_MONOTONIC_RAW when available?
>

done.

> - you can use HAVE_CLOCK_GETTIME instead of “(defined _POSIX_TIMERS) &&
> (_POSIX_TIMERS > 0)”
>

yup, that is look better.

> - the copyright notice should be “Percona LLC and/or its affiliates”
> rather than “Percona Ireland Ltd”. Same for the author field in
> mysql_declare_plugin
>

fixed

> - string.h / time.h / unistd.h – all should be included by
> my_global.h. You can either wrap them into HAVE_... macros, or just
> rely on my_global.h to include them
>

string.h is not included by my_global.h, but it is included by m_string.h

> - do you really need to include CoreServices/CoreServices.h and libkern/OSAtomic.h?
>

they were left after earlier experiments, it's a garbage

> - stdint.h is incompatible with Windows. And the server code expects
> my_global.h types anyway (i.e. longlong, etc.). I understand that
> stdint.h types are more explicit, but since we have to use server
> types anyway, and longlong types are guaranteed to be 64-bit on all
> platforms, let’s use them instead?
>

changed all to ulonglong

> - the __attribute__ portability code is redundant (taken care of in
> my_global.h)
>

yes, it is

> - use my_micro_time() if clock_gettime() is not available? That gives
> us less OSX-specific code and more supported platforms at the cost
> of slightly less efficient code on OSX. Which is not a priority
> anyway.
>

done

> - thd_list_mutex is supposed to protect thd_list, but in sm_reset() it
> is used to reset other global variables? What was the intention?
>

there is no special need. removed.

> - would scalability_metrics_control be a better name for scalability_metrics_ctl?
>

sounds good. renamed.

> - it looks like sm_thd_data_t::end is not really needed (can be an
> auto variable in sm_query_finished)
>

indeed, it is

> - the timestamp global variable is also not really needed
>

yes

> - there is a race condition in sm_query_finished() around the
> concurrency check:
>
> suppose that multiple threads first execute
> __sync_bool_compare_and_swap(&concurrency, 1, 0) (which would return
> false), and then all proceed to decrement concurrency. If it becomes
> 0 as the result, busytime will not be updated.
>

agree. Changed to decrement and test for zero.

> - memory leak in sm_thd_data_release(): list_delete() does not free
> LIST structure allocated by list_push()
>

thanks for pointing this. I messed up list_pop adn list_delete

> - sequential ‘if’ statements in sm_notify() in case event_class ==
> MYSQL_AUDIT_GENERAL_CLASS should be “else if”. Otherwise we will be
> ch...

Alexey,

> 
> This looks good in general. I suspect that implementing per-thread
> accounting via TLS / my_pthread_getspecific() would be more elegant and
> a bit more efficient. But the current approach looks acceptable.
>

the very first issue with TLS is that THD will float between OS threads in
case of threadpool

> Some comments on the patch:
> 
>   - could you also document scalability_metrics_ctl and the status vars
>     in the blueprint?
>

done.

>   - let’s use CLOCK_MONOTONIC_RAW when available?
>

done.

>   - you can use HAVE_CLOCK_GETTIME instead of “(defined _POSIX_TIMERS) &&
>     (_POSIX_TIMERS >  0)”
>

yup, that is look better.

>   - the copyright notice should be “Percona LLC and/or its affiliates”
>     rather than “Percona Ireland Ltd”. Same for the author field in
>     mysql_declare_plugin
>

fixed

>   - string.h / time.h / unistd.h – all should be included by
>     my_global.h. You can either wrap them into HAVE_... macros, or just
>     rely on my_global.h to include them
>

string.h is not included by my_global.h, but it is included by m_string.h

>   - do you really need to include CoreServices/CoreServices.h and libkern/OSAtomic.h?
>

they were left after earlier experiments, it's a garbage

>   - stdint.h is incompatible with Windows. And the server code expects
>     my_global.h types anyway (i.e. longlong, etc.). I understand that
>     stdint.h types are more explicit, but since we have to use server
>     types anyway, and longlong types are guaranteed to be 64-bit on all
>     platforms, let’s use them instead?
>

changed all to ulonglong

>   - the __attribute__ portability code is redundant (taken care of in
>     my_global.h)
>

yes, it is

>   - use my_micro_time() if clock_gettime() is not available? That gives
>     us less OSX-specific code and more supported platforms at the cost
>     of slightly less efficient code on OSX. Which is not a priority
>     anyway.
>

done

>   - thd_list_mutex is supposed to protect thd_list, but in sm_reset() it
>     is used to reset other global variables? What was the intention?
>

there is no special need. removed.

>   - would scalability_metrics_control be a better name for scalability_metrics_ctl?
>

sounds good. renamed.

>   - it looks like sm_thd_data_t::end is not really needed (can be an
>     auto variable in sm_query_finished)
>

indeed, it is

>   - the timestamp global variable is also not really needed
>

yes

>   - there is a race condition in sm_query_finished() around the
>     concurrency check:
> 
>     suppose that multiple threads first execute
>     __sync_bool_compare_and_swap(&concurrency, 1, 0) (which would return
>     false), and then all proceed to decrement concurrency. If it becomes
>     0 as the result, busytime will not be updated.
>

agree. Changed to decrement and test for zero.

>   - memory leak in sm_thd_data_release(): list_delete() does not free
>     LIST structure allocated by list_push()
>

thanks for pointing this. I messed up list_pop adn list_delete

>   - sequential ‘if’ statements in sm_notify() in case event_class ==
>     MYSQL_AUDIT_GENERAL_CLASS should be “else if”. Otherwise we will be
>     checking all conditions even in the common case (i.e. Query events)
>

agree

>   - shouldn’t the check for (sm_ctl == CTL_ON) be done regardless of
>     event_class? Otherwise we will be doing unnecessary bookkeeping for
>     MYSQL_AUDIT_CONNECTION_CLASS events even if the plugin functionality
>     is turned off
>

This is needed because we don't want to miss disconnect events and forget to release memory. (see below)

>   - shouldn’t setting sm_ctl to CTL_OFF also release all previously
>     allocated memory and destroy thd_list?
>

It will not work because we do not protect thd_data from being destroyed. We can introduce mutex for each thd_data, but shoul we?

>   - I don’t quite understand the current logic wrt sm_reset() and
>     query_series. So if we reset the stats, setting starttime to the
>     current time looks correct. Setting concurrency to 0 does not (there
>     may still be running queries). concurrency and busytime will get
>     skewed as a result (which can also be observed in the test case
>     results). Shouldn’t you also set busystart to the current
>     time depending on the current concurrency value?
>

the idea was that RESET will work like OFF followed by ON. So we should abandon all
currently running queries.

>   - query_series. I understand that it has been implemented to prevent
>     queries that started before a reset from accounting. But why do we
>     want to do that? Suppose we have 100 queries in progress at the time
>     we issue a reset. Those queries will complete sometime later, but
>     will not be reflected in the stats? This is also visible in the test
>     case.
>

Yes, they will not be reflected. Following the logic RESET=OFF+ON all the queries running at the "ON" moment will not be reflected, same done for RESET.

>   - Jenkins: note failures on centos5 (use ULONGLONG_MAX instead of
>     ULLONG_MAX) and debug build failure on centos6
>

wow. my bad not to notice it.

>   - testcase: you can avoid “sleep 3” with the
>     count_sessions/wait_until_count_sessions combo.

wait_until_count_sessions will produce variable number of queries during its execution, which I am trying to avoid

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2014-01-06:

#

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/926/

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2014-01-07:

#

Download full text (6.4 KiB)

Hi Sergei,

On Mon, Jan 06 2014 17:10:58 +0400, Sergei Glushchenko wrote:

> Alexey,
>
>>
>> This looks good in general. I suspect that implementing per-thread
>> accounting via TLS / my_pthread_getspecific() would be more elegant and
>> a bit more efficient. But the current approach looks acceptable.
>>
>
> the very first issue with TLS is that THD will float between OS threads in
> case of threadpool
>

Right, but that’s precisely what you need in terms of scalability
metrics. I.e. you need to keep track of concurrent queries
(i.e. execution threads), rather than user connections. With the default
scheduler, there’s 1:1 mapping, so there’s no difference
(functionality-wise, not implementation-wise). But in case of
threadpool, if you have, say, thousands of connections with only a few
worker threads executing queries, you can keep stats per connection, but
that would be redundant, as tracking per-thread stats would be
sufficient.

[...]

>
>> - the copyright notice should be “Percona LLC and/or its affiliates”
>> rather than “Percona Ireland Ltd”. Same for the author field in
>> mysql_declare_plugin
>>
>
> fixed

The ‘author’ field in mysql_declare_plugin() stills reads “Percona Ireland Ltd.”

>
>> - string.h / time.h / unistd.h – all should be included by
>> my_global.h. You can either wrap them into HAVE_... macros, or just
>> rely on my_global.h to include them
>>
>
> string.h is not included by my_global.h, but it is included by m_string.h
>

OK, though I don’t think you need to include the entire m_string.h just
to include string.h.

[...]

>> - use my_micro_time() if clock_gettime() is not available? That gives
>> us less OSX-specific code and more supported platforms at the cost
>> of slightly less efficient code on OSX. Which is not a priority
>> anyway.
>>
>
> done

Hm, but now we have wrong values being returned by
sm_clock_time_duration() when my_micro_time() is used. I.e. all
timestamps are already in microseconds, we don’t have to divide them by
1000. Better yet, do division for the clock_gettime() case in
sm_clock_time_get() and then you won’t need sm_clock_time_duration() at all.

[...]

>
>> - it looks like sm_thd_data_t::end is not really needed (can be an
>> auto variable in sm_query_finished)
>>
>
> indeed, it is

Thanks. But I wonder why ‘end’ and ‘save_busystart’ have also become volatile.

[...]

>> - shouldn’t the check for (sm_ctl == CTL_ON) be done regardless of
>> event_class? Otherwise we will be doing unnecessary bookkeeping for
>> MYSQL_AUDIT_CONNECTION_CLASS events even if the plugin functionality
>> is turned off
>>
>
> This is needed because we don't want to miss disconnect events and forget to release memory. (see below)
>
>> - shouldn’t setting sm_ctl to CTL_OFF also release all previously
>> allocated memory and destroy thd_list?
>>
>
> It will not work because we do not protect thd_data from being
> destroyed. We can introduce mutex for each thd_data, but shoul we?

Right, I see. No, mutex will not help us here. What we want to achieve
here is make getting a pointer from THDVAR and dereferencing it to get
its own mutex an atomic opera...

Hi Sergei,

On Mon, Jan 06 2014 17:10:58 +0400, Sergei Glushchenko wrote:

> Alexey,
>
>> 
>> This looks good in general. I suspect that implementing per-thread
>> accounting via TLS / my_pthread_getspecific() would be more elegant and
>> a bit more efficient. But the current approach looks acceptable.
>> 
>
> the very first issue with TLS is that THD will float between OS threads in
> case of threadpool
>

Right, but that’s precisely what you need in terms of scalability
metrics. I.e. you need to keep track of concurrent queries
(i.e. execution threads), rather than user connections. With the default
scheduler, there’s 1:1 mapping, so there’s no difference
(functionality-wise, not implementation-wise). But in case of
threadpool, if you have, say, thousands of connections with only a few
worker threads executing queries, you can keep stats per connection, but
that would be redundant, as tracking per-thread stats would be
sufficient.

[...]

>
>>   - the copyright notice should be “Percona LLC and/or its affiliates”
>>     rather than “Percona Ireland Ltd”. Same for the author field in
>>     mysql_declare_plugin
>> 
>
> fixed

The ‘author’ field in mysql_declare_plugin() stills reads “Percona Ireland Ltd.”

>
>>   - string.h / time.h / unistd.h – all should be included by
>>     my_global.h. You can either wrap them into HAVE_... macros, or just
>>     rely on my_global.h to include them
>> 
>
> string.h is not included by my_global.h, but it is included by m_string.h
>

OK, though I don’t think you need to include the entire m_string.h just
to include string.h.

[...]

>>   - use my_micro_time() if clock_gettime() is not available? That gives
>>     us less OSX-specific code and more supported platforms at the cost
>>     of slightly less efficient code on OSX. Which is not a priority
>>     anyway.
>> 
>
> done

Hm, but now we have wrong values being returned by
sm_clock_time_duration() when my_micro_time() is used. I.e. all
timestamps are already in microseconds, we don’t have to divide them by
1000. Better yet, do division for the clock_gettime() case in
sm_clock_time_get() and then you won’t need sm_clock_time_duration() at all.

[...]

>
>>   - it looks like sm_thd_data_t::end is not really needed (can be an
>>     auto variable in sm_query_finished)
>> 
>
> indeed, it is

Thanks. But I wonder why ‘end’ and ‘save_busystart’ have also become volatile.

[...]

>>   - shouldn’t the check for (sm_ctl == CTL_ON) be done regardless of
>>     event_class? Otherwise we will be doing unnecessary bookkeeping for
>>     MYSQL_AUDIT_CONNECTION_CLASS events even if the plugin functionality
>>     is turned off
>> 
>
> This is needed because we don't want to miss disconnect events and forget to release memory. (see below)
>
>>   - shouldn’t setting sm_ctl to CTL_OFF also release all previously
>>     allocated memory and destroy thd_list?
>> 
>
> It will not work because we do not protect thd_data from being
> destroyed. We can introduce mutex for each thd_data, but shoul we?

Right, I see. No, mutex will not help us here. What we want to achieve
here is make getting a pointer from THDVAR and dereferencing it to get
its own mutex an atomic operation. Which is impossible without a global
mutex, which defeats the purpose.

But I see you have modified sm_ctl_update() to at least destroy thd_list.

Hm, this makes the pthread_*specific()/TLS approach even more
attractive. I certainly don’t like that we have some (probably minor,
but that’s just an assumption) overhead even when the plugin is
disabled. But let’s keep it as is for now.

>
>>   - I don’t quite understand the current logic wrt sm_reset() and
>>     query_series. So if we reset the stats, setting starttime to the
>>     current time looks correct. Setting concurrency to 0 does not (there
>>     may still be running queries). concurrency and busytime will get
>>     skewed as a result (which can also be observed in the test case
>>     results). Shouldn’t you also set busystart to the current
>>     time depending on the current concurrency value?
>> 
>
> the idea was that RESET will work like OFF followed by ON. So we should abandon all
> currently running queries.
>

OK, let me explain how I see it. There are two groups of SM stats:

- cumulative (queries, totaltime, busytime)
- transient (concurrency, elapsedtime)

The first one provides information about queries that have already been
completed by the time we query the corresponding stats, i.e. it describes
the past.

The second one describes the current state of the world.

Now, when we enable a previously disabled plugin, we obviously don’t
have any information, neither about the past, nor about the present,
because we are limited to notifications issued by the server to collect
both.

When we explicitly reset an active plugin, we obviously want to flush
cumulative data. But we do have a picture of the current state of the
world, don’t we? We can pretend that we don’t, i.e. simulate a newly
started plugin and provide inaccurate information on currently executing
queries, but that’s an artificial limitation IMHO.

From this POV, we have all information required to display the _current_
concurrency correctly. And busytime, even though it’s cumulative, is
closely related to concurrency.

In other words, I see no reasons to replicate the _drawbacks_ of OFF+ON
in RESET. What do you think?

>>   - query_series. I understand that it has been implemented to prevent
>>     queries that started before a reset from accounting. But why do we
>>     want to do that? Suppose we have 100 queries in progress at the time
>>     we issue a reset. Those queries will complete sometime later, but
>>     will not be reflected in the stats? This is also visible in the test
>>     case.
>> 
>
> Yes, they will not be reflected. Following the logic RESET=OFF+ON all the queries running at the "ON" moment will not be reflected, same done for RESET.
>

Yes, this is closely related to the question of what exactly should be
reset on RESET. Provided that my take on it looks reasonable to you, do you
think we can remove query series?

[...]

>
>>   - testcase: you can avoid “sleep 3” with the
>>     count_sessions/wait_until_count_sessions combo.
>
> wait_until_count_sessions will produce variable number of queries during its execution, which I am trying to avoid

Well, all those pushups with concurrent sessions and synchronization
points are basically required to test concurrency. There are many simpler ways to test queries, right?

-- 
Best regards,
Alexey.

Revision history for this message

Alexey Kopytov (akopytov) on 2014-01-07:

#

review: Needs Fixing

Revision history for this message

Sergei Glushchenko (sergei.glushchenko) wrote on 2014-02-10:

#

Hi Alexey,

I don't see how can thread-local storage be used without maintaining
per-thrad list similar to what I did for per-THD case since we cannot access
other thread's local variables and we cannot enumerate runnung threads.
The issue will remain the same for per-thread accounting.

changed in this MP:

Fixed author field for plugin.
Reverted include of string.h.
Removed volatile for 'end' and ‘save_busystart’.
Used my_getsystime because of better precision and fixed bug inroduced in prev MP.
Changed 'RESET' semantic.
Simplified test case.

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/951/

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2014-03-16:

#

Sergei,

The thing is that with thread-local storage you don't need to enumerate running threads. But, as I wrote, the current implementation is still acceptable. Approved.

review: Approve

Percona Server moved to https://jira.percona.com/projects/PS

Merge lp:~sergei.glushchenko/percona-server/5.5-ps-blueprint-scalability-metrics into lp:percona-server/5.5

Commit message

Description of the change

Preview Diff

Subscribers

 === added file 'mysql-test/include/have_scalability_metrics_plugin.inc'
 --- mysql-test/include/have_scalability_metrics_plugin.inc	1970-01-01 00:00:00 +0000
 +++ mysql-test/include/have_scalability_metrics_plugin.inc	2014-02-09 18:51:30 +0000
@@ -0,0 +1,21 @@
++#
++# Check if server has support for loading plugins
++#
++if (`SELECT @@have_dynamic_loading != 'YES'`) {
++  --skip Example plugin requires dynamic loading
++}
++
++#
++# Check if the variable SCALABILITY_METRICS is set
++#
++if (!$SCALABILITY_METRICS) {
++  --skip Example plugin requires the environment variable \$SCALABILITY_METRICS to be set (normally done by mtr)
++}
++
++#
++# Check if --plugin-dir was setup for exampledb
++#
++if (`SELECT CONCAT('--plugin-dir=', REPLACE(@@plugin_dir, '\\\\', '/')) != '$SCALABILITY_METRICS_OPT/'`) {
++  --skip Example plugin requires that --plugin-dir is set to the example plugin dir (either the .opt file does not contain \$SCALABILITY_METRICS_OPT or another plugin is in use)
++}
++enable_query_log;
 === modified file 'mysql-test/include/plugin.defs'
 --- mysql-test/include/plugin.defs	2012-10-31 07:10:48 +0000
 +++ mysql-test/include/plugin.defs	2014-02-09 18:51:30 +0000
@@ -41,3 +41,4 @@
  mypluglib          plugin/fulltext    SIMPLE_PARSER
  libdaemon_example  plugin/daemon_example DAEMONEXAMPLE
  adt_null           plugin/audit_null  AUDIT_NULL
++scalability_metrics plugin/scalability_metrics SCALABILITY_METRICS
 === added file 'mysql-test/r/scalability_metrics.result'
 --- mysql-test/r/scalability_metrics.result	1970-01-01 00:00:00 +0000
 +++ mysql-test/r/scalability_metrics.result	2014-02-09 18:51:30 +0000
@@ -0,0 +1,65 @@
++set debug_sync= 'RESET';
++INSTALL PLUGIN scalability_metrics SONAME 'scalability_metrics.so';
++SHOW STATUS LIKE 'scalability_metrics%';
++Variable_name	Value
++scalability_metrics_busytime	0
++scalability_metrics_concurrency	0
++scalability_metrics_elapsedtime	0
++scalability_metrics_queries	0
++scalability_metrics_totaltime	0
++SET GLOBAL scalability_metrics_control = ON;
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++Variable_name	Value
++scalability_metrics_concurrency	1
++SHOW STATUS LIKE 'scalability_metrics_queries';
++Variable_name	Value
++scalability_metrics_queries	1
++SET DEBUG_SYNC="func_sleep_before_sleep SIGNAL sleep1";
++SELECT SLEEP(100);
++SET DEBUG_SYNC= 'now WAIT_FOR sleep1';
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++Variable_name	Value
++scalability_metrics_concurrency	2
++SHOW STATUS LIKE 'scalability_metrics_queries';
++Variable_name	Value
++scalability_metrics_queries	7
++KILL QUERY @id;
++SLEEP(100)
++1
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++Variable_name	Value
++scalability_metrics_concurrency	1
++SHOW STATUS LIKE 'scalability_metrics_queries';
++Variable_name	Value
++scalability_metrics_queries	11
++SET DEBUG_SYNC="func_sleep_before_sleep SIGNAL sleep2";
++SELECT SLEEP(100);
++SET DEBUG_SYNC= 'now WAIT_FOR sleep2';
++SET GLOBAL scalability_metrics_control = RESET;
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++Variable_name	Value
++scalability_metrics_concurrency	2
++SHOW STATUS LIKE 'scalability_metrics_queries';
++Variable_name	Value
++scalability_metrics_queries	2
++KILL QUERY @id;
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++Variable_name	Value
++scalability_metrics_concurrency	1
++SHOW STATUS LIKE 'scalability_metrics_queries';
++Variable_name	Value
++scalability_metrics_queries	6
++SLEEP(100)
++1
++SET GLOBAL scalability_metrics_control = OFF;
++SHOW STATUS LIKE 'scalability_metrics%';
++Variable_name	Value
++scalability_metrics_busytime	0
++scalability_metrics_concurrency	1
++scalability_metrics_elapsedtime	0
++scalability_metrics_queries	0
++scalability_metrics_totaltime	0
++UNINSTALL PLUGIN scalability_metrics;
++Warnings:
++Warning	1620	Plugin is busy and will be uninstalled on shutdown
++set debug_sync= 'RESET';
 === added file 'mysql-test/t/scalability_metrics-master.opt'
 --- mysql-test/t/scalability_metrics-master.opt	1970-01-01 00:00:00 +0000
 +++ mysql-test/t/scalability_metrics-master.opt	2014-02-09 18:51:30 +0000
@@ -0,0 +1,1 @@
++$SCALABILITY_METRICS_OPT
 === added file 'mysql-test/t/scalability_metrics.test'
 --- mysql-test/t/scalability_metrics.test	1970-01-01 00:00:00 +0000
 +++ mysql-test/t/scalability_metrics.test	2014-02-09 18:51:30 +0000
@@ -0,0 +1,81 @@
++################################################################################
++# Test for plugin scalability_metrics
++################################################################################
++
++--source include/have_scalability_metrics_plugin.inc
++--source include/have_debug_sync.inc
++
++set debug_sync= 'RESET';
++
++INSTALL PLUGIN scalability_metrics SONAME 'scalability_metrics.so';
++
++SHOW STATUS LIKE 'scalability_metrics%';
++SET GLOBAL scalability_metrics_control = ON;
++
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++SHOW STATUS LIKE 'scalability_metrics_queries';
++
++connect (con1,localhost,root,,);
++
++let $ID= `SELECT @id := CONNECTION_ID()`;
++
++SET DEBUG_SYNC="func_sleep_before_sleep SIGNAL sleep1";
++
++send SELECT SLEEP(100);
++
++connection default;
++
++SET DEBUG_SYNC= 'now WAIT_FOR sleep1';
++
++let $ignore= `SELECT @id := $ID`;
++
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++SHOW STATUS LIKE 'scalability_metrics_queries';
++
++KILL QUERY @id;
++
++connection default;
++
++connection con1;
++reap;
++
++sleep 3;
++
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++SHOW STATUS LIKE 'scalability_metrics_queries';
++
++SET DEBUG_SYNC="func_sleep_before_sleep SIGNAL sleep2";
++
++send SELECT SLEEP(100);
++
++connection default;
++
++SET DEBUG_SYNC= 'now WAIT_FOR sleep2';
++
++let $ignore= `SELECT @id := $ID`;
++
++SET GLOBAL scalability_metrics_control = RESET;
++
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++SHOW STATUS LIKE 'scalability_metrics_queries';
++
++KILL QUERY @id;
++
++SHOW STATUS LIKE 'scalability_metrics_concurrency';
++SHOW STATUS LIKE 'scalability_metrics_queries';
++
++connection con1;
++reap;
++
++sleep 3;
++
++SET GLOBAL scalability_metrics_control = OFF;
++SHOW STATUS LIKE 'scalability_metrics%';
++
++disconnect con1;
++
++connection default;
++
++UNINSTALL PLUGIN scalability_metrics;
++
++set debug_sync= 'RESET';
 === added directory 'plugin/scalability_metrics'
 === added file 'plugin/scalability_metrics/CMakeLists.txt'
 --- plugin/scalability_metrics/CMakeLists.txt	1970-01-01 00:00:00 +0000
 +++ plugin/scalability_metrics/CMakeLists.txt	2014-02-09 18:51:30 +0000
@@ -0,0 +1,17 @@
++# Copyright (c) 2014 Percona LLC and/or its affiliates. All rights reserved.
++#
++# This program is free software; you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation; version 2 of the License.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program; if not, write to the Free Software
++# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
++
++MYSQL_ADD_PLUGIN(scalability_metrics scalability_metrics.c
++ MODULE_ONLY MODULE_OUTPUT_NAME "scalability_metrics")
 === added file 'plugin/scalability_metrics/scalability_metrics.c'
 --- plugin/scalability_metrics/scalability_metrics.c	1970-01-01 00:00:00 +0000
 +++ plugin/scalability_metrics/scalability_metrics.c	2014-02-09 18:51:30 +0000
@@ -0,0 +1,451 @@
++/* Copyright (c) 2014 Percona LLC and/or its affiliates. All rights reserved.
++
++   This program is free software; you can redistribute it and/or
++   modify it under the terms of the GNU General Public License
++   as published by the Free Software Foundation; version 2 of
++   the License.
++
++   This program is distributed in the hope that it will be useful,
++   but WITHOUT ANY WARRANTY; without even the implied warranty of
++   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
++   GNU General Public License for more details.
++
++   You should have received a copy of the GNU General Public License
++   along with this program; if not, write to the Free Software
++   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA */
++
++#include <mysql/plugin.h>
++#include <mysql/plugin_audit.h>
++#include <my_global.h>
++#include <my_sys.h>
++#include <my_list.h>
++#include <my_pthread.h>
++#include <typelib.h>
++#include <limits.h>
++#include <string.h>
++
++static volatile ulonglong starttime= 0;
++static volatile ulonglong concurrency= 0;
++static volatile ulonglong busystart= 0;
++static volatile ulonglong busytime= 0;
++static volatile ulonglong totaltime= 0;
++static volatile ulonglong queries= 0;
++
++#ifdef HAVE_PSI_INTERFACE
++PSI_mutex_key key_thd_list_mutex;
++#endif
++mysql_mutex_t thd_list_mutex;
++
++LIST *thd_list_root= NULL;
++
++typedef struct sm_thd_data_struct {
++  ulonglong start;
++  ulonglong duration;
++  ulonglong queries;
++  LIST *backref;
++} sm_thd_data_t;
++
++typedef enum { CTL_ON= 0, CTL_OFF= 1, CTL_RESET= 2 } sm_ctl_t;
++static const char* sm_ctl_names[]= { "ON", "OFF", "RESET", NullS };
++static TYPELIB sm_ctl_typelib= {
++  array_elements(sm_ctl_names) - 1,
++  "",
++  sm_ctl_names,
++  NULL
++};
++
++static
++void sm_ctl_update(MYSQL_THD thd, struct st_mysql_sys_var *var,
++                   void *var_ptr, const void *save);
++
++static ulong sm_ctl= CTL_OFF;
++
++static
++MYSQL_THDVAR_ULONGLONG(thd_data,
++  PLUGIN_VAR_READONLY | PLUGIN_VAR_NOSYSVAR | PLUGIN_VAR_NOCMDOPT,
++  "scalability metrics data", NULL, NULL, 0, 0, ULONGLONG_MAX, 0);
++
++
++static MYSQL_SYSVAR_ENUM(
++  control,                           /* name */
++  sm_ctl,                            /* var */
++  PLUGIN_VAR_RQCMDARG,
++  "Control the scalability metrics. Use this to turn ON/OFF or RESET metrics.",
++  NULL,                              /* check func. */
++  sm_ctl_update,                     /* update func. */
++  CTL_OFF,                           /* default */
++  &sm_ctl_typelib                    /* typelib */
++);
++
++static
++ulonglong sm_clock_time_get()
++{
++#if (defined HAVE_CLOCK_GETTIME)
++  struct timespec ts;
++#ifdef CLOCK_MONOTONIC_RAW
++  clock_gettime(CLOCK_MONOTONIC_RAW, &ts);
++#else
++  clock_gettime(CLOCK_MONOTONIC, &ts);
++#endif
++  return((ulonglong) ts.tv_sec * 1000000000 + ts.tv_nsec);
++#else
++  /* since output values measured in microseconds anyway,
++  100 nanoseconds precision should be enough here */
++  return(my_getsystime() * 100);
++#endif
++}
++
++
++/* Get duration in microseconds */
++static
++ulonglong sm_clock_time_duration(ulonglong beg, ulonglong end)
++{
++  return((end - beg) / 1000);
++}
++
++static
++sm_thd_data_t *sm_thd_data_get(MYSQL_THD thd)
++{
++  sm_thd_data_t *thd_data = (sm_thd_data_t *) (intptr) THDVAR(thd, thd_data);
++  if (unlikely(thd_data == NULL))
++  {
++    thd_data= calloc(sizeof(sm_thd_data_t), 1);
++    mysql_mutex_lock(&thd_list_mutex);
++    thd_data->backref= list_push(thd_list_root, thd_data);
++    mysql_mutex_unlock(&thd_list_mutex);
++    THDVAR(thd, thd_data)= (ulonglong) (intptr) thd_data;
++  }
++  return thd_data;
++}
++
++
++static
++void sm_thd_data_release(MYSQL_THD thd)
++{
++  sm_thd_data_t *thd_data = (sm_thd_data_t *) (intptr) THDVAR(thd, thd_data);
++  if (likely(thd_data != NULL && thd_data->backref != NULL))
++  {
++    (void) __sync_add_and_fetch(&queries, thd_data->queries);
++    (void) __sync_add_and_fetch(&totaltime, thd_data->duration);
++    mysql_mutex_lock(&thd_list_mutex);
++    thd_list_root= list_delete(thd_list_root, thd_data->backref);
++    mysql_mutex_unlock(&thd_list_mutex);
++    free(thd_data->backref);
++    free(thd_data);
++    THDVAR(thd, thd_data)= 0;
++  }
++}
++
++static
++int sm_reset_one(void *data, void *argument)
++{
++  sm_thd_data_t *thd_data= (sm_thd_data_t *) data;
++  thd_data->queries= 0;
++  thd_data->duration= 0;
++  return(0);
++}
++
++static
++void sm_reset()
++{
++  starttime= sm_clock_time_get();
++  busytime= totaltime= queries= 0;
++  mysql_mutex_lock(&thd_list_mutex);
++  list_walk(thd_list_root, sm_reset_one, NULL);
++  mysql_mutex_unlock(&thd_list_mutex);
++}
++
++
++static
++void sm_ctl_update(MYSQL_THD thd __attribute__((unused)),
++                   struct st_mysql_sys_var *var __attribute__((unused)),
++                   void *var_ptr __attribute__((unused)),
++                   const void *save) {
++  ulong new_val= *((sm_ctl_t*) save);
++
++  if (new_val != sm_ctl)
++    sm_reset();
++
++  if (new_val != CTL_RESET)
++  {
++    sm_ctl= new_val;
++
++    if (new_val == CTL_OFF)
++    {
++      mysql_mutex_lock(&thd_list_mutex);
++      list_free(thd_list_root, FALSE);
++      thd_list_root= NULL;
++      mysql_mutex_unlock(&thd_list_mutex);
++    }
++  }
++
++}
++
++
++static
++int sm_plugin_init(void *arg __attribute__((unused)))
++{
++  mysql_mutex_init(key_thd_list_mutex, &thd_list_mutex, MY_MUTEX_INIT_FAST);
++
++  sm_reset();
++
++  return(0);
++}
++
++
++static
++int sm_plugin_deinit(void *arg __attribute__((unused)))
++{
++  list_free(thd_list_root, FALSE);
++  thd_list_root= NULL;
++
++  mysql_mutex_destroy(&thd_list_mutex);
++
++  return(0);
++}
++
++static
++void sm_query_started(MYSQL_THD thd,
++                      const char* query __attribute__((unused))) {
++
++  sm_thd_data_t *thd_data= sm_thd_data_get(thd);
++
++  if (__sync_bool_compare_and_swap(&concurrency, 0, 1))
++  {
++    thd_data->start= sm_clock_time_get();
++    busystart= thd_data->start;
++  }
++  else
++  {
++    thd_data->start= sm_clock_time_get();
++    (void) __sync_add_and_fetch(&concurrency, 1);
++  }
++}
++
++static
++void sm_query_finished(MYSQL_THD thd,
++                       const char* query __attribute__((unused))) {
++
++  sm_thd_data_t *thd_data= sm_thd_data_get(thd);
++  ulonglong end, save_busystart;
++
++  if (thd_data->start != 0)
++  {
++    save_busystart= busystart;
++    if (__sync_sub_and_fetch(&concurrency, 1) == 0)
++    {
++      end= sm_clock_time_get();
++      (void) __sync_add_and_fetch(&busytime,
++                           sm_clock_time_duration(save_busystart, end));
++    }
++    else
++    {
++      end= sm_clock_time_get();
++    }
++
++    thd_data->duration+= sm_clock_time_duration(thd_data->start, end);
++    thd_data->queries++;
++  }
++}
++
++static
++void sm_query_failed(MYSQL_THD thd,
++                     const char* query,
++                     int err __attribute__((unused))) {
++
++  /* currently there is no difference between success and failure */
++
++  sm_query_finished(thd, query);
++
++}
++
++
++static
++int sm_elapsedtime(MYSQL_THD thd __attribute__((unused)),
++                   struct st_mysql_show_var* var,
++                   char *buff)
++{
++  *((ulonglong*)buff)= (sm_ctl == CTL_ON) ?
++                      sm_clock_time_duration(starttime, sm_clock_time_get()) :
++                      0;
++  var->type= SHOW_LONGLONG;
++  var->value= buff;
++  return(0);
++}
++
++
++static
++int sm_sum_queries(void *data, void *argument)
++{
++  sm_thd_data_t *thd_data= (sm_thd_data_t *) data;
++  *((ulonglong *) argument)+= thd_data->queries;
++  return(0);
++}
++
++
++static
++int sm_queries(MYSQL_THD thd __attribute__((unused)),
++               struct st_mysql_show_var* var,
++               char *buff)
++{
++  ulonglong sum_queries= 0;
++
++  if (sm_ctl == CTL_ON)
++  {
++    mysql_mutex_lock(&thd_list_mutex);
++    list_walk(thd_list_root, sm_sum_queries, (unsigned char *) &sum_queries);
++    mysql_mutex_unlock(&thd_list_mutex);
++  }
++  *((ulonglong *) buff)= queries + sum_queries;
++  var->type= SHOW_LONGLONG;
++  var->value= buff;
++  return(0);
++}
++
++
++static
++int sm_sum_totaltime(void *data, void *argument)
++{
++  sm_thd_data_t *thd_data= (sm_thd_data_t *) data;
++  *((ulonglong *) argument)+= thd_data->duration;
++  return(0);
++}
++
++
++static
++int sm_totaltime(MYSQL_THD thd __attribute__((unused)),
++               struct st_mysql_show_var* var,
++               char *buff)
++{
++  ulonglong sum_totaltime= 0;
++
++  if (sm_ctl == CTL_ON)
++  {
++    mysql_mutex_lock(&thd_list_mutex);
++    list_walk(thd_list_root, sm_sum_totaltime,
++              (unsigned char *) &sum_totaltime);
++    mysql_mutex_unlock(&thd_list_mutex);
++  }
++  *((ulonglong *) buff)= totaltime + sum_totaltime;
++  var->type= SHOW_LONGLONG;
++  var->value= buff;
++  return(0);
++}
++
++
++static void sm_notify(MYSQL_THD thd, unsigned int event_class,
++                      const void *event)
++{
++
++  if (event_class == MYSQL_AUDIT_GENERAL_CLASS)
++  {
++    const struct mysql_event_general *event_general=
++      (const struct mysql_event_general *) event;
++
++    if (sm_ctl != CTL_ON)
++    {
++      return;
++    }
++
++    if (event_general->general_command &&
++        event_general->event_subclass == MYSQL_AUDIT_GENERAL_LOG &&
++        strcmp(event_general->general_command, "Query") == 0)
++    {
++      sm_query_started(thd, event_general->general_query);
++    }
++    else if (event_general->general_command &&
++        event_general->event_subclass == MYSQL_AUDIT_GENERAL_LOG &&
++        strcmp(event_general->general_command, "Execute") == 0)
++    {
++      sm_query_started(thd, event_general->general_query);
++    }
++    else if (event_general->general_query &&
++        event_general->event_subclass == MYSQL_AUDIT_GENERAL_RESULT)
++    {
++      sm_query_finished(thd, event_general->general_query);
++    }
++    else if (event_general->general_query &&
++        event_general->event_subclass == MYSQL_AUDIT_GENERAL_ERROR)
++    {
++      sm_query_failed(thd, event_general->general_query,
++                                event_general->general_error_code);
++    }
++
++  }
++  else if (event_class == MYSQL_AUDIT_CONNECTION_CLASS)
++  {
++    const struct mysql_event_connection *event_connection=
++      (const struct mysql_event_connection *) event;
++    switch (event_connection->event_subclass)
++    {
++    case MYSQL_AUDIT_CONNECTION_CONNECT:
++      sm_thd_data_get(thd);
++      break;
++    case MYSQL_AUDIT_CONNECTION_DISCONNECT:
++      sm_thd_data_release(thd);
++      break;
++    default:
++      break;
++    }
++  }
++}
++
++/*
++ * Plugin system vars
++ */
++static struct st_mysql_sys_var* scalability_metrics_system_variables[] =
++{
++  MYSQL_SYSVAR(thd_data),
++  MYSQL_SYSVAR(control),
++  NULL
++};
++
++/*
++  Plugin type-specific descriptor
++*/
++static struct st_mysql_audit scalability_metrics_descriptor=
++{
++  MYSQL_AUDIT_INTERFACE_VERSION,                    /* interface version    */
++  NULL,                                             /* release_thd function */
++  sm_notify,                                        /* notify function      */
++  { MYSQL_AUDIT_GENERAL_CLASSMASK |
++    MYSQL_AUDIT_CONNECTION_CLASSMASK }              /* class mask           */
++};
++
++/*
++  Plugin status variables for SHOW STATUS
++*/
++
++static struct st_mysql_show_var simple_status[]=
++{
++  { "scalability_metrics_elapsedtime", (char *) &sm_elapsedtime, SHOW_FUNC },
++  { "scalability_metrics_queries", (char *) &sm_queries, SHOW_FUNC },
++  { "scalability_metrics_concurrency", (char *) &concurrency, SHOW_LONGLONG },
++  { "scalability_metrics_totaltime", (char *) &sm_totaltime, SHOW_FUNC },
++  { "scalability_metrics_busytime", (char *) &busytime, SHOW_LONGLONG },
++  { 0, 0, 0}
++};
++
++
++/*
++  Plugin library descriptor
++*/
++
++mysql_declare_plugin(scalability_metrics)
++{
++  MYSQL_AUDIT_PLUGIN,                     /* type                            */
++  &scalability_metrics_descriptor,        /* descriptor                      */
++  "scalability_metrics",                  /* name                            */
++  "Percona LLC and/or its affiliates",    /* author                          */
++  "Scalability metrics",                  /* description                     */
++  PLUGIN_LICENSE_GPL,
++  sm_plugin_init,                         /* init function (when loaded)     */
++  sm_plugin_deinit,                       /* deinit function (when unloaded) */
++  0x0001,                                 /* version                         */
++  simple_status,                          /* status variables                */
++  scalability_metrics_system_variables,   /* system variables                */
++  NULL,
++  0,
++}
++mysql_declare_plugin_end;
++
 === modified file 'sql/item_func.cc'
 --- sql/item_func.cc	2013-06-27 15:35:20 +0000
 +++ sql/item_func.cc	2014-02-09 18:51:30 +0000
@@ -4135,6 +4135,8 @@
    thd->mysys_var->current_mutex= &LOCK_user_locks;
    thd->mysys_var->current_cond=  &cond;
++  DEBUG_SYNC(current_thd, "func_sleep_before_sleep");
++
    error= 0;
    thd_wait_begin(thd, THD_WAIT_SLEEP);
    while (!thd->killed)