From 44c7fdcf18f9a678446e07c36c3a4b022285977f Mon Sep 17 00:00:00 2001 From: Alessandro Vetere Date: Thu, 30 Jul 2026 18:02:44 +0200 Subject: [PATCH] MDEV-38056 Assertion 'bpage->state() >= buf_page_t::UNFIXED' in buf_page_get_zip() Three callers reconstruct old versions of a clustered index record, derive secondary index entries from them, and dereference the BLOB pointers of externally stored columns: row_vers_impl_x_locked_low() for the implicit lock check, row_undo_mod_sec_is_unsafe() for rollback, and row_check_index() for CHECK TABLE ... EXTENDED. purge_sys.view is what keeps those pages allocated, but purge_sys_t::view_guard froze it only for the duration of trx_undo_prev_version_build(), and the dereference happens after. On ROW_FORMAT=COMPRESSED this trips the assertion above; in a release build the freed page is read anyway, which is silent corruption. All three now hold purge_sys.latch across the dereference, and only where the version has externally stored columns, since row_build() dereferences nothing otherwise. Freezing that late means the view may have advanced since the walk decided a version was reachable, so each caller re-establishes that under the freeze, by testing the oldest writer whose undo log record it applied. That covers the newer ones: a newer version can only have been written once the older writer released the exclusive lock on the record. The first two can treat the test as an invariant, and end the walk if it fails. row_check_index() cannot: it decides reachability from purge_sys.end_view, which lags behind, and that lag is how it finds orphan secondary index records. It therefore stops where the test fails, as if the version chain had ended, which is how a version that can no longer be rebuilt is treated as well. Its two purge_sys.is_purgeable() tests now read the frozen view, which makes them atomic with the fetch they guard. trx_undo_prev_version(): remove the gate that was meant to stop CHECK TABLE ... EXTENDED from fetching BLOBs it may no longer own, and view_guard::is_extended() with it, which no guard mode could satisfy. That decision belongs to the caller, the only one that knows whether it will dereference anything. trx_purge(): a debug-only keyword that holds a batch open between its last purged record and purge_sys_t::batch_cleanup(), which is the window where a reader that goes by purge_sys.end_view can still reach history the batch has removed. innodb.old_blob_updel needs that window, and it is not otherwise addressable from a test, because it opens and closes within one batch. Tests: old_blob and old_blob_updel for the implicit lock check, old_blob_rollback for rollback, old_blob_check for CHECK TABLE ... EXTENDED. Each parks a walk at a dereference, makes the BLOB freeable, and asserts that the counter of purged update records, which is what would free it, stays at zero while parked and advances once the walk is over. old_blob_updel covers an undo log record that stores only the 20-byte reference; old_blob_rollback parks at a reference that the version merely inherited. All four fail with the original assertion when the freeze is removed, and skip above a 16k page size, which ROW_FORMAT=COMPRESSED requires. --- .../innodb/include/assert_blob_not_purged.inc | 28 ++++ mysql-test/suite/innodb/r/old_blob.result | 43 +++++++ .../suite/innodb/r/old_blob_check.result | 57 +++++++++ .../suite/innodb/r/old_blob_rollback.result | 53 ++++++++ .../suite/innodb/r/old_blob_updel.result | 65 ++++++++++ mysql-test/suite/innodb/t/old_blob.test | 80 ++++++++++++ mysql-test/suite/innodb/t/old_blob_check.test | 105 +++++++++++++++ .../suite/innodb/t/old_blob_rollback.test | 106 +++++++++++++++ mysql-test/suite/innodb/t/old_blob_updel.test | 121 ++++++++++++++++++ storage/innobase/include/trx0purge.h | 4 +- storage/innobase/row/row0purge.cc | 3 +- storage/innobase/row/row0sel.cc | 71 +++++++--- storage/innobase/row/row0umod.cc | 41 +++++- storage/innobase/row/row0vers.cc | 32 ++++- storage/innobase/trx/trx0purge.cc | 16 +++ storage/innobase/trx/trx0rec.cc | 39 +++--- 16 files changed, 812 insertions(+), 52 deletions(-) create mode 100644 mysql-test/suite/innodb/include/assert_blob_not_purged.inc create mode 100644 mysql-test/suite/innodb/r/old_blob.result create mode 100644 mysql-test/suite/innodb/r/old_blob_check.result create mode 100644 mysql-test/suite/innodb/r/old_blob_rollback.result create mode 100644 mysql-test/suite/innodb/r/old_blob_updel.result create mode 100644 mysql-test/suite/innodb/t/old_blob.test create mode 100644 mysql-test/suite/innodb/t/old_blob_check.test create mode 100644 mysql-test/suite/innodb/t/old_blob_rollback.test create mode 100644 mysql-test/suite/innodb/t/old_blob_updel.test diff --git a/mysql-test/suite/innodb/include/assert_blob_not_purged.inc b/mysql-test/suite/innodb/include/assert_blob_not_purged.inc new file mode 100644 index 0000000000000..378b6d5031a10 --- /dev/null +++ b/mysql-test/suite/innodb/include/assert_blob_not_purged.inc @@ -0,0 +1,28 @@ +# Assert that purge has not freed the BLOB while a version-chain walk is parked +# at the dereference of its reference. Purging an update record is what would +# free it; the caller must have enabled and reset that monitor, the counter +# being cumulative over the lifetime of the server. +# +# The wait gives purge every chance to get there first, which is what makes the +# assertion fail rather than pass by accident; without the freeze it needs about +# a second. It cannot be a wait_condition, because a frozen purge_sys.view is +# precisely what stops every purge counter from moving. + +--disable_query_log +let $purge_wait_rounds= 20; +while ($purge_wait_rounds) +{ + let $freed= `SELECT count FROM information_schema.innodb_metrics WHERE name = 'purge_upd_exist_or_extern_records'`; + if ($freed) + { + let $purge_wait_rounds= 0; + } + if (!$freed) + { + real_sleep 0.1; + dec $purge_wait_rounds; + } +} +--enable_query_log +SELECT count AS upd_records_purged_while_parked FROM information_schema.innodb_metrics +WHERE name = 'purge_upd_exist_or_extern_records'; diff --git a/mysql-test/suite/innodb/r/old_blob.result b/mysql-test/suite/innodb/r/old_blob.result new file mode 100644 index 0000000000000..0721e3dea915d --- /dev/null +++ b/mysql-test/suite/innodb/r/old_blob.result @@ -0,0 +1,43 @@ +# +# MDEV-38056 An implicit-lock check must keep purge_sys.view frozen across +# the dereference of an externally stored column of a version it rebuilt. +# The transaction it references commits meanwhile, deregistering itself +# from trx_sys before it waits for the reader's reference, which makes its +# history purgeable under the reader. +# +InnoDB 0 transactions not purged +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; +INSERT INTO t VALUES (1, CONCAT('x', REPEAT('x', @@innodb_page_size))); +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; +BEGIN; +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; +UPDATE t SET b = CONCAT('z', REPEAT('z', @@innodb_page_size)) WHERE a = 1; +connect reader,localhost,root,,; +SET DEBUG_SYNC='row_vers_impl_x_locked_row_build SIGNAL parked WAIT_FOR resume'; +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'z%' FOR UPDATE; +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +COMMIT; +connect con3,localhost,root,,; +SELECT count AS upd_records_purged_while_parked FROM information_schema.innodb_metrics +WHERE name = 'purge_upd_exist_or_extern_records'; +upd_records_purged_while_parked +0 +SET DEBUG_SYNC='now SIGNAL resume'; +connection reader; +a +1 +connection default; +disconnect reader; +connection con3; +connection default; +disconnect con3; +SET DEBUG_SYNC='RESET'; +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; diff --git a/mysql-test/suite/innodb/r/old_blob_check.result b/mysql-test/suite/innodb/r/old_blob_check.result new file mode 100644 index 0000000000000..c7d180c57074f --- /dev/null +++ b/mysql-test/suite/innodb/r/old_blob_check.result @@ -0,0 +1,57 @@ +# +# MDEV-38056 CHECK TABLE ... EXTENDED must keep purge_sys.view frozen +# across the dereference of an externally stored column of a version it +# rebuilt. It reaches versions that purge_sys.view already permits to be +# freed, deciding reachability from the lagging purge_sys.end_view. +# +InnoDB 0 transactions not purged +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; +INSERT INTO t VALUES (1, CONCAT('z', REPEAT('z', @@innodb_page_size))); +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; +connect purge_control,localhost,root,,; +START TRANSACTION WITH CONSISTENT SNAPSHOT; +connection default; +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; +connect victim,localhost,root,,; +BEGIN; +UPDATE t SET b = CONCAT('x', REPEAT('x', @@innodb_page_size)) WHERE a = 1; +connect checker,localhost,root,,; +SET DEBUG_SYNC='row_check_index_extended_match SIGNAL parked WAIT_FOR resume EXECUTE 2'; +CHECK TABLE t EXTENDED; +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +connection purge_control; +COMMIT; +disconnect purge_control; +connection default; +SELECT count AS upd_records_purged_while_parked FROM information_schema.innodb_metrics +WHERE name = 'purge_upd_exist_or_extern_records'; +upd_records_purged_while_parked +0 +SET DEBUG_SYNC='now SIGNAL resume'; +connection checker; +Table Op Msg_type Msg_text +test.t check status OK +connection default; +disconnect checker; +connection victim; +COMMIT; +connection default; +disconnect victim; +SET DEBUG_SYNC='RESET'; +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'xx%'; +a +1 +CHECK TABLE t EXTENDED; +Table Op Msg_type Msg_text +test.t check status OK +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; diff --git a/mysql-test/suite/innodb/r/old_blob_rollback.result b/mysql-test/suite/innodb/r/old_blob_rollback.result new file mode 100644 index 0000000000000..40880daa9c86c --- /dev/null +++ b/mysql-test/suite/innodb/r/old_blob_rollback.result @@ -0,0 +1,53 @@ +# +# MDEV-38056 A rollback must keep purge_sys.view frozen across the +# dereference of an externally stored column of a version it rebuilt. It +# froze that view only inside trx_undo_prev_version_build(), so purge was +# free to release the BLOB pages before the caller fetched them. +# +InnoDB 0 transactions not purged +CREATE TABLE t(a INT PRIMARY KEY, pad INT, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; +INSERT INTO t VALUES (1, 0, CONCAT('x', REPEAT('x', @@innodb_page_size))); +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; +connect purge_control,localhost,root,,; +START TRANSACTION WITH CONSISTENT SNAPSHOT; +connection default; +UPDATE t SET pad = 1 WHERE a = 1; +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; +connect victim,localhost,root,,; +BEGIN; +UPDATE t SET b = CONCAT('z', REPEAT('z', @@innodb_page_size)) WHERE a = 1; +SET DEBUG_SYNC='row_undo_mod_sec_is_unsafe_row_build SIGNAL parked WAIT_FOR resume EXECUTE 3'; +ROLLBACK; +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +connection purge_control; +COMMIT; +disconnect purge_control; +connection default; +SELECT count AS upd_records_purged_while_parked FROM information_schema.innodb_metrics +WHERE name = 'purge_upd_exist_or_extern_records'; +upd_records_purged_while_parked +0 +SET DEBUG_SYNC='now SIGNAL resume'; +connection victim; +connection default; +disconnect victim; +SET DEBUG_SYNC='RESET'; +SELECT a, pad FROM t FORCE INDEX(b) WHERE b LIKE 'yy%'; +a pad +1 1 +CHECK TABLE t; +Table Op Msg_type Msg_text +test.t check status OK +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; diff --git a/mysql-test/suite/innodb/r/old_blob_updel.result b/mysql-test/suite/innodb/r/old_blob_updel.result new file mode 100644 index 0000000000000..94a7591dbbdea --- /dev/null +++ b/mysql-test/suite/innodb/r/old_blob_updel.result @@ -0,0 +1,65 @@ +# +# MDEV-38056 As innodb.old_blob, but the version is rebuilt from the undo +# log record of an update of a record that another, committed transaction +# delete-marked. Such a record stores only the 20-byte reference, so the +# frozen purge_sys.view is the only thing that can keep the dereference +# valid. Reaching that version needs a batch to be still in progress. +# +InnoDB 0 transactions not purged +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; +INSERT INTO t VALUES (1, CONCAT('x', REPEAT('x', @@innodb_page_size))); +SET GLOBAL innodb_monitor_reset_all='purge_del_mark_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_del_mark_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; +connect purge_control,localhost,root,,; +START TRANSACTION WITH CONSISTENT SNAPSHOT; +connection default; +DELETE FROM t WHERE a = 1; +connect victim,localhost,root,,; +BEGIN; +INSERT INTO t VALUES (1, CONCAT('z', REPEAT('z', @@innodb_page_size))); +connection default; +SET @old_dbug= @@GLOBAL.debug_dbug; +SET GLOBAL debug_dbug='+d,purge_hold_cleanup'; +connection purge_control; +COMMIT; +disconnect purge_control; +connection default; +connect reader,localhost,root,,; +SET DEBUG_SYNC='row_vers_impl_x_locked_row_build SIGNAL parked WAIT_FOR resume'; +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'z%' FOR UPDATE; +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET GLOBAL debug_dbug= @old_dbug; +connection victim; +COMMIT; +connection default; +SELECT count AS upd_records_purged_while_parked FROM information_schema.innodb_metrics +WHERE name = 'purge_upd_exist_or_extern_records'; +upd_records_purged_while_parked +0 +SET DEBUG_SYNC='now SIGNAL resume'; +connection victim; +connection reader; +a +1 +connection default; +disconnect reader; +disconnect victim; +SET DEBUG_SYNC='RESET'; +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'zz%'; +a +1 +CHECK TABLE t; +Table Op Msg_type Msg_text +test.t check status OK +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_del_mark_records'; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_del_mark_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; diff --git a/mysql-test/suite/innodb/t/old_blob.test b/mysql-test/suite/innodb/t/old_blob.test new file mode 100644 index 0000000000000..7bdadf6d8f2fc --- /dev/null +++ b/mysql-test/suite/innodb/t/old_blob.test @@ -0,0 +1,80 @@ +--source include/have_innodb.inc +--source include/have_innodb_max_16k.inc +--source include/have_debug_sync.inc + +--echo # +--echo # MDEV-38056 An implicit-lock check must keep purge_sys.view frozen across +--echo # the dereference of an externally stored column of a version it rebuilt. +--echo # The transaction it references commits meanwhile, deregistering itself +--echo # from trx_sys before it waits for the reader's reference, which makes its +--echo # history purgeable under the reader. +--echo # + +--source include/wait_all_purged.inc + +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; + +INSERT INTO t VALUES (1, CONCAT('x', REPEAT('x', @@innodb_page_size))); + +# Filler rows keep the delete-marked entries off the leaf page holding the live +# 'z...' entry, which the reader latches; purge would block on it otherwise. +--disable_query_log +let $i= 60; +while ($i) +{ + eval INSERT INTO t VALUES (100 + $i, + CONCAT('y~', LPAD($i, 3, '0'), REPEAT('-', 800))); + dec $i; +} +--enable_query_log + +# The counter is cumulative over the lifetime of the server. +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; + +BEGIN; +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; +UPDATE t SET b = CONCAT('z', REPEAT('z', @@innodb_page_size)) WHERE a = 1; + +# The locking read references the still active transaction above and parks after +# building the previous version, before dereferencing its BLOB pointers. +connect (reader,localhost,root,,); +SET DEBUG_SYNC='row_vers_impl_x_locked_row_build SIGNAL parked WAIT_FOR resume'; +--send SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'z%' FOR UPDATE + +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +--send COMMIT + +# Both old BLOBs must still be allocated while the reader is parked on them. +connect (con3,localhost,root,,); +--source suite/innodb/include/assert_blob_not_purged.inc +SET DEBUG_SYNC='now SIGNAL resume'; + +connection reader; +--reap + +connection default; +--reap +disconnect reader; + +# Once the reader is done, purge advances, which confirms that only the frozen +# view held it back. +connection con3; +let $wait_condition= + SELECT count >= 2 FROM information_schema.innodb_metrics + WHERE name = 'purge_upd_exist_or_extern_records'; +--source include/wait_condition.inc + +connection default; +disconnect con3; +SET DEBUG_SYNC='RESET'; +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +--disable_warnings +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; +--enable_warnings diff --git a/mysql-test/suite/innodb/t/old_blob_check.test b/mysql-test/suite/innodb/t/old_blob_check.test new file mode 100644 index 0000000000000..2b7162a08a5e5 --- /dev/null +++ b/mysql-test/suite/innodb/t/old_blob_check.test @@ -0,0 +1,105 @@ +--source include/have_innodb.inc +--source include/have_innodb_max_16k.inc +--source include/have_debug_sync.inc + +--echo # +--echo # MDEV-38056 CHECK TABLE ... EXTENDED must keep purge_sys.view frozen +--echo # across the dereference of an externally stored column of a version it +--echo # rebuilt. It reaches versions that purge_sys.view already permits to be +--echo # freed, deciding reachability from the lagging purge_sys.end_view. +--echo # + +--source include/wait_all_purged.inc + +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; + +INSERT INTO t VALUES (1, CONCAT('z', REPEAT('z', @@innodb_page_size))); + +# Filler rows keep the delete-marked 'z...' entry off the first leaf page, which +# CHECK TABLE holds while parked; purge would block on it otherwise. +--disable_query_log +let $i= 60; +while ($i) +{ + eval INSERT INTO t VALUES (100 + $i, + CONCAT('y~', LPAD($i, 3, '0'), REPEAT('-', 800))); + dec $i; +} +--enable_query_log + +# The counter is cumulative over the lifetime of the server. +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; + +# Pin a read view on the 'z...' state, so that the BLOB the update below disowns +# stays allocated until we release it. +connect (purge_control,localhost,root,,); +START TRANSACTION WITH CONSISTENT SNAPSHOT; + +connection default; +# Committed update that disowns the 'z...' BLOB, which purge will free. +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; + +# An active transaction keeps the newest version invisible to CHECK TABLE, which +# is what makes it descend the version chain at all. +connect (victim,localhost,root,,); +BEGIN; +UPDATE t SET b = CONCAT('x', REPEAT('x', @@innodb_page_size)) WHERE a = 1; + +# While checking the live 'x...' entry, CHECK TABLE rebuilds the 'y...' and then +# the 'z...' version and compares the column prefix of each against it. +connect (checker,localhost,root,,); +SET DEBUG_SYNC='row_check_index_extended_match SIGNAL parked WAIT_FOR resume EXECUTE 2'; +--send CHECK TABLE t EXTENDED + +connection default; +# Hit 1: the 'y...' version, whose BLOB the active transaction above disowned and +# which is therefore not freeable. +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; + +# Hit 2: the 'z...' version, whose BLOB the committed update disowned. Release +# the read view, the last thing that kept that BLOB from being freeable. Purge +# has to remove the delete-marked 'z...' entry first, and that sits on the last +# leaf page while CHECK TABLE holds the first. +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +connection purge_control; +COMMIT; +disconnect purge_control; + +connection default; +--source suite/innodb/include/assert_blob_not_purged.inc +SET DEBUG_SYNC='now SIGNAL resume'; + +connection checker; +--reap + +connection default; +disconnect checker; + +connection victim; +COMMIT; + +connection default; +disconnect victim; + +# Once CHECK TABLE is done, purge advances and frees that BLOB, which confirms +# that only the frozen view held it back. +let $wait_condition= + SELECT count >= 1 FROM information_schema.innodb_metrics + WHERE name = 'purge_upd_exist_or_extern_records'; +--source include/wait_condition.inc + +SET DEBUG_SYNC='RESET'; + +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'xx%'; +CHECK TABLE t EXTENDED; +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +--disable_warnings +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; +--enable_warnings diff --git a/mysql-test/suite/innodb/t/old_blob_rollback.test b/mysql-test/suite/innodb/t/old_blob_rollback.test new file mode 100644 index 0000000000000..f0945d3de3eb0 --- /dev/null +++ b/mysql-test/suite/innodb/t/old_blob_rollback.test @@ -0,0 +1,106 @@ +--source include/have_innodb.inc +--source include/have_innodb_max_16k.inc +--source include/have_debug_sync.inc + +--echo # +--echo # MDEV-38056 A rollback must keep purge_sys.view frozen across the +--echo # dereference of an externally stored column of a version it rebuilt. It +--echo # froze that view only inside trx_undo_prev_version_build(), so purge was +--echo # free to release the BLOB pages before the caller fetched them. +--echo # + +--source include/wait_all_purged.inc + +CREATE TABLE t(a INT PRIMARY KEY, pad INT, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; + +INSERT INTO t VALUES (1, 0, CONCAT('x', REPEAT('x', @@innodb_page_size))); + +# Filler rows keep the delete-marked entries off the leaf page holding the live +# 'z...' entry, which the rollback latches; purge would block on it otherwise. +--disable_query_log +let $i= 60; +while ($i) +{ + eval INSERT INTO t VALUES (100 + $i, 0, + CONCAT('y~', LPAD($i, 3, '0'), REPEAT('-', 800))); + dec $i; +} +--enable_query_log + +# The counter is cumulative over the lifetime of the server. +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; + +# Pin a read view on the 'x...' state, so that the BLOB the updates below disown +# stays allocated until we release it. +connect (purge_control,localhost,root,,); +START TRANSACTION WITH CONSISTENT SNAPSHOT; + +connection default; +# Committed update that does not mention b: the version it produces inherits +# whatever reference b holds in the next newer version. +UPDATE t SET pad = 1 WHERE a = 1; +# Committed update that disowns the 'x...' BLOB, which purge will free. +UPDATE t SET b = CONCAT('y', REPEAT('y', @@innodb_page_size)) WHERE a = 1; + +connect (victim,localhost,root,,); +BEGIN; +UPDATE t SET b = CONCAT('z', REPEAT('z', @@innodb_page_size)) WHERE a = 1; + +# On ROLLBACK the walk is 'z...' -> 'y...' -> 'x...' -> 'x...' inherited. Park +# before each of the three row_build() calls. +SET DEBUG_SYNC='row_undo_mod_sec_is_unsafe_row_build SIGNAL parked WAIT_FOR resume EXECUTE 3'; +--send ROLLBACK + +connection default; +# Hit 1: the 'y...' version, whose BLOB belongs to the rolling back +# transaction and is never freeable. +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; + +# Hit 2: the 'x...' version, rebuilt from the b update's undo log record. The +# read view still pins that BLOB, so there is nothing to observe here. +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +SET DEBUG_SYNC='now SIGNAL resume'; + +# Hit 3: the version the pad update produced. Its undo log record says nothing +# about b, so it merely inherits the reference that the b update disowned, and a +# frozen purge_sys.view is the only thing that can keep the dereference valid. +# Release the read view, the last thing that kept that BLOB from being freeable. +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +connection purge_control; +COMMIT; +disconnect purge_control; + +connection default; +--source suite/innodb/include/assert_blob_not_purged.inc +SET DEBUG_SYNC='now SIGNAL resume'; + +connection victim; +--reap + +connection default; +disconnect victim; + +# Once the rollback is over, purge advances past both committed updates, which +# confirms that only the frozen view held it back. Only the b update frees a +# BLOB, the pad update having disowned none. +let $wait_condition= + SELECT count >= 2 FROM information_schema.innodb_metrics + WHERE name = 'purge_upd_exist_or_extern_records'; +--source include/wait_condition.inc + +SET DEBUG_SYNC='RESET'; + +# The rollback reverted to the committed 'y...' value, and the indexes agree. +SELECT a, pad FROM t FORCE INDEX(b) WHERE b LIKE 'yy%'; +CHECK TABLE t; +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +--disable_warnings +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; +--enable_warnings diff --git a/mysql-test/suite/innodb/t/old_blob_updel.test b/mysql-test/suite/innodb/t/old_blob_updel.test new file mode 100644 index 0000000000000..d0f6ae56a52fc --- /dev/null +++ b/mysql-test/suite/innodb/t/old_blob_updel.test @@ -0,0 +1,121 @@ +--source include/have_innodb.inc +--source include/have_innodb_max_16k.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc + +--echo # +--echo # MDEV-38056 As innodb.old_blob, but the version is rebuilt from the undo +--echo # log record of an update of a record that another, committed transaction +--echo # delete-marked. Such a record stores only the 20-byte reference, so the +--echo # frozen purge_sys.view is the only thing that can keep the dereference +--echo # valid. Reaching that version needs a batch to be still in progress. +--echo # + +--source include/wait_all_purged.inc + +CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(700))) +ENGINE=InnoDB STATS_PERSISTENT=0 ROW_FORMAT=COMPRESSED; + +INSERT INTO t VALUES (1, CONCAT('x', REPEAT('x', @@innodb_page_size))); + +# Filler rows keep the delete-marked 'x...' entry off the leaf page holding the +# live 'z...' entry, which the reader latches; purge would block on it otherwise. +--disable_query_log +let $i= 60; +while ($i) +{ + eval INSERT INTO t VALUES (100 + $i, + CONCAT('y~', LPAD($i, 3, '0'), REPEAT('-', 800))); + dec $i; +} +--enable_query_log + +# The counters are cumulative over the lifetime of the server. +SET GLOBAL innodb_monitor_reset_all='purge_del_mark_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_enable='purge_del_mark_records'; +SET GLOBAL innodb_monitor_enable='purge_upd_exist_or_extern_records'; + +# Pin purge so that the DELETE below stays unpurged until we choose. +connect (purge_control,localhost,root,,); +START TRANSACTION WITH CONSISTENT SNAPSHOT; + +connection default; +# T1 delete-marks the row; the 'x...' BLOB is still owned by the record. +DELETE FROM t WHERE a = 1; + +# T2 re-inserts the same PRIMARY KEY: an update of a record that T1 +# delete-marked, so the undo log record stores only the reference, no prefix. +connect (victim,localhost,root,,); +BEGIN; +INSERT INTO t VALUES (1, CONCAT('z', REPEAT('z', @@innodb_page_size))); + +# Hold the batch below open once it has purged T1, which is the window in which +# purge_sys.end_view still presents that history as reconstructible. +connection default; +SET @old_dbug= @@GLOBAL.debug_dbug; +SET GLOBAL debug_dbug='+d,purge_hold_cleanup'; + +# Let purge see T1 and remove the delete-marked 'x...' entry: nobody holds the +# clustered index leaf yet. T2 is still active, so purge stops there. +connection purge_control; +COMMIT; +disconnect purge_control; + +connection default; +let $wait_condition= + SELECT count >= 1 FROM information_schema.innodb_metrics + WHERE name = 'purge_del_mark_records'; +--source include/wait_condition.inc + +# The reader references the active T2. end_view cannot see T1, so it rebuilds the +# delete-marked 'x...' version and parks before dereferencing it. +connect (reader,localhost,root,,); +SET DEBUG_SYNC='row_vers_impl_x_locked_row_build SIGNAL parked WAIT_FOR resume'; +--send SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'z%' FOR UPDATE + +connection default; +SET DEBUG_SYNC='now WAIT_FOR parked TIMEOUT 60'; +# The reader has descended, so the batch may finish. The next one cannot start +# before the reader is done anyway, the reader holding the freeze. +SET GLOBAL debug_dbug= @old_dbug; + +# T2 commits, making purgeable the undo log record that disowned the 'x...' BLOB, +# and then waits for the reference the parked reader holds. +connection victim; +--send COMMIT + +connection default; +--source suite/innodb/include/assert_blob_not_purged.inc + +SET DEBUG_SYNC='now SIGNAL resume'; + +connection victim; +--reap +connection reader; +--reap + +connection default; +disconnect reader; +disconnect victim; +SET DEBUG_SYNC='RESET'; + +# Once the reader is done, purge advances and frees that BLOB, which confirms +# that only the frozen view held it back. +let $wait_condition= + SELECT count >= 1 FROM information_schema.innodb_metrics + WHERE name = 'purge_upd_exist_or_extern_records'; +--source include/wait_condition.inc + +SELECT a FROM t FORCE INDEX(b) WHERE b LIKE 'zz%'; +CHECK TABLE t; +DROP TABLE t; +SET GLOBAL innodb_monitor_disable='purge_del_mark_records'; +SET GLOBAL innodb_monitor_disable='purge_upd_exist_or_extern_records'; +SET GLOBAL innodb_monitor_reset_all='purge_del_mark_records'; +SET GLOBAL innodb_monitor_reset_all='purge_upd_exist_or_extern_records'; +--disable_warnings +SET GLOBAL innodb_monitor_enable=default; +SET GLOBAL innodb_monitor_disable=default; +SET GLOBAL innodb_monitor_reset_all=default; +--enable_warnings diff --git a/storage/innobase/include/trx0purge.h b/storage/innobase/include/trx0purge.h index 49c5ddfb276e1..d6dbc4fd72157 100644 --- a/storage/innobase/include/trx0purge.h +++ b/storage/innobase/include/trx0purge.h @@ -478,9 +478,6 @@ class purge_sys_t /** @return purge_sys.view or purge_sys.end_view */ inline const ReadViewBase &view() const; - - /** @return whether this is part of CHECK TABLE ... EXTENDED */ - bool is_extended() const noexcept { return latch < END_VIEW; } }; struct end_view_guard @@ -524,6 +521,7 @@ purge_sys_t::view_guard::view_guard(purge_sys_t::view_guard::guard latch) : { switch (latch) { case VIEW: + ut_ad(!purge_sys.latch.have_any()); /* nesting would hang on a writer */ purge_sys.latch.rd_lock(SRW_LOCK_CALL); break; case END_VIEW: diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc index 837e1b60edd1c..a896b1962f910 100644 --- a/storage/innobase/row/row0purge.cc +++ b/storage/innobase/row/row0purge.cc @@ -693,7 +693,8 @@ static bool row_purge_is_unsafe(const purge_node_t &node, rec_offs_comp(clust_offsets))) { row_ext_t* ext; - /* The stack of versions is locked by mtr. + /* The stack of versions is locked by mtr, and + purge_sys.view is frozen for the whole batch. Thus, it is safe to fetch the prefixes for externally stored columns. */ row = row_build(ROW_COPY_POINTERS, clust_index, diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 663bfd2dbc4dc..f4c8dbef0ffb9 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -6541,24 +6541,42 @@ dberr_t row_check_index(row_prebuilt_t *prebuilt, ulint *n_rows) check_latest_version: /* In CHECK TABLE...EXTENDED, always check if the secondary index record matches the latest clustered index record - version, no matter if it is visible in our own read view. - - If the latest clustered index version is delete-marked and - purgeable, it is not safe to fetch any BLOBs for column prefix - indexes because they may already have been freed. */ - if (rec_trx_id && - rec_get_deleted_flag(clust_rec, - prebuilt->table->not_redundant()) && - purge_sys.is_purgeable(rec_trx_id)) - goto did_not_find; - + version, no matter if it is visible in our own read view. */ if (!clust_offsets) clust_offsets= rec_get_offsets(clust_rec, clust_index, nullptr, clust_index->n_core_fields, ULINT_UNDEFINED, &heap); - err= row_check_index_match(prebuilt, - clust_rec, clust_index, clust_offsets, - rec, index, offsets); + + /* If the latest clustered index version is delete-marked and + purgeable, it is not safe to fetch any BLOBs for column prefix + indexes because they may already have been freed. Where anything + will be dereferenced, freeze purge_sys.view across the test and + the fetch, so that purge cannot start freeing in between. */ + { + const bool deleted= rec_trx_id && + rec_get_deleted_flag(clust_rec, prebuilt->table->not_redundant()); + + if (!rec_offs_any_extern(clust_offsets)) + { + if (deleted && purge_sys.is_purgeable(rec_trx_id)) + goto did_not_find; + + err= row_check_index_match(prebuilt, + clust_rec, clust_index, clust_offsets, + rec, index, offsets); + } + else + { + purge_sys_t::view_guard freeze{purge_sys_t::view_guard::VIEW}; + + if (deleted && freeze.view().changes_visible(rec_trx_id)) + goto did_not_find; + + err= row_check_index_match(prebuilt, + clust_rec, clust_index, clust_offsets, + rec, index, offsets); + } + } switch (err) { default: @@ -6631,6 +6649,9 @@ dberr_t row_check_index(row_prebuilt_t *prebuilt, ulint *n_rows) for (;;) { + /* The writer of clust_rec, whose undo log record rebuilds the next + version. Read before clust_rec is replaced by it below. */ + const trx_id_t version_trx_id= rec_trx_id; mem_heap_t *prev_heap= vers_heap; vers_heap= mem_heap_create(1024); err= trx_undo_prev_version_build(clust_rec, @@ -6698,10 +6719,30 @@ dberr_t row_check_index(row_prebuilt_t *prebuilt, ulint *n_rows) if (&view != &prebuilt->trx->read_view) { + /* Freeze purge_sys.view across the dereference in + row_check_index_match() and confirm that purge cannot see the + writer of the version clust_rec was rebuilt from. Unlike every + other walk over old versions, this one decides reachability from + the lagging purge_sys.end_view, so it does reach history that + purge is already free to remove; stop there as if the version + chain had ended. */ + purge_sys_t::view_guard freeze{purge_sys_t::view_guard::VIEW}; + + if (rec_offs_any_extern(clust_offsets) && + freeze.view().changes_visible(version_trx_id)) + { + mem_heap_free(vers_heap); + if (!got_extended_match) + goto extended_not_found; + goto did_not_find; + } + + DEBUG_SYNC_C("row_check_index_extended_match"); + /* It is not safe to fetch BLOBs of committed delete-marked records that may have been freed in purge. */ err= clust_rec_deleted && rec_trx_id && - purge_sys.is_purgeable(rec_trx_id) + freeze.view().changes_visible(rec_trx_id) ? DB_SUCCESS_LOCKED_REC : row_check_index_match(prebuilt, clust_rec, clust_index, clust_offsets, diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc index 24d7283bfc0c1..98f7379aeb101 100644 --- a/storage/innobase/row/row0umod.cc +++ b/storage/innobase/row/row0umod.cc @@ -520,6 +520,11 @@ static bool row_undo_mod_sec_is_unsafe(const rec_t *rec, dict_index_t *index, version = rec; for (;;) { + /* The writer of version, whose undo log record rebuilds the + next one. Read before the heap holding version is freed. */ + const trx_id_t version_trx_id = row_get_rec_trx_id( + version, clust_index, clust_offsets); + heap2 = heap; heap = mem_heap_create(1024); vrow = NULL; @@ -566,12 +571,36 @@ static bool row_undo_mod_sec_is_unsafe(const rec_t *rec, dict_index_t *index, if (!rec_get_deleted_flag(prev_version, comp)) { row_ext_t* ext; - /* The stack of versions is locked by mtr. - Thus, it is safe to fetch the prefixes for - externally stored columns. */ - row = row_build(ROW_COPY_POINTERS, clust_index, - prev_version, clust_offsets, - NULL, NULL, NULL, &ext, heap); + if (!rec_offs_any_extern(clust_offsets)) { + /* Nothing to dereference: the stack of + versions being locked by mtr is enough. */ + row = row_build(ROW_COPY_POINTERS, clust_index, + prev_version, clust_offsets, + NULL, NULL, NULL, &ext, heap); + } else { + /* Freeze purge_sys.view across the + dereference and confirm that purge cannot see + the writer of version, the oldest one this + walk has applied. If it can, stop and report + the secondary index entry as still needed, + which is the conservative answer. Only the + dereference needs the freeze: the prefixes end + up copied into heap. */ + purge_sys_t::view_guard freeze{ + purge_sys_t::view_guard::VIEW}; + + if (freeze.view().changes_visible( + version_trx_id)) { + break; + } + + DEBUG_SYNC_C( + "row_undo_mod_sec_is_unsafe_row_build"); + + row = row_build(ROW_COPY_POINTERS, clust_index, + prev_version, clust_offsets, + NULL, NULL, NULL, &ext, heap); + } if (dict_index_has_virtual(index)) { ut_ad(cur_vrow); diff --git a/storage/innobase/row/row0vers.cc b/storage/innobase/row/row0vers.cc index 30b859625c7ee..4815b332057fa 100644 --- a/storage/innobase/row/row0vers.cc +++ b/storage/innobase/row/row0vers.cc @@ -255,13 +255,33 @@ row_vers_impl_x_locked_low( prev_trx_id = row_get_rec_trx_id(prev_version, clust_index, clust_offsets); - /* The stack of versions is locked by mtr. Thus, it - is safe to fetch the prefixes for externally stored - columns. */ + if (!rec_offs_any_extern(clust_offsets)) { + /* Nothing to dereference: the stack of versions + being locked by mtr is enough. */ + row = row_build(ROW_COPY_POINTERS, clust_index, + prev_version, clust_offsets, + NULL, NULL, NULL, &ext, heap); + } else { + /* Freeze purge_sys.view across the dereference and + confirm that purge has not seen trx, which wrote + every undo log record this walk applies (the loop + only continues while prev_trx_id is trx->id). If it + has, trx is committed and holds no implicit lock. + Only the dereference needs the freeze: the prefixes + end up copied into heap. */ + purge_sys_t::view_guard freeze{ + purge_sys_t::view_guard::VIEW}; + + if (freeze.view().changes_visible(trx->id)) { + goto not_locked; + } + + DEBUG_SYNC_C("row_vers_impl_x_locked_row_build"); - row = row_build(ROW_COPY_POINTERS, clust_index, prev_version, - clust_offsets, - NULL, NULL, NULL, &ext, heap); + row = row_build(ROW_COPY_POINTERS, clust_index, + prev_version, clust_offsets, + NULL, NULL, NULL, &ext, heap); + } if (dict_index_has_virtual(index)) { if (vrow) { diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc index 2d5745bc33a52..2af0692fad792 100644 --- a/storage/innobase/trx/trx0purge.cc +++ b/storage/innobase/trx/trx0purge.cc @@ -1542,6 +1542,22 @@ TRANSACTIONAL_TARGET ulint trx_purge(ulint n_tasks, ulint history_size) trx_purge_close_tables(thd, nullptr, true); } +#ifdef UNIV_DEBUG + /* Hold the batch open between its last purged record and the advance of + purge_sys.head and purge_sys.end_view, which is the window where a reader + that goes by end_view can still reach history this batch has removed. That + window is not otherwise addressable from a test, because it opens and closes + within this function. Capped, so that a keyword left set cannot hang a run. */ + for (auto i= 100; i--; ) + { + bool hold= false; + DBUG_EXECUTE_IF("purge_hold_cleanup", hold= true;); + if (!hold) + break; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } +#endif + purge_sys.batch_cleanup(head); MONITOR_INC_VALUE(MONITOR_PURGE_INVOKED, 1); diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index 276d4d5d55777..d764406d63dd0 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -2256,16 +2256,19 @@ static dberr_t trx_undo_prev_version(const rec_t *rec, dict_index_t *index, cannot have purged the BLOBs referenced by that version yet). - This function does not fetch any BLOBs. The callers might, by - possibly invoking row_ext_create() via row_build(). However, - they should have all needed information in the *old_vers - returned by this function. This is because *old_vers is based - on the transaction undo log records. The function - trx_undo_page_fetch_ext() will write BLOB prefixes to the - transaction undo log that are at least as long as the longest - possible column prefix in a secondary index. Thus, secondary - index entries for *old_vers can be constructed without - dereferencing any BLOB pointers. */ + This function does not fetch any BLOBs, but the callers might, + by invoking row_ext_create() via row_build(). Both (a) and (b) + are statements about purge_sys.view, and they cover a reference + that the version merely inherited only if no transaction whose + undo log record was applied on the way down is visible to that + view, because one of those records disowned the reference. Such + a caller must therefore freeze purge_sys.view across the + dereference, by holding purge_sys.latch or by being a purge task, + and establish that condition under the freeze. Testing the + oldest writer applied suffices: a newer version can only have + been written once the older writer released the exclusive lock on + the record, so a view that sees a newer writer saw the older one + end before the view was created. */ ptr = trx_undo_rec_skip_row_ref(ptr, index); @@ -2281,18 +2284,12 @@ static dberr_t trx_undo_prev_version(const rec_t *rec, dict_index_t *index, records in secondary indexes, it normally covers some history that is already being purged. This is safe as long as the undo log records have not been freed yet. + Whether the externally stored columns of the version + built here may still be dereferenced is left to the + caller, the only one that knows whether it will + dereference any and can test that atomically with it. - However, BLOBs are only safe to access as long as the - purge_sys.view does not permit them to be freed. The - check.latch will freeze the purge_sys.view by blocking - purge_sys.clone_oldest_view() at the start of - trx_purge() or by blocking purge_sys.batch_cleanup() - at the end of trx_purge(). */ - if (check.is_extended() && purge_sys.is_purgeable(trx_id)) { - return DB_SUCCESS; - } - - /* We should confirm the existence of disowned external data, + We should confirm the existence of disowned external data, if the previous version record is delete marked. If the trx_id of the previous record is seen by purge view, we should treat it as missing history, because the disowned external data