Skip to content

Commit 02e72f7

Browse files
committed
MDEV-26769 follow-up: Remove unnecessary page locking
btr_cur_optimistic_latch_leaves(): Use transactional_shared_lock_guard. btr_cur_latch_leaves(): Avoid acquiring some page latches, because the changes are already blocked by index->lock. btr_cur_search_to_nth_level_func(): Remove a redundant variable retrying_for_search_prev=!!prev_tree_blocks, and avoid acquiring some page latches.
1 parent 14c5178 commit 02e72f7

File tree

2 files changed

+38
-28
lines changed

2 files changed

+38
-28
lines changed

storage/innobase/btr/btr0cur.cc

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ btr_cur_latch_leaves(
224224

225225
spatial = dict_index_is_spatial(cursor->index) && cursor->rtr_info;
226226
ut_ad(block->page.in_file());
227+
ut_ad(srv_read_only_mode
228+
|| mtr->memo_contains_flagged(&cursor->index->lock,
229+
MTR_MEMO_S_LOCK
230+
| MTR_MEMO_X_LOCK
231+
| MTR_MEMO_SX_LOCK));
227232

228233
switch (latch_mode) {
229234
case BTR_SEARCH_LEAF:
@@ -338,10 +343,10 @@ btr_cur_latch_leaves(
338343
case BTR_SEARCH_PREV:
339344
case BTR_MODIFY_PREV:
340345
mode = latch_mode == BTR_SEARCH_PREV ? RW_S_LATCH : RW_X_LATCH;
341-
/* latch also left sibling */
342-
block->lock.s_lock();
346+
/* Because we are holding index->lock, no page splits
347+
or merges may run concurrently, and we may read
348+
FIL_PAGE_PREV from a buffer-fixed, unlatched page. */
343349
left_page_no = btr_page_get_prev(block->frame);
344-
block->lock.s_unlock();
345350

346351
if (left_page_no != FIL_NULL) {
347352
latch_leaves.savepoints[0] = mtr_set_savepoint(mtr);
@@ -753,6 +758,7 @@ bool btr_cur_instant_root_init(dict_index_t* index, const page_t* page)
753758
@param[in,out] cursor cursor
754759
@param[in] mtr mini-transaction
755760
@return true if success */
761+
TRANSACTIONAL_TARGET
756762
bool
757763
btr_cur_optimistic_latch_leaves(
758764
buf_block_t* block,
@@ -774,14 +780,16 @@ btr_cur_optimistic_latch_leaves(
774780
modify_clock, mtr));
775781
case BTR_SEARCH_PREV:
776782
case BTR_MODIFY_PREV:
777-
block->lock.s_lock();
778-
if (block->modify_clock != modify_clock) {
779-
block->lock.s_unlock();
780-
return false;
783+
uint32_t curr_page_no, left_page_no;
784+
{
785+
transactional_shared_lock_guard<block_lock> g{
786+
block->lock};
787+
if (block->modify_clock != modify_clock) {
788+
return false;
789+
}
790+
curr_page_no = block->page.id().page_no();
791+
left_page_no = btr_page_get_prev(block->frame);
781792
}
782-
const uint32_t curr_page_no = block->page.id().page_no();
783-
const uint32_t left_page_no = btr_page_get_prev(block->frame);
784-
block->lock.s_unlock();
785793

786794
const rw_lock_type_t mode = *latch_mode == BTR_SEARCH_PREV
787795
? RW_S_LATCH : RW_X_LATCH;
@@ -1271,7 +1279,6 @@ btr_cur_search_to_nth_level_func(
12711279
ulint n_releases = 0;
12721280
bool detected_same_key_root = false;
12731281

1274-
bool retrying_for_search_prev = false;
12751282
ulint leftmost_from_level = 0;
12761283
buf_block_t** prev_tree_blocks = NULL;
12771284
ulint* prev_tree_savepoints = NULL;
@@ -1494,9 +1501,6 @@ btr_cur_search_to_nth_level_func(
14941501
default:
14951502
if (!srv_read_only_mode) {
14961503
if (s_latch_by_caller) {
1497-
ut_ad(mtr->memo_contains_flagged(
1498-
&index->lock, MTR_MEMO_S_LOCK
1499-
| MTR_MEMO_SX_LOCK));
15001504
} else if (!modify_external) {
15011505
/* BTR_SEARCH_TREE is intended to be used with
15021506
BTR_ALREADY_S_LATCHED */
@@ -1568,7 +1572,7 @@ btr_cur_search_to_nth_level_func(
15681572
if (height != 0) {
15691573
/* We are about to fetch the root or a non-leaf page. */
15701574
if ((latch_mode != BTR_MODIFY_TREE || height == level)
1571-
&& !retrying_for_search_prev) {
1575+
&& !prev_tree_blocks) {
15721576
/* If doesn't have SX or X latch of index,
15731577
each pages should be latched before reading. */
15741578
if (height == ULINT_UNDEFINED
@@ -1698,18 +1702,18 @@ btr_cur_search_to_nth_level_func(
16981702
goto retry_page_get;
16991703
}
17001704

1701-
if (retrying_for_search_prev && height != 0) {
1705+
if (height && prev_tree_blocks) {
17021706
/* also latch left sibling */
1703-
uint32_t left_page_no;
17041707
buf_block_t* get_block;
17051708

17061709
ut_ad(rw_latch == RW_NO_LATCH);
17071710

17081711
rw_latch = upper_rw_latch;
17091712

1710-
block->lock.s_lock();
1711-
left_page_no = btr_page_get_prev(buf_block_get_frame(block));
1712-
block->lock.s_unlock();
1713+
/* Because we are holding index->lock, no page splits
1714+
or merges may run concurrently, and we may read
1715+
FIL_PAGE_PREV from a buffer-fixed, unlatched page. */
1716+
uint32_t left_page_no = btr_page_get_prev(block->frame);
17131717

17141718
if (left_page_no != FIL_NULL) {
17151719
ut_ad(prev_n_blocks < leftmost_from_level);
@@ -1852,7 +1856,7 @@ btr_cur_search_to_nth_level_func(
18521856
}
18531857

18541858
/* release upper blocks */
1855-
if (retrying_for_search_prev) {
1859+
if (prev_tree_blocks) {
18561860
ut_ad(!autoinc);
18571861
for (;
18581862
prev_n_releases < prev_n_blocks;
@@ -2238,7 +2242,7 @@ btr_cur_search_to_nth_level_func(
22382242
BTR_MODIFY_PREV latches prev_page of the leaf page. */
22392243
if ((latch_mode == BTR_SEARCH_PREV
22402244
|| latch_mode == BTR_MODIFY_PREV)
2241-
&& !retrying_for_search_prev) {
2245+
&& !prev_tree_blocks) {
22422246
/* block should be latched for consistent
22432247
btr_page_get_prev() */
22442248
ut_ad(mtr->memo_contains_flagged(
@@ -2258,8 +2262,6 @@ btr_cur_search_to_nth_level_func(
22582262
if (height == 0 && leftmost_from_level > 0) {
22592263
/* should retry to get also prev_page
22602264
from level==leftmost_from_level. */
2261-
retrying_for_search_prev = true;
2262-
22632265
prev_tree_blocks = static_cast<buf_block_t**>(
22642266
ut_malloc_nokey(sizeof(buf_block_t*)
22652267
* leftmost_from_level));
@@ -2481,10 +2483,8 @@ btr_cur_search_to_nth_level_func(
24812483
mem_heap_free(heap);
24822484
}
24832485

2484-
if (retrying_for_search_prev) {
2485-
ut_free(prev_tree_blocks);
2486-
ut_free(prev_tree_savepoints);
2487-
}
2486+
ut_free(prev_tree_blocks);
2487+
ut_free(prev_tree_savepoints);
24882488

24892489
if (mbr_adj) {
24902490
/* remember that we will need to adjust parent MBR */

storage/innobase/include/sux_lock.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ class sux_lock final
271271

272272
/** @return whether any writer is waiting */
273273
bool is_waiting() const { return lock.is_waiting(); }
274+
275+
bool is_write_locked() const { return lock.is_write_locked(); }
276+
277+
inline void lock_shared();
278+
inline void unlock_shared();
274279
};
275280

276281
typedef sux_lock<ssux_lock_impl<true>> block_lock;
@@ -351,6 +356,11 @@ template<typename ssux> inline void sux_lock<ssux>::s_lock()
351356
ut_d(s_lock_register());
352357
}
353358

359+
template<typename ssux>
360+
inline void sux_lock<ssux>::lock_shared() { s_lock(); }
361+
template<typename ssux>
362+
inline void sux_lock<ssux>::unlock_shared() { s_unlock(); }
363+
354364
template<typename ssux> inline void sux_lock<ssux>::u_lock()
355365
{
356366
os_thread_id_t id= os_thread_get_curr_id();

0 commit comments

Comments
 (0)