[MDEV-15104] Remove trx_sys_t::rw_trx_ids and trx_sys_t::serialisation_list - Jira

Details

Type: Task
Status: Closed (View Workflow)
Priority: Major
Resolution: Fixed
Fix Version/s: 10.3.5
Component/s: Storage Engine - InnoDB
Labels:
None

Epic Link:
arm64 optimization
Sprint:
10.2.13

Description

Downside of this approach is bigger overhead for view opening, because
iterating LF_HASH is more expensive compared to taking snapshot of an
array. However for low concurrency overhead difference is negligible,
while for high concurrency mutex is much bigger evil.

Currently we still take trx_sys.mutex to serialise ReadView creation.
This is required to keep serialisation_list ordered by trx->no as well
as not to let purge thread to create more recent snapshot while another
thread gets suspended during creation of older snapshot. This will
become completely mutex free along with serialisation_list removal.

Compared to previous implementation removing element from rw_trx_hash
and serialisation_list is not atomic. We disregard all possible bad
consequences (if there're any) since it will be solved along with
serialisation_list removal.

serialisation_list was supposed to instantly give minimum registered
transaction serialisation number. However maintaining and accessing
this list requires global mutex protection.

Since we already take MVCC snapshot by iterating trx_sys_t::rw_trx_hash,
it is cheap to integrate minimum registered transaction lookup into this
iteration.

Attachments

Issue Links

causes

MDEV-15155 [Draft] Assertion failed: m_state == 3 || m_state == 0 || m_state == 1, file d:\qa-win-debug\build\storage\innobase\include\read0types.h, line 156

Closed

MDEV-15246 InnoDB: Unknown error code 20: Required history data has been deleted

Closed

MDEV-22680 InnoDB trx_sys improvements

Open

is blocked by

MDEV-15132 Avoid accessing the TRX_SYS page

Closed

relates to

MDEV-29612 ReadViewBase::snapshot() misses an optimization

Closed

MDEV-33067 SCN(Sequence Commit Number) based MVCC

Open

(1 relates to)

Activity

Ascending order - Click to sort in descending order

Sergey Vojtovich created issue - 2018-01-28 20:06

Sergey Vojtovich made changes - 2018-01-28 20:06

Field	Original Value	New Value
Epic Link		MDEV-14442 [ 64369 ]

Sergey Vojtovich made changes - 2018-01-28 20:12

Description

Take snapshot of registered read-write transaction identifiers directly
from rw_trx_hash. It immediately saves one trx_sys.mutex lock, reduces
size of another critical section protected by this mutex, and makes
further optimisations like removing trx_sys_t::serialisation_list
possible.

Downside of this approach is bigger overhead for view opening, because
iterating LF_HASH is more expensive compared to taking snapshot of an
array. However for low concurrency overhead difference is negligible,
while for high concurrency mutex is much bigger evil.

Currently we still take trx_sys.mutex to serialise ReadView creation.
This is required to keep serialisation_list ordered by trx->no as well
as not to let purge thread to create more recent snapshot while another
thread gets suspended during creation of older snapshot. This will
become completely mutex free along with serialisation_list removal.

Compared to previous implementation removing element from rw_trx_hash
and serialisation_list is not atomic. We disregard all possible bad
consequences (if there're any) since it will be solved along with
serialisation_list removal.

serialisation_list was supposed to instantly give minimum registered
transaction serialisation number. However maintaining and accessing
this list requires global mutex protection.

Since we already take MVCC snapshot by iterating trx_sys_t::rw_trx_hash,
it is cheap to integrate minimum registered transaction lookup into this
iteration.

Sergey Vojtovich made changes - 2018-01-28 20:41

Status

Open [ 1 ]

Confirmed [ 10101 ]

Sergey Vojtovich added a comment - 2018-01-28 20:42

marko, please review patches for this task: https://github.com/MariaDB/server/commits/bb-10.3-arm

Sergey Vojtovich added a comment - 2018-01-28 20:42 marko , please review patches for this task: https://github.com/MariaDB/server/commits/bb-10.3-arm

Sergey Vojtovich made changes - 2018-01-28 20:42

Assignee	Sergey Vojtovich [ svoj ]	Marko Mäkelä [ marko ]
Status	Confirmed [ 10101 ]	In Review [ 10002 ]

Marko Mäkelä added a comment - 2018-01-29 19:07

This looks OK, except for a possible race or starvation where we fail to update the max_trx_id in the TRX_SYS page before writing newer trx_id values to undo logs.

I think that a proper fix to that is to do the equivalent of SELECT MAX(trx_id) FROM undo_logs on startup, and only when there are no undo logs (not even to purge), read the value from the TRX_SYS page. (And only write to the TRX_SYS page when trx_undo_truncate_start() discards the last undo log.)

Marko Mäkelä added a comment - 2018-01-29 19:07 This looks OK, except for a possible race or starvation where we fail to update the max_trx_id in the TRX_SYS page before writing newer trx_id values to undo logs. I think that a proper fix to that is to do the equivalent of SELECT MAX(trx_id) FROM undo_logs on startup, and only when there are no undo logs (not even to purge), read the value from the TRX_SYS page. (And only write to the TRX_SYS page when trx_undo_truncate_start() discards the last undo log.)

Marko Mäkelä made changes - 2018-01-29 19:07

Assignee	Marko Mäkelä [ marko ]	Sergey Vojtovich [ svoj ]
Status	In Review [ 10002 ]	Stalled [ 10000 ]

Marko Mäkelä made changes - 2018-01-30 18:06

Link

This issue is blocked by ~~MDEV-15132~~ [ ~~MDEV-15132~~ ]

Sergey Vojtovich made changes - 2018-01-31 14:55

Assignee	Sergey Vojtovich [ svoj ]	Marko Mäkelä [ marko ]
Status	Stalled [ 10000 ]	In Review [ 10002 ]

Marko Mäkelä added a comment - 2018-01-31 15:04

Thank you! Very good work.
I posted some minor comments to the MVCC snapshot optimization, mainly suggesting additional comments around state transitions.

Marko Mäkelä added a comment - 2018-01-31 15:04 Thank you! Very good work. I posted some minor comments to the MVCC snapshot optimization , mainly suggesting additional comments around state transitions.

Marko Mäkelä made changes - 2018-01-31 15:04

Assignee	Marko Mäkelä [ marko ]	Sergey Vojtovich [ svoj ]
Status	In Review [ 10002 ]	Stalled [ 10000 ]

Marko Mäkelä added a comment - 2018-01-31 15:05

For the record, ~~MDEV-15132~~ removes the race condition by removing the updates of the TRX_SYS page.

Marko Mäkelä added a comment - 2018-01-31 15:05 For the record, MDEV-15132 removes the race condition by removing the updates of the TRX_SYS page.

Sergey Vojtovich made changes - 2018-01-31 16:23

Fix Version/s		10.3.5 [ 22905 ]
Fix Version/s	10.3 [ 22126 ]
Resolution		Fixed [ 1 ]
Status	Stalled [ 10000 ]	Closed [ 6 ]

Sergei Golubchik made changes - 2018-01-31 18:44

Sprint

10.2.13 [ 228 ]

zhai weixiang added a comment - 2018-02-08 06:09

I got an assertion while running benchmark.(50% update-non-index, 50% select-on-pk), not sure if it relates to this change.

ersion: '10.3.5-MariaDB-rds-dev' socket: '/u01/maria/run/mysql.sock' port: 3306 Source distribution
2018-02-08 13:55:24 269 [ERROR] [FATAL] InnoDB: Unknown error code 20: Required history data has been deleted
180208 13:55:24 [ERROR] mysqld got signal 6 ;

the latest commit is 029ab11cc883d486117f30900c787c4b2765b742

zhai weixiang added a comment - 2018-02-08 06:09 I got an assertion while running benchmark.(50% update-non-index, 50% select-on-pk), not sure if it relates to this change. ersion: '10.3.5-MariaDB-rds-dev' socket: '/u01/maria/run/mysql.sock' port: 3306 Source distribution 2018-02-08 13:55:24 269 [ERROR] [FATAL] InnoDB: Unknown error code 20: Required history data has been deleted 180208 13:55:24 [ERROR] mysqld got signal 6 ; the latest commit is 029ab11cc883d486117f30900c787c4b2765b742

Marko Mäkelä made changes - 2018-02-08 06:44

Link

This issue causes ~~MDEV-15246~~ [ ~~MDEV-15246~~ ]

Marko Mäkelä added a comment - 2018-02-08 06:46

zhaiwx1987, sorry, we forgot to update this ticket to inform of this problem that axel found out when running benchmarks (actually to evaluate if ~~MDEV-15058~~ could cause trouble). He got exactly the same type of errors, but only when running the benchmark for long enough. He isolated the problem to the very last scalability commit by svoj:

commit bc7a1dc1fbd27e6064d3b40443fe242397668af7
Author: Sergey Vojtovich
Date: Tue Jan 30 20:59:42 2018 +0400

~~MDEV-15104~~ - Optimise MVCC snapshot

With trx_sys_t::rw_trx_ids removal, MVCC snapshot overhead became
slightly higher. That is instead of copying an array we now have to
iterate LF_HASH. All this done under trx_sys.mutex protection.

This patch moves MVCC snapshot out of trx_sys.mutex.

Please revert this commit, or test with its parent. axel ran benchmarks with is parent over last weekend, and did not report of any crashes.
svoj has been working on a fix. I just filed ~~MDEV-15246~~ to track this.

Marko Mäkelä added a comment - 2018-02-08 06:46 zhaiwx1987 , sorry, we forgot to update this ticket to inform of this problem that axel found out when running benchmarks (actually to evaluate if MDEV-15058 could cause trouble). He got exactly the same type of errors, but only when running the benchmark for long enough. He isolated the problem to the very last scalability commit by svoj : commit bc7a1dc1fbd27e6064d3b40443fe242397668af7 Author: Sergey Vojtovich Date: Tue Jan 30 20:59:42 2018 +0400 MDEV-15104 - Optimise MVCC snapshot With trx_sys_t::rw_trx_ids removal, MVCC snapshot overhead became slightly higher. That is instead of copying an array we now have to iterate LF_HASH . All this done under trx_sys.mutex protection. This patch moves MVCC snapshot out of trx_sys.mutex . Please revert this commit, or test with its parent. axel ran benchmarks with is parent over last weekend, and did not report of any crashes. svoj has been working on a fix. I just filed MDEV-15246 to track this.

zhai weixiang added a comment - 2018-02-08 07:04 - edited

Hi, Marko, thank you for your reply,
I just also noticed that the code format of class ReadView is mixed with innodb style and server style, maybe that needs an adjustment ?

zhai weixiang added a comment - 2018-02-08 07:04 - edited Hi, Marko, thank you for your reply, I just also noticed that the code format of class ReadView is mixed with innodb style and server style, maybe that needs an adjustment ?

Sergey Vojtovich added a comment - 2018-02-08 08:06

zhaiwx1987, it was agreed to use server coding style in new InnoDB code.

Sergey Vojtovich added a comment - 2018-02-08 08:06 zhaiwx1987 , it was agreed to use server coding style in new InnoDB code.

Marko Mäkelä made changes - 2018-02-23 02:53

Link

This issue causes ~~MDEV-15155~~ [ ~~MDEV-15155~~ ]

Sergey Vojtovich made changes - 2020-05-23 08:44

Link

This issue causes MDEV-22680 [ MDEV-22680 ]

Sergei Golubchik made changes - 2021-12-06 21:23

Workflow

MariaDB v3 [ 85183 ]

MariaDB v4 [ 133456 ]

Marko Mäkelä made changes - 2022-09-23 07:04

Link

This issue relates to ~~MDEV-29612~~ [ ~~MDEV-29612~~ ]

Marko Mäkelä made changes - 2023-12-19 13:35

Link

This issue relates to MDEV-33067 [ MDEV-33067 ]

People

Assignee:: Sergey Vojtovich

Reporter:: Sergey Vojtovich

Votes:: 0 Vote for this issue

Watchers:: 3 Start watching this issue

Dates

Created:: 2018-01-28 20:06

Updated:: 2023-12-19 13:35

Resolved:: 2018-01-31 16:23

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB Server

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration