Bug #40760 "set global innodb_thread_concurrency = 0;" is not safe

Description:
During transactions are running on the other sessions,
"set global innodb_thread_concurrency = 0;" from the other value may cause error log
at the close of the session.

And at that time "set global innodb_thread_concurrency = 1;" can halt InnoDB.

How to repeat:
session-1> set global innodb_thread_concurrency = 8; /* !=0 value */

session-2> (execute long query using InnoDB table)

(during the query of session-2)
session-1> set global innodb_thread_concurrency = 0;

session-2> exit

===(The message is outputted to .err file)========================
081116  3:10:21  InnoDB: Error: Freeing a trx which is declared to be processing
InnoDB: inside InnoDB.
TRANSACTION 0 2567, not started, process no 4287, OS thread id 1141483840, thread declared inside InnoDB 53
MySQL thread id 7, query id 44 localhost root
==================================================================

In addition, at that time,

> set global innodb_thread_concurrency = 1;

Then nobody can use InnoDB tables....

Thank you for a problem report. Sorry, but I was not able to repeat the behaviour described neither on 5.0.70 nor on latest 5.0.74 from bzr with only two sessions whith one of them doing "select count(*) from big_innodb_table" or "update big_innodb_table set ...".

So, please, provide a more detailed test case. Do I need to use multi-core system to repeat this?

Valeriy,

I think any long query is OK. And it don't need SMP.

My test case is intended to change srv_thread_concurrency between these functions at ha_innodb.cc

/**********************************************************************
Save some CPU by testing the value of srv_thread_concurrency in inline
functions. */
inline
void
innodb_srv_conc_enter_innodb(
/*=========================*/
        trx_t*  trx)    /* in: transaction handle */
{
        if (UNIV_LIKELY(!srv_thread_concurrency)) {  /* intended value == 8 */

                return;
        }

        srv_conc_enter_innodb(trx);
}

/**********************************************************************
Save some CPU by testing the value of srv_thread_concurrency in inline
functions. */
inline
void
innodb_srv_conc_exit_innodb(
/*========================*/
        trx_t*  trx)    /* in: transaction handle */
{
        if (UNIV_LIKELY(!srv_thread_concurrency)) { /* intended value == 0 */

                return;
        }

        srv_conc_exit_innodb(trx);
}

In addition, the another test case to halt is that
when benchmark (like DBT-2) is running, repeat to switch
"set global innodb_thread_concurrency = 0;" and
"set global innodb_thread_concurrency = 8;"(smaller value than the sessions of benchmark) many times.

Verified with 5.0.70 on our sol10-sparc-c based on last comment. I had started 10 long running transactions (INSERT ... SELECT, UPDATE, DELETE) with innodb_thread_concurrency=8 and then, from the other session, did:

set global innodb_thread_concurrency=0;
set global innodb_thread_concurrency=1;
set global innodb_thread_concurrency=4;

dozen of times. I've got in the error log eventually:

Version: '5.0.70-enterprise-gpl-log'  socket: '/tmp/vk.sock'  port: 4444  MySQL Enterprise Server (GPL)
081116 12:18:30  InnoDB: Error: Freeing a trx which is declared to be processing
InnoDB: inside InnoDB.
TRANSACTION 0 23807000, not started, OS thread id 56, thread declared inside InnoDB 49
MySQL thread id 46, query id 373 localhost root

Thank you Yasufumi. You again found a bug concerned with races.

In very old versions of InnoDB, one could not dynamically adjust thread concurrency, and the bug was not present.

Hmm... a fix might be that in the two functions below, we test if trx->declared_to_be_inside_innodb is TRUE. If that is the case, we must do the .._exit() regardless of the value of srv_thread_concurrency.

That is, in the first function below, replace:

"        if (UNIV_LIKELY(!srv_thread_concurrency)) {"

with:

"        if (UNIV_LIKELY(!srv_thread_concurrency && !trx->declared_to_be_inside_innodb )) {"

In the second function, do a similar replacement.

This solves the problem of srv_thread_concurrency > 0 at a thread ENTER and srv_thread_concurrency == 0 at a thread EXIT.

What about the inverse problem? I think that problem is already solved in the code of the second function by:

"        if (trx->declared_to_be_inside_innodb == FALSE) {

                return;
        }
"

/**********************************************************************
Save some CPU by testing the value of srv_thread_concurrency in inline
functions. */
inline
void
innodb_srv_conc_exit_innodb(
/*========================*/
        trx_t*  trx)    /* in: transaction handle */
{
        if (UNIV_LIKELY(!srv_thread_concurrency)) {

                return;
        }

        srv_conc_exit_innodb(trx);
}

/*************************************************************************
This must be called when a thread exits InnoDB in a lock wait or at the
end of an SQL statement. */

void
srv_conc_force_exit_innodb(
/*=======================*/
        trx_t*  trx)    /* in: transaction object associated with the
                        thread */
{
        srv_conc_slot_t*        slot    = NULL;

        if (UNIV_LIKELY(!srv_thread_concurrency)) {

                return;
        }

        if (trx->mysql_thd != NULL
            && thd_is_replication_slave_thread(trx->mysql_thd)) {

                return;
        }

        if (trx->declared_to_be_inside_innodb == FALSE) {

                return;
        }

Assigning this to Inaam, who is our race condition man.

This comment in srv0srv.c is wrong:
"
lint    srv_conc_n_threads      = 0;    /* number of OS threads currently
                                        inside InnoDB; it is not an error
                                        if this drops temporarily below zero
                                        because we do not demand that every
                                        thread increments this, but a thread
                                        waiting for a lock decrements this
                                        temporarily */
"

In the current code, it can never become negative!

There are invariants, which should be stated in the comment:

(1) srv_conc_n_threads is the number of threads that have trx->declared_to_be_inside_innodb == TRUE;
(2) trx->declared_to_be_inside_innodb will eventually become FALSE for every thread.

Noted in 5.1.32, 6.0.10 changelog.

Changing innodb_thread_concurrency at runtime could cause errors.

It would be nice to get a fix for this in 5.0

Pushed into 5.1.47 (revid:joro@sun.com-20100505145753-ivlt4hclbrjy8eye) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)

Push resulted from incorporation of InnoDB tree. No changes pertinent to this bug.
Re-closing.

Pushed into mysql-next-mr (revid:alik@sun.com-20100524190136-egaq7e8zgkwb9aqi) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (pib:16)

Pushed into 6.0.14-alpha (revid:alik@sun.com-20100524190941-nuudpx60if25wsvx) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)

Pushed into 5.5.5-m3 (revid:alik@sun.com-20100524185725-c8k5q7v60i5nix3t) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)

Push resulted from incorporation of InnoDB tree. No changes pertinent to this bug.
Re-closing.

Pushed into 5.5.5-m3 (revid:alik@sun.com-20100615080459-smuswd9ooeywcxuc) (version source revid:mmakela@bk-internal.mysql.com-20100415070122-1nxji8ym4mao13ao) (merge vers: 5.1.47) (pib:16)

Pushed into mysql-next-mr (revid:alik@sun.com-20100615080558-cw01bzdqr1bdmmec) (version source revid:mmakela@bk-internal.mysql.com-20100415070122-1nxji8ym4mao13ao) (pib:16)

Pushed into 5.1.47-ndb-7.0.16 (revid:martin.skold@mysql.com-20100617114014-bva0dy24yyd67697) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)

Pushed into 5.1.47-ndb-6.2.19 (revid:martin.skold@mysql.com-20100617115448-idrbic6gbki37h1c) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)

Pushed into 5.1.47-ndb-6.3.35 (revid:martin.skold@mysql.com-20100617114611-61aqbb52j752y116) (version source revid:vasil.dimov@oracle.com-20100331130613-8ja7n0vh36a80457) (merge vers: 5.1.46) (pib:16)