Skip to content

Commit be0e3b2

Browse files
committed
MDEV-37753 lock_sec_rec_some_has_impl() unnecessarily fetches history
row_vers_impl_x_locked_low(): If a secondary index record points to a clustered index record that carries the current transaction identifier, then there cannot possibly be any implicit locks to that secondary index record, because those would have been checked before the current transaction got the implicit lock (modified the clustered index record) in the first place. This fix will avoid unnecessary access to undo log and possible BLOB pages, which may already have been freed in a purge operation. buf_page_get_zip(): Assert that the page is not marked as freed in the tablespace. This assertion could fire in a scenario like the test case when the table is created in ROW_FORMAT=COMPRESSED.
1 parent d04fc1a commit be0e3b2

File tree

4 files changed

+62
-3
lines changed

4 files changed

+62
-3
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#
2+
# MDEV-37753 lock_sec_rec_some_has_impl() unnecessarily fetches history
3+
#
4+
CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(1)))
5+
ENGINE=InnoDB STATS_PERSISTENT=0;
6+
connect purge_control,localhost,root,,;
7+
START TRANSACTION WITH CONSISTENT SNAPSHOT;
8+
connection default;
9+
SET GLOBAL innodb_max_purge_lag_wait=1;
10+
INSERT INTO t VALUES
11+
(1,REPEAT('x',@@innodb_page_size/2)),(2,REPEAT('z',@@innodb_page_size/2));
12+
DELETE FROM t;
13+
SELECT variable_value INTO @reads
14+
FROM information_schema.global_status
15+
WHERE variable_name = 'INNODB_BUFFER_POOL_READ_REQUESTS';
16+
INSERT INTO t VALUES(0,'y'),(1,'x'),(2,'z');
17+
SELECT variable_value-@reads INTO @diff
18+
FROM information_schema.global_status
19+
WHERE variable_name = 'INNODB_BUFFER_POOL_READ_REQUESTS';
20+
SELECT if(@diff between 22 and 25,'ok',@diff);
21+
if(@diff between 22 and 25,'ok',@diff)
22+
ok
23+
disconnect purge_control;
24+
DROP TABLE t;
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--source include/have_innodb.inc
2+
3+
--echo #
4+
--echo # MDEV-37753 lock_sec_rec_some_has_impl() unnecessarily fetches history
5+
--echo #
6+
CREATE TABLE t(a INT PRIMARY KEY, b TEXT, UNIQUE(b(1)))
7+
ENGINE=InnoDB STATS_PERSISTENT=0;
8+
9+
connect (purge_control,localhost,root,,);
10+
START TRANSACTION WITH CONSISTENT SNAPSHOT;
11+
connection default;
12+
SET GLOBAL innodb_max_purge_lag_wait=1;
13+
14+
INSERT INTO t VALUES
15+
(1,REPEAT('x',@@innodb_page_size/2)),(2,REPEAT('z',@@innodb_page_size/2));
16+
DELETE FROM t;
17+
18+
SELECT variable_value INTO @reads
19+
FROM information_schema.global_status
20+
WHERE variable_name = 'INNODB_BUFFER_POOL_READ_REQUESTS';
21+
22+
INSERT INTO t VALUES(0,'y'),(1,'x'),(2,'z');
23+
24+
SELECT variable_value-@reads INTO @diff
25+
FROM information_schema.global_status
26+
WHERE variable_name = 'INNODB_BUFFER_POOL_READ_REQUESTS';
27+
28+
# Typically, this reports 24 pages accessed.
29+
# If the fix is absent, we would access at least 26 pages.
30+
SELECT if(@diff between 22 and 25,'ok',@diff);
31+
disconnect purge_control;
32+
33+
DROP TABLE t;

storage/innobase/buf/buf0buf.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2336,6 +2336,7 @@ buf_page_t *buf_page_get_zip(const page_id_t page_id) noexcept
23362336
#ifdef UNIV_DEBUG
23372337
if (!(++buf_dbg_counter % 5771)) buf_pool.validate();
23382338
#endif /* UNIV_DEBUG */
2339+
ut_ad(bpage->state() >= buf_page_t::UNFIXED);
23392340
return bpage;
23402341
}
23412342

storage/innobase/row/row0vers.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,20 +120,21 @@ row_vers_impl_x_locked_low(
120120
clust_index->n_core_fields,
121121
ULINT_UNDEFINED, &heap);
122122

123+
trx_t* trx = nullptr;
123124
trx_id = row_get_rec_trx_id(clust_rec, clust_index, clust_offsets);
124125
if (trx_id <= mtr->trx->max_inactive_id) {
125126
/* The transaction history was already purged. */
127+
done:
126128
mem_heap_free(heap);
127-
DBUG_RETURN(0);
129+
DBUG_RETURN(trx);
128130
}
129131

130132
ut_ad(!clust_index->table->is_temporary());
131133

132-
trx_t* trx;
133-
134134
if (trx_id == mtr->trx->id) {
135135
trx = mtr->trx;
136136
trx->reference();
137+
goto done;
137138
} else {
138139
trx = trx_sys.find(mtr->trx, trx_id);
139140
if (trx == 0) {

0 commit comments

Comments
 (0)