Bug #89115 Unnecessary to hold trx mutex while updating trx_t::n_ref
Submitted: 5 Jan 2018 10:05 Modified: 15 Jan 2018 3:23
Reporter: zhai weixiang (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S4 (Feature request)
Version:MySQL8.0 OS:Any
Assigned to: CPU Architecture:Any

[5 Jan 2018 10:05] zhai weixiang
Description:
In function trx_rw_is_active(), it will 
1. acquire trx_sys->mutex 
2. found trx_t from trx_sys->rw_trx_set
3. acquire trx_t::mutex
4. check if the trx is commted in memory, if not, increase the trx_t::n_ref
5. release trx_t::mutex
6. release trx_sys->mutex
7. return trx_t

While committing a transaction in trx_commit_in_memory, it will:
1. acquire trx_sys->mutex
2. remove trx_t from rw_trx_set and rw_trx_list, etc...
3. release trx_sys->mutex
4. change trx_t::state to TRX_STATE_COMMITTED_IN_MEMORY
5. check if the trx_t::n_ref equals to zero, if not, loop and wait.
6. release locks,etc...

1~3 in function trx_erase_lists(), 4~6 in function lock_trx_release_locks

So obvisouly, if the trx_t is in trx_sys->rw_trx_set, it's state shouldn't be TRX_STATE_COMMITTED_IN_MEMORY. which means if we found a trx_t from the set in function trx_rw_is_active, it shouldn't be committed, and it doesn't need to acquire trx_t::mutex and check trx_t::state

I didn't run test but just get the conclusion by reading code. Let me know if I was wrong :) 

How to repeat:
Read the code 

Suggested fix:
use std::atomic to define trx_t::n_ref, and dont' acquire trx_t::mutex while operationg the counter.
[12 Jan 2018 13:32] MySQL Verification Team
Hi!

Thank you for your bug report. Your analysis is oversimplified, but, it is also relatively correct.

The only bad consequence of this design is that several of the  mutex locks are overused, which increases contention and that is not what we want.

Hence, this makes a good feature request and I am verifying it as such.
[12 Jan 2018 17:30] MySQL Verification Team
Hi!

It turns out that trx_sys contention is known, but we have not seen a contention on the trx_t mutex. Can you underline scenario for that contention, please .......
[15 Jan 2018 3:23] zhai weixiang
Hi, Sinisa

Thank you for verifying this bug. I didn't see hot mutex contnetion on trx_t::mutex while running benchmark. I found this issue while investigating bug #89127