Skip to content

[demo] Do not call lock_rec_convert_impl_to_expl if a table S-lock is… #5526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mariadb-YuchenPei wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −0
Draft

[demo] Do not call lock_rec_convert_impl_to_expl if a table S-lock is… #5526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions storage/innobase/lock/lock0lock.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6628,6 +6628,7 @@ lock_clust_rec_read_check_and_lock(

trx_t *trx = thr_get_trx(thr);
if (lock_table_has(trx, index->table, LOCK_X)
|| lock_table_has(trx, index->table, LOCK_S)
|| heap_no == PAGE_HEAP_NO_SUPREMUM) {
Comment on lines 6630 to 6632

@dr-m dr-m Aug 17, 2026

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.

Because lock_table_has() is invoking lock_mode_stronger_or_eq() during the iteration, and based on the definition of lock_strength_matrix, it suffices to test for LOCK_S only. Besides, I would check the cheaper condition first:

diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 1cadfd1fa1d..072fe3354ed 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -6620,8 +6620,8 @@ lock_clust_rec_read_check_and_lock(
 	const ulint heap_no = lock_get_heap_no(*block, rec);
 
 	trx_t *trx = thr_get_trx(thr);
-	if (lock_table_has(trx, index->table, LOCK_X)
-	    || heap_no == PAGE_HEAP_NO_SUPREMUM) {
+	if (heap_no == PAGE_HEAP_NO_SUPREMUM
+	    || lock_table_has(trx, index->table, LOCK_S)) {
 	} else if (lock_rec_convert_impl_to_expl<true>(trx, *block, rec, index,
 						       offsets) == trx
 	    && gap_mode == LOCK_REC_NOT_GAP) {

I think that this simple improvement is safe to apply to GA releases.

@dr-m dr-m Aug 17, 2026

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.

I think that this needs a source code comment as well:

	if (heap_no == PAGE_HEAP_NO_SUPREMUM
	    || lock_table_has(trx, index->table, LOCK_S)) {
		/* A conflicting record lock (implicit LOCK_X or
		explicit LOCK_S or LOCK_X) may only be acquired by a
		transaction that holds LOCK_IX or LOCK_IS on
		the table; @see lock_rec_lock().

		Our LOCK_S or LOCK_X on the table is mutually
		exclusive with such transactions. */
	} else if (lock_rec_convert_impl_to_expl<true>(trx, *block, rec, index,

} else if (lock_rec_convert_impl_to_expl<true>(trx, *block, rec, index,
offsets) == trx
Expand Down