Details
-
Task
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
None
Description
The code is not necessary after MDEV-12289 implementation.
TrxUndoRsegs is wrapper for vector of trx_rseg_t*. It has two
constructors, both initialize the vector with only one element. And they
are used to push transactions rseg(the singular) to purge queue. There is
no function to add elements to the vector. The default constructor is used
only for declaration of NullElement.
The TrxUndoRsegs was introduced in WL#6915 in MySQL 5.7 and. MySQL 5.7
would unnecessarily let the purge of history parse the
temporary undo records, and then look up the table (via a global hash
table), and only at the point of processing the parsed undo log record
determine that the table is a temporary table and the undo record must be
thrown away.
In MariaDB 10.2 we have two disjoint sets of rollback segments (128 for
persistent, 128 for temporary), and purge does not even see the temporary
tables. The only reason why temporary tables are visible to other threads
is a SQL layer bug (MDEV-17805).
Attachments
Issue Links
- relates to
-
MDEV-12289 Keep 128 persistent rollback segments for compatibility and performance
-
- Closed
-
Activity
Thank you, this is OK after addressing my comments. It would be interesting to run a small benchmark, maybe something similar to what I did in MDEV-33213, that is, oltp_update_non_index followed by a shutdown with innodb_fast_shutdown=0 so that everything will be purged. Hopefully, perf record will not highlight anything obvious to be improved around the purge_sys.purge_queue.
oltp_update_non_index showed 150685.72 against 155543.60 tps in favour of version with the changes. innodb_fast_shutdown=0 did not cause any crashes both for release and debug builds. I have not found anything suspicious related to the changes in perf report.
Thank you. I think that simplifying this code should improve performance. It looks like this code was last refactored by
MDEV-33213in 10.6. Therefore, it would seem to make sense to fix this in the 10.6 branch, after extensive testing of course.This fix would merge the relevant part of TrxUndoRsegsIterator::set_next() to the start of purge_sys_t::choose_next_log(). As part of this, we could add a tail call to purge_sys_t::rseg_get_next_history_log() and adjust the callers, to simplify the control flow further:
diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc--- a/storage/innobase/trx/trx0purge.cc+++ b/storage/innobase/trx/trx0purge.cc@@ -920,6 +920,7 @@ void purge_sys_t::rseg_get_next_history_log()}rseg->latch.wr_unlock();+ return choose_next_log();}