Skip to content

MDEV-24813 Signal full scan to storage engines. - #3487

Open
mariadb-YuchenPei wants to merge 2 commits into
mainfrom
mdev-24813
Open

MDEV-24813 Signal full scan to storage engines.#3487
mariadb-YuchenPei wants to merge 2 commits into
mainfrom
mdev-24813

Conversation

@mariadb-YuchenPei

@mariadb-YuchenPei mariadb-YuchenPei commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

This is an initial patch for discussion.

Will work in the following minimal example

--source include/have_innodb.inc
CREATE TABLE t (a INT PRIMARY KEY) ENGINE=InnoDB;
insert into t values (42);

BEGIN;
SELECT * FROM t LOCK IN SHARE MODE; # will acquire S lock
COMMIT;

DROP TABLE t;

@CLAassistant

CLAassistant commented Aug 29, 2024

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mariadb-YuchenPei
mariadb-YuchenPei marked this pull request as draft August 29, 2024 05:57
Comment thread storage/innobase/include/row0mysql.h Outdated
Comment thread sql/records.cc Outdated
@spetrunia

Copy link
Copy Markdown
Member

Also, I've tried this testcase

create table t10 (a int, b varchar(100), index(a));
insert into t10 select seq, uuid() from seq_1_to_10000;
create table t11(a int);
explain insert into t11 select a from t10;
insert into t11 select a from t10;

and init_read_record() isn't invoked for it.

The index scan on t10 is initialized here:

#0  ha_innobase::index_init (this=0x7fffb80b6eb0, keynr=0)   at /home/psergey/dev-git/12.3-no-lock/storage/innobase/handler/ha_innodb.cc:8767
#1  0x0000555555f0a5a0 in handler::ha_index_init (this=0x7fffb80b6eb0,     idx=0, sorted=true)    at /home/psergey/dev-git/12.3-no-lock/sql/handler.h:3527
#2  0x0000555556125194 in join_read_first (tab=0x7fffb80cae40)    at /home/psergey/dev-git/12.3-no-lock/sql/sql_select.cc:24768

@spetrunia

Copy link
Copy Markdown
Member

With this patch, we have "Using where with pushed condition" all over the place:

MariaDB [test]> explain select * from t1 where b <1;
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
| id   | select_type | table | type | possible_keys | key  | key_len | ref  | rows  | Extra                             |
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
|    1 | SIMPLE      | t1    | ALL  | NULL          | NULL | NULL    | NULL | 10157 | Using where with pushed condition |
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
1 row in set (0.002 sec)

@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor Author

Also, I've tried this testcase

create table t10 (a int, b varchar(100), index(a));
insert into t10 select seq, uuid() from seq_1_to_10000;
create table t11(a int);
explain insert into t11 select a from t10;
insert into t11 select a from t10;

and init_read_record() isn't invoked for it.

The index scan on t10 is initialized here:

#0  ha_innobase::index_init (this=0x7fffb80b6eb0, keynr=0)   at /home/psergey/dev-git/12.3-no-lock/storage/innobase/handler/ha_innodb.cc:8767
#1  0x0000555555f0a5a0 in handler::ha_index_init (this=0x7fffb80b6eb0,     idx=0, sorted=true)    at /home/psergey/dev-git/12.3-no-lock/sql/handler.h:3527
#2  0x0000555556125194 in join_read_first (tab=0x7fffb80cae40)    at /home/psergey/dev-git/12.3-no-lock/sql/sql_select.cc:24768

You are right. I had forgotten to make the call in join_read_first (and join_read_last). Added the calls to ha_extra in a new commit now.

@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor Author

With this patch, we have "Using where with pushed condition" all over the place:

No longer the case with the new patch that does not comment out the cond_push if condition

@mariadb-YuchenPei
mariadb-YuchenPei changed the base branch from main to 11.4 March 1, 2026 23:22
@mariadb-YuchenPei
mariadb-YuchenPei marked this pull request as ready for review March 12, 2026 02:04
@mariadb-YuchenPei mariadb-YuchenPei changed the title MDEV-24813 [poc] Signal full table lock when cond is NULL MDEV-24813 Signal full scan to storage engines. Mar 12, 2026
@Thirunarayanan
Thirunarayanan requested review from iMineLink and removed request for Thirunarayanan June 2, 2026 12:24

@iMineLink iMineLink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my few minor comments, especially the ones around innodb_full_scan.test, thanks.

Comment thread mysql-test/main/innodb_full_scan.test Outdated
Comment thread mysql-test/main/innodb_full_scan.test Outdated
Comment thread storage/innobase/handler/ha_innodb.cc Outdated
Comment thread sql/sql_select.cc
Comment thread storage/innobase/handler/ha_innodb.cc Outdated
@Thirunarayanan
Thirunarayanan force-pushed the mdev-24813 branch 3 times, most recently from 5cd6733 to 8ab8cc5 Compare June 9, 2026 08:49
@iMineLink

Copy link
Copy Markdown
Contributor

Thanks for addressing my review comments, LGTM.

@mariadb-YuchenPei
mariadb-YuchenPei changed the base branch from 11.4 to main August 5, 2026 01:49
Comment thread mysql-test/suite/innodb/r/autoinc_debug,table_lock.rdiff Outdated
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the mdev-24813 branch 3 times, most recently from d3465cc to 5b84ca8 Compare August 6, 2026 06:05
When starting to do a full table/index scan without a WHERE or JOIN
condition, tell the storage engine so and the corresponding
ulong-truncated LIMIT.

Include an innodb implementation: added an innodb switch
table_lock_on_full_scan, so that when the switch is on, on receiving
the full scan signal from the sql layer, if the truncated LIMIT is
ULONG_MAX (likely no LIMIT), attempt to acquire a table lock.

Updated tests that have different results with the switch on.

The three deadlock_*_race tests cannot reach their DEBUG_SYNC race
under a table lock (it degenerates to a timeout), and the three I_S
tests only restate a lock-mode change already covered
by innodb_full_scan.test.

(Comment and code edited by Sergei Petrunia <sergey@mariadb.com> and
Thirunarayanan Balathandayuthapani <thiru@mariadb.com>)
For windows %p in `fprintf(f, "TRANSACTION (%p)", trx);` is not
prefixed with 0x, so we add a replacement /TRANSACTION
\([0-9A-F]{8}[0-9A-F]*\)/TRANSACTION (0xTHD)/. We use 8 as the minimum
length because not requiring one could cause something like
TRANSACTION (3) to be replaced with TRANSACTION (0xTHD)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

6 participants