Skip to content

Commit

Permalink
MDEV-34458 wait_for_read in buf_page_get_low hurts performance
Browse files Browse the repository at this point in the history 
BTR_MODIFY_PREV: Remove. This mode was only used by the change buffer,
which commit f27e9c8 (MDEV-29694)
removed.

buf_page_get_gen(): Revert the change that was made in
commit 90b95c6 (MDEV-33543)
because it is not applicable after MDEV-29694. This fixes the
performance regression that Vladislav Vaintroub reported.

This is a 11.x specific fix; this needs to be fixed differently
in older major versions where the change buffer is present.
  • Loading branch information
@dr-m
dr-m committed Jun 26, 2024
1 parent cc13630 commit 2f6df93
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 36 deletions.
6 changes: 1 addition & 5 deletions storage/innobase/btr/btr0cur.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,7 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode,
ut_ad(rw_lock_type_t(latch_mode & ~12) == RW_X_LATCH);
goto relatch_x;
}
if (latch_mode != BTR_MODIFY_PREV)
else
{
if (!latch_by_caller)
/* Release the tree s-latch */
Expand Down Expand Up @@ -1292,8 +1292,6 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode,

switch (latch_mode) {
case BTR_SEARCH_PREV:
case BTR_MODIFY_PREV:
static_assert(BTR_MODIFY_PREV & BTR_MODIFY_LEAF, "");
static_assert(BTR_SEARCH_PREV & BTR_SEARCH_LEAF, "");
ut_ad(!latch_by_caller);
ut_ad(rw_latch ==
Expand Down Expand Up @@ -1470,7 +1468,6 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode,
case BTR_MODIFY_ROOT_AND_LEAF:
rw_latch= RW_X_LATCH;
break;
case BTR_MODIFY_PREV: /* btr_pcur_move_to_prev() */
case BTR_SEARCH_PREV: /* btr_pcur_move_to_prev() */
ut_ad(rw_latch == RW_S_LATCH || rw_latch == RW_X_LATCH);

Expand Down Expand Up @@ -1854,7 +1851,6 @@ dberr_t btr_cur_t::open_leaf(bool first, dict_index_t *index,
ut_ad(!(latch_mode & 8));
/* This function doesn't need to lock left page of the leaf page */
static_assert(int{BTR_SEARCH_PREV} == (4 | BTR_SEARCH_LEAF), "");
static_assert(int{BTR_MODIFY_PREV} == (4 | BTR_MODIFY_LEAF), "");
latch_mode= btr_latch_mode(latch_mode & (RW_S_LATCH | RW_X_LATCH));
ut_ad(!latch_by_caller ||
mtr->memo_contains_flagged(&index->lock,
Expand Down
21 changes: 6 additions & 15 deletions storage/innobase/btr/btr0pcur.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,24 +215,18 @@ static bool btr_pcur_optimistic_latch_leaves(btr_pcur_t *pcur,
btr_latch_mode *latch_mode,
mtr_t *mtr)
{
static_assert(BTR_SEARCH_PREV & BTR_SEARCH_LEAF, "");
static_assert(BTR_MODIFY_PREV & BTR_MODIFY_LEAF, "");
static_assert((BTR_SEARCH_PREV ^ BTR_MODIFY_PREV) ==
(RW_S_LATCH ^ RW_X_LATCH), "");

buf_block_t *const block=
buf_page_optimistic_fix(pcur->btr_cur.page_cur.block, pcur->old_page_id);

if (!block)
return false;

if (*latch_mode == BTR_SEARCH_LEAF || *latch_mode == BTR_MODIFY_LEAF)
if (*latch_mode != BTR_SEARCH_PREV)
{
ut_ad(*latch_mode == BTR_SEARCH_LEAF || *latch_mode == BTR_MODIFY_LEAF);
return buf_page_optimistic_get(block, rw_lock_type_t(*latch_mode),
pcur->modify_clock, mtr);

ut_ad(*latch_mode == BTR_SEARCH_PREV || *latch_mode == BTR_MODIFY_PREV);
const rw_lock_type_t mode=
rw_lock_type_t(*latch_mode & (RW_X_LATCH | RW_S_LATCH));
}

uint64_t modify_clock;
uint32_t left_page_no;
Expand All @@ -258,7 +252,7 @@ static bool btr_pcur_optimistic_latch_leaves(btr_pcur_t *pcur,
{
prev= buf_page_get_gen(page_id_t(pcur->old_page_id.space(),
left_page_no), block->zip_size(),
mode, nullptr, BUF_GET_POSSIBLY_FREED, mtr);
RW_S_LATCH, nullptr, BUF_GET_POSSIBLY_FREED, mtr);
if (!prev ||
page_is_comp(prev->page.frame) != page_is_comp(block->page.frame) ||
memcmp_aligned<2>(block->page.frame, prev->page.frame, 2) ||
Expand All @@ -269,7 +263,7 @@ static bool btr_pcur_optimistic_latch_leaves(btr_pcur_t *pcur,
else
prev= nullptr;

mtr->upgrade_buffer_fix(savepoint, mode);
mtr->upgrade_buffer_fix(savepoint, RW_S_LATCH);

if (UNIV_UNLIKELY(block->modify_clock != modify_clock) ||
UNIV_UNLIKELY(block->page.is_freed()) ||
Expand Down Expand Up @@ -343,11 +337,9 @@ btr_pcur_t::restore_position(btr_latch_mode restore_latch_mode, mtr_t *mtr)
ut_a(old_n_fields);

static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), "");
static_assert(BTR_MODIFY_PREV == (4 | BTR_MODIFY_LEAF), "");

switch (restore_latch_mode | 4) {
case BTR_SEARCH_PREV:
case BTR_MODIFY_PREV:
/* Try optimistic restoration. */
if (btr_pcur_optimistic_latch_leaves(this, &restore_latch_mode,
mtr)) {
Expand Down Expand Up @@ -579,7 +571,6 @@ btr_pcur_move_backward_from_page(
mtr_start(mtr);

static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), "");
static_assert(BTR_MODIFY_PREV == (4 | BTR_MODIFY_LEAF), "");

if (UNIV_UNLIKELY(cursor->restore_position(
btr_latch_mode(4 | latch_mode), mtr)
Expand Down
15 changes: 3 additions & 12 deletions storage/innobase/buf/buf0buf.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2740,31 +2740,23 @@ buf_page_get_gen(
in buf_page_t::read_complete() or
buf_pool_t::corrupted_evict(), or
after buf_zip_decompress() in this function. */
if (rw_latch != RW_NO_LATCH) {
block->page.lock.s_lock();
} else if (!block->page.lock.s_lock_try()) {
/* For RW_NO_LATCH, we should not try to acquire S or X
latch directly as we could be violating the latching
order resulting in deadlock. Instead we try latching the
page and retry in case of a failure. */
goto wait_for_read;
}
block->page.lock.s_lock();
state = block->page.state();
ut_ad(state < buf_page_t::READ_FIX
|| state >= buf_page_t::WRITE_FIX);
const page_id_t id{block->page.id()};
block->page.lock.s_unlock();

if (UNIV_UNLIKELY(state < buf_page_t::UNFIXED)) {
block->page.unfix();
if (UNIV_UNLIKELY(id == page_id)) {
/* The page read was completed, and
another thread marked the page as free
while we were waiting. */
goto ignore_block;
goto ignore_unfixed;
}

ut_ad(id == page_id_t{~0ULL});
block->page.unfix();

if (++retries < BUF_PAGE_READ_MAX_RETRIES) {
goto loop;
Expand Down Expand Up @@ -2803,7 +2795,6 @@ buf_page_get_gen(
if (UNIV_UNLIKELY(!block->page.frame)) {
if (!block->page.lock.x_lock_try()) {
wait_for_unzip:
wait_for_read:
/* The page is being read or written, or
another thread is executing buf_zip_decompress()
in buf_page_get_gen() on it. */
Expand Down
1 change: 0 additions & 1 deletion storage/innobase/gis/gis0sea.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,6 @@ dberr_t rtr_search_to_nth_level(btr_cur_t *cur, que_thr_t *thr,
upper_rw_latch= RW_X_LATCH;
break;
default:
ut_ad(latch_mode != BTR_MODIFY_PREV);
ut_ad(latch_mode != BTR_SEARCH_PREV);
if (!latch_by_caller)
mtr_s_lock_index(index, mtr);
Expand Down
3 changes: 0 additions & 3 deletions storage/innobase/include/btr0types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ enum btr_latch_mode {
/** Search the previous record.
Used in btr_pcur_move_backward_from_page(). */
BTR_SEARCH_PREV = 4 | BTR_SEARCH_LEAF,
/** Modify the previous record.
Used in btr_pcur_move_backward_from_page(). */
BTR_MODIFY_PREV = 4 | BTR_MODIFY_LEAF,
/** Start modifying the entire B-tree. */
BTR_MODIFY_TREE = 8 | BTR_MODIFY_LEAF,
/** Continue modifying the entire R-tree.
Expand Down

0 comments on commit 2f6df93

Please sign in to comment.