Bug #76728 reduce lock_sys->mutex contention for transaction that only contains SELECT
Submitted: 17 Apr 2015 4:48 Modified: 3 Jun 2016 19:58
Reporter: zhai weixiang (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.7.7 OS:Any
Assigned to: CPU Architecture:Any

[17 Apr 2015 4:48] zhai weixiang
Description:
While testing the read-only performance of MySQL 5.7.7-RC, I found a lot of threads stall in such backtrace:

23 pthread_cond_wait,os_cond_wait(os0sync.cc:214),os_event_wait_low(os0sync.cc:214),sync_array_wait_event(sync0arr.cc:424),mutex_spin_wait(sync0sync.cc:579),mutex_enter_func(sync0sync.ic:220),pfs_mutex_enter_func(sync0sync.ic:220),lock_trx_release_locks(sync0sync.ic:220),trx_commit_in_memory(trx0trx.cc:1381),trx_commit_low(trx0trx.cc:1381),trx_commit(trx0trx.cc:1609),trx_commit_for_mysql(trx0trx.cc:1836),innobase_commit_low(ha_innodb.cc:3447),innobase_commit(ha_innodb.cc:3447),ha_commit_low(handler.cc:1514),MYSQL_BIN_LOG::commit(binlog.cc:6327),ha_commit_trans(handler.cc:1457),trans_commit(transaction.cc:228),mysql_execute_command(sql_parse.cc:4558),mysql_parse(sql_parse.cc:6620),dispatch_command(sql_parse.cc:1445),do_handle_one_connection(sql_connect.cc:1000),handle_one_connection(sql_connect.cc:916),pfs_spawn_thread(pfs.cc:1858),start_thread(libpthread.so.0),clone(libc.so.6)

From performance schema, lock_sys mutex is the hottest one !!!

mysql> SELECT COUNT_STAR, SUM_TIMER_WAIT, AVG_TIMER_WAIT, EVENT_NAME FROM events_waits_summary_global_by_event_name where COUNT_STAR > 0 and EVENT_NAME like 'wait/synch/%' order by SUM_TIMER_WAIT desc limit 10;
+------------+-----------------+----------------+---------------------------------------------+
| COUNT_STAR | SUM_TIMER_WAIT | AVG_TIMER_WAIT | EVENT_NAME |
+------------+-----------------+----------------+---------------------------------------------+
| 17739300 | 172218176088930 | 9707895 | wait/synch/mutex/innodb/lock_mutex |
| 35479372 | 77340476989560 | 2179785 | wait/synch/mutex/innodb/trx_sys_mutex |
| 35465620 | 27221504947890 | 767340 | wait/synch/mutex/sql/LOCK_table_cache |
| 159575929 | 20214954245040 | 126585 | wait/synch/mutex/sql/THD::LOCK_query_plan |
| 195132740 | 14580625066155 | 74385 | wait/synch/mutex/innodb/trx_mutex |
| 88650706 | 12744866651745 | 143550 | wait/synch/mutex/sql/THD::LOCK_thd_data |
| 53706864 | 9759830008440 | 181395 | wait/synch/sxlock/innodb/hash_table_locks |
| 106379778 | 9373681754280 | 87870 | wait/synch/mutex/sql/THD::LOCK_thd_query |
| 17741460 | 4780200354420 | 269265 | wait/synch/rwlock/sql/LOCK_grant |
| 17741647 | 3150520526940 | 177480 | wait/synch/sxlock/innodb/index_tree_rw_lock |
+------------+-----------------+----------------+---------------------------------------------+
10 rows in set (0.18 sec)

My Workload is very simple:

BEGIN; SELECT  where PK= ?; SELECT ...Where pk = ?; COMMIT; 

tx_isolation:  READ-COMMIT

If the transaction only contains SELECT statement and doesn’t change anything, I think we can simply skip acquiring lock_sys->mutex in function lock_trx_release_locks

How to repeat:
Read the code , or run a similar workload.

Suggested fix:
Not sure if this is a safe patch .. Just for verifying :)
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 2265cd8..3f2a467 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -6632,6 +6632,19 @@ lock_trx_release_locks(
                ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE));
        }

+       /* If the transaction didn't change anything, skip acquiring
+       lock sys mutex. */
+       if ((trx->lock.n_rec_locks == 0)
+           && (trx->lock.table_locks.size() == 0)
+           && (trx->rsegs.m_redo.rseg == 0)) {
+               ut_a(!trx_is_referenced(trx));
+               trx_mutex_enter(trx);
+               trx->state = TRX_STATE_COMMITTED_IN_MEMORY;
+               trx->is_recovered = false;
+               trx_mutex_exit(trx);
+               return;
+       }
+
        /* The transition of trx->state to TRX_STATE_COMMITTED_IN_MEMORY
        is protected by both the lock_sys->mutex and the trx->mutex. */
        lock_mutex_enter();
[27 Jan 2016 17:13] MySQL Verification Team
Hi Zhai,

Thank you for your bug report. Unfortunately, this is not a bug.

In spite of the fact that only read queries are utilized, many mutexes still have to be locked. First of all, some of your queries might not be in auto-commit mode or might have locking used due to the choice of the isolation level used. In that case, many locks are used.

Next, regardless of the queries being read-only, bin-logging might be turned on and that would carry couple of other mutex locking / unlocking.

Even if you do not need anything of this, a program might access some global data, just for reading, and during those reads, no writes could be possible permitted !!!! How is that achieved ?? By mutex locking and unlocking. Next, thread-local data are definitely changed and again we have locking / unlocking. A SELECT query resolution might need the utilization of temporary tables(s), whose creation and removal again require locking / unlocking.

If you only read from InnoDB table, you have to lock a specific mutex, which would prevent that the same set of pages is written over by some other connection. You have other mutexes here. Like using adaptive hash index, must also prevent even partial writes of it until reading is over. If the query utilizes indices, then each index tree used must not be written to, until reading is over.

This can of course depend on isolation level.

There are other examples, but I think this is enough ....
[28 Jan 2016 0:00] zhai weixiang
hI, Sinisa

I guess what  you want to point out is read-only transaction could hold transaction locks too . But the diff will check this condition to reduce lock requirement of lock_sys->mutex.

if ((trx->lock.n_rec_locks == 0)    -----  doesn't hold any record locks 
+           && (trx->lock.table_locks.size() == 0)  ---- doesn't hold any table locks
+           && (trx->rsegs.m_redo.rseg == 0)) {

only these conditions be satisfied, we will skip requiring lock_sys->mutex and return directly from function lock_trx_release_locks
[28 Jan 2016 14:40] MySQL Verification Team
Hi!

Actually, your patch, as is, can not be applied, because read-only queries might require temporary table(s) in order to be resolved. And temporary table require locks for the undo logs.

However, it is possible to add additional conditions, under which no locks would be used. This would be  a small improvement that we can work on.

Hence, this bug is now verified.
[3 Jun 2016 19:58] Daniel Price
Posted by developer:
 
Fixed as of the upcoming 5.7.14 release, and here's the changelog entry:

In READ COMMITTED isolation level, InnoDB unnecessarily acquired the
lock_sys mutex at COMMIT for a transaction block consisting of read-only
SELECT statements. 

Thanks to Zhai Weixiang for the patch.