Bug #85394 Some confusing code in function row_vers_build_for_semi_consistent_read
Submitted: 10 Mar 2017 1:59 Modified: 10 Mar 2017 17:54
Reporter: zhai weixiang (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.7.17 OS:Any
Assigned to: CPU Architecture:Any

[10 Mar 2017 1:59] zhai weixiang
Description:
In function row_vers_build_for_semi_consistent_read, it'll extract  trx_id from primary record, and then check if the corresponding transaction is active or not

quoted code from function row_vers_build_for_semi_consistent_read(5.7.17):

                trx_sys_mutex_enter();
                version_trx = (version_trx_id);
                /* Because version_trx is a read-write transaction,
                its state cannot change from or to NOT_STARTED while
                we are holding the trx_sys->mutex.  It may change from
                ACTIVE to PREPARED or COMMITTED. */
                if (version_trx
                    && trx_state_eq(version_trx,
                                    TRX_STATE_COMMITTED_IN_MEMORY)) {
                        version_trx = NULL;
                }
                trx_sys_mutex_exit();

If the trx exists in rw_trx_set, then check if the transaction state equals to TRX_STATE_COMMITTED_IN_MEMORY.

But according to the code in function trx_commit_in_memory, It will firstly invoke trx_erase_lists ( which removes the trx_t from trx_sys->rw_trx_set) and then changes the trx state to TRX_STATE_COMMITTED_IN_MEMORY.

The quoted code above and trx_earse_lists both acquire trx_sys->mutex. So I think if the trx exists in the set, its state shouldn't equal to TRX_STATE_COMMITTED_IN_MEMORY. And the checking is unnecessary.  

Just simplify the code as bellow:

                trx_sys_mutex_enter();
                version_trx = trx_get_rw_trx_by_id(version_trx_id);
                trx_sys_mutex_exit();

How to repeat:
code review 

Suggested fix:
Let me know if I missed something :)
[10 Mar 2017 17:54] Sinisa Milivojevic
Hi Zhai,

Your analysis seems correct. Still, there are three other places where a trx can get the state TRX_STATE_COMMITTED_IN_MEMORY so I think they need to be looked at in detil and see if they affect this particular code flow. 

This requires a very deep look by InnoDB developers , so I am verifying this bug.

Thank you.