From 7ead48a72ba285d037e4ca668378dbdd21ba8fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 29 Jul 2024 14:13:30 +0300 Subject: [PATCH] MDEV-34458: Remove more traces of BTR_MODIFY_PREV In commit 2f6df9374806665db0764c74187a384b562b150a we fixed an observed case of the bug by removing some code related to the no longer needed BTR_MODIFY_PREV mode. In commit 73ad436e16c3ac022a2f1c477eac0e9a7e702707 an alternative fix was applied that also fixes the BTR_SEARCH_PREV case. Let us clean up some implicit references to BTR_MODIFY_PREV that were missed in 2f6df9374806665db0764c74187a384b562b150a. btr_pcur_move_backward_from_page(): Assume that the latch mode was BTR_SEARCH_LEAF. btr_pcur_move_to_prev(): Assert that the latch mode is BTR_SEARCH_LEAF. This function is mostly invoked in row0sel.cc for read operations, as well as in row0merge.cc for reading from the clustered index. All callers indeed use a cursor in the BTR_SEARCH_LEAF mode. --- storage/innobase/btr/btr0cur.cc | 8 +++----- storage/innobase/btr/btr0pcur.cc | 17 +++++------------ 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 2d5756b0dbf..461976497f5 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -1301,11 +1301,9 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode, } switch (latch_mode) { - case BTR_SEARCH_PREV: - static_assert(BTR_SEARCH_PREV & BTR_SEARCH_LEAF, ""); + case BTR_SEARCH_PREV: /* btr_pcur_move_to_prev() */ ut_ad(!latch_by_caller); - ut_ad(rw_latch == - rw_lock_type_t(latch_mode & (RW_X_LATCH | RW_S_LATCH))); + ut_ad(rw_latch == RW_S_LATCH); /* latch also siblings from left to right */ if (page_has_prev(block->page.frame) && @@ -1479,7 +1477,7 @@ release_tree: rw_latch= RW_X_LATCH; break; case BTR_SEARCH_PREV: /* btr_pcur_move_to_prev() */ - ut_ad(rw_latch == RW_S_LATCH || rw_latch == RW_X_LATCH); + ut_ad(rw_latch == RW_S_LATCH); if (!not_first_access) buf_read_ahead_linear(page_id, zip_size); diff --git a/storage/innobase/btr/btr0pcur.cc b/storage/innobase/btr/btr0pcur.cc index 527ecfe354a..10005e5b501 100644 --- a/storage/innobase/btr/btr0pcur.cc +++ b/storage/innobase/btr/btr0pcur.cc @@ -336,10 +336,9 @@ btr_pcur_t::restore_position(btr_latch_mode restore_latch_mode, mtr_t *mtr) ut_a(old_n_core_fields <= index->n_core_fields); ut_a(old_n_fields); - static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), ""); + static_assert(int{BTR_SEARCH_PREV} == (4 | BTR_SEARCH_LEAF), ""); - switch (restore_latch_mode | 4) { - case BTR_SEARCH_PREV: + if ((restore_latch_mode | 4) == BTR_SEARCH_PREV) { /* Try optimistic restoration. */ if (btr_pcur_optimistic_latch_leaves(this, &restore_latch_mode, mtr)) { @@ -561,19 +560,13 @@ btr_pcur_move_backward_from_page( ut_ad(btr_pcur_is_before_first_on_page(cursor)); ut_ad(!btr_pcur_is_before_first_in_tree(cursor)); - const auto latch_mode = cursor->latch_mode; - ut_ad(latch_mode == BTR_SEARCH_LEAF || latch_mode == BTR_MODIFY_LEAF); - btr_pcur_store_position(cursor, mtr); mtr_commit(mtr); mtr_start(mtr); - static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), ""); - - if (UNIV_UNLIKELY(cursor->restore_position( - btr_latch_mode(4 | latch_mode), mtr) + if (UNIV_UNLIKELY(cursor->restore_position(BTR_SEARCH_PREV, mtr) == btr_pcur_t::CORRUPTED)) { return true; } @@ -605,7 +598,7 @@ btr_pcur_move_backward_from_page( mtr->rollback_to_savepoint(1); ut_ad(block == mtr->at_savepoint(0)); - cursor->latch_mode = latch_mode; + cursor->latch_mode = BTR_SEARCH_LEAF; cursor->old_rec = nullptr; return false; } @@ -622,7 +615,7 @@ btr_pcur_move_to_prev( mtr_t* mtr) /*!< in: mtr */ { ut_ad(cursor->pos_state == BTR_PCUR_IS_POSITIONED); - ut_ad(cursor->latch_mode != BTR_NO_LATCHES); + ut_ad(cursor->latch_mode == BTR_SEARCH_LEAF); cursor->old_rec = nullptr;