MySQL Bugs: #113648: Reduced lock interval for purge_sys->pq

Bug #113648	Reduced lock interval for purge_sys->pq_mutex
Submitted:	16 Jan 2024 9:19	Modified:	2 May 2024 8:26
Reporter:	alex xing (OCA)	Email Updates:
Status:	Not a Bug	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S5 (Performance)
Version:	8.0.32	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Contribution

Description:
1. Reduced lock interval for purge_sys->pq_mutex in trx_serialisation_number_get
2. no need to execute trx_add_to_serialisation_list and elem.set_trx_no within pq_mutex 

How to repeat:
just read the code

Suggested fix:
optimize it just as the below patch based on 8.0.32

a simple patch to describe the optimization (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: trx_commit_optimize.patch (text/plain), 567 bytes.

Hello Alex Xing,

Thank you for the report and contribution.

regards,
umesh

IIUC your patch, then two threads which would execute this fragment of code in parallel can get assigned two `added_trx_no` values in ascedning order, but when they enqueue rsegs to the purge_queue they might do that in descending order of trx->no. 
If purge coordinator thread looks into purge_queue in between those two inserts, then it will pop the larger of the two values only, and thus the batch will contain the Undo Log records of the later transaction but not the former.
This in turn will mean that undo log purge threads will try to purge Undo Logs of transaction with higher trx->no before purging the Undo Logs of transaction with smaller trx->no.
Moreover, the purge coordinator thread might conclude, that all of the Undo Logs with smaller trx->no were already purged, by bumping the purge_sys->limit and purge_sys->iter positions to match the most recently purged Undo Log record, of the transaction with higher trx->no, even though the Undo Log records of the trx with smaller trx->no was not yet processed by purge.
This would be bad, because after purging the current batch the `trx_purge()` calls `trx_purge_truncate()` to free the old "uneeded" Undo Logs based on these iterators - it assumes that any Undo Logs with trx->no smaller than the one iterator points to are no longer needed.
Thus it might end up freeing Undo Logs of the trx which wasn't purged yet.
That will lead to leaking of delete marked records in secondary indexes, and perhaps to other problems, such as old versions of BLOBs not being garbage collected.

Hello  Jakub Lopuszanski
Thank you for your detailed explanation, your innodb kernel skills are very profound, referring to my learning

Posted by developer:
 
This report describes an optimization, not a correctness issue.
Moreover, the proposed patch would introduce a correctness issue.
Therefore I am rejecting this bug report as "Closed, Not a Bug".
Thank you for trying to make MySQL better - feel free to report another patch without correctness issues