Skip to content

Commit

Permalink
MDEV-24142: Remove the LatchDebug interface to rw-locks
Browse files Browse the repository at this point in the history
The latching order checks for rw-locks have not caught many bugs
in the past few years and they are greatly complicating the code.

Last time the debug checks were useful was in
commit 59caf2c (MDEV-13485).

The B-tree hang MDEV-14637 was not caught by LatchDebug,
because the granularity of the checks is not sufficient
to distinguish the levels of non-leaf B-tree pages.

The interface was already made dead code by the grandparent
commit 03ca649.
  • Loading branch information
dr-m committed Dec 3, 2020
1 parent 06efef4 commit ac028ec
Show file tree
Hide file tree
Showing 31 changed files with 33 additions and 561 deletions.
27 changes: 1 addition & 26 deletions storage/innobase/btr/btr0btr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,6 @@ btr_root_adjust_on_import(
goto func_exit;
}

buf_block_dbg_add_level(block, SYNC_TREE_NODE);

page = buf_block_get_frame(block);
page_zip = buf_block_get_page_zip(block);

Expand Down Expand Up @@ -490,8 +488,6 @@ btr_page_alloc_for_ibuf(
index->table->space->zip_size(),
RW_X_LATCH, mtr);

buf_block_dbg_add_level(new_block, SYNC_IBUF_TREE_NODE_NEW);

flst_remove(root, PAGE_HEADER + PAGE_BTR_IBUF_FREE_LIST,
new_block, PAGE_HEADER + PAGE_BTR_IBUF_FREE_LIST_NODE,
mtr);
Expand Down Expand Up @@ -557,21 +553,13 @@ btr_page_alloc(
for x-latching and initializing
the page */
{
buf_block_t* new_block;

if (dict_index_is_ibuf(index)) {

return(btr_page_alloc_for_ibuf(index, mtr));
}

new_block = btr_page_alloc_low(
return btr_page_alloc_low(
index, hint_page_no, file_direction, level, mtr, init_mtr);

if (new_block) {
buf_block_dbg_add_level(new_block, SYNC_TREE_NODE_NEW);
}

return(new_block);
}

/**************************************************************//**
Expand Down Expand Up @@ -979,8 +967,6 @@ btr_free_root_check(
page_id, zip_size, RW_X_LATCH, mtr);

if (block) {
buf_block_dbg_add_level(block, SYNC_TREE_NODE);

if (fil_page_index_page_check(block->frame)
&& index_id == btr_page_get_index_id(block->frame)) {
/* This should be a root page.
Expand Down Expand Up @@ -1030,9 +1016,6 @@ btr_create(
return(FIL_NULL);
}

buf_block_dbg_add_level(
ibuf_hdr_block, SYNC_IBUF_TREE_NODE_NEW);

ut_ad(ibuf_hdr_block->page.id().page_no()
== IBUF_HEADER_PAGE_NO);
/* Allocate then the next page to the segment: it will be the
Expand All @@ -1050,8 +1033,6 @@ btr_create(

ut_ad(block->page.id() == page_id_t(0,IBUF_TREE_ROOT_PAGE_NO));

buf_block_dbg_add_level(block, SYNC_IBUF_TREE_NODE_NEW);

flst_init(block, PAGE_HEADER + PAGE_BTR_IBUF_FREE_LIST, mtr);
} else {
block = fseg_create(space, PAGE_HEADER + PAGE_BTR_SEG_TOP,
Expand All @@ -1061,19 +1042,13 @@ btr_create(
return(FIL_NULL);
}

buf_block_dbg_add_level(block, SYNC_TREE_NODE_NEW);

if (!fseg_create(space, PAGE_HEADER + PAGE_BTR_SEG_LEAF, mtr,
false, block)) {
/* Not enough space for new segment, free root
segment before return. */
btr_free_root(block, mtr);
return(FIL_NULL);
}

/* The fseg create acquires a second latch on the page,
therefore we must declare it: */
buf_block_dbg_add_level(block, SYNC_TREE_NODE_NEW);
}

ut_ad(!page_has_siblings(block->frame));
Expand Down
19 changes: 2 additions & 17 deletions storage/innobase/btr/btr0cur.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ static dberr_t btr_cur_instant_init_low(dict_index_t* index, mtr_t* mtr)
page_id_t(space->id,
mach_read_from_4(ptr + BTR_EXTERN_PAGE_NO)),
0, RW_S_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_EXTERN_STORAGE);
if (fil_page_get_type(block->frame) != FIL_PAGE_TYPE_BLOB
|| mach_read_from_4(&block->frame[FIL_PAGE_DATA
+ BTR_BLOB_HDR_NEXT_PAGE_NO])
Expand Down Expand Up @@ -1798,17 +1797,13 @@ btr_cur_search_to_nth_level_func(
goto search_loop;
}

if (rw_latch != RW_NO_LATCH) {
#ifdef UNIV_ZIP_DEBUG
if (rw_latch != RW_NO_LATCH) {
const page_zip_des_t* page_zip
= buf_block_get_page_zip(block);
ut_a(!page_zip || page_zip_validate(page_zip, page, index));
#endif /* UNIV_ZIP_DEBUG */

buf_block_dbg_add_level(
block, dict_index_is_ibuf(index)
? SYNC_IBUF_TREE_NODE : SYNC_TREE_NODE);
}
#endif /* UNIV_ZIP_DEBUG */

ut_ad(fil_page_index_page_check(page));
ut_ad(index->id == btr_page_get_index_id(page));
Expand Down Expand Up @@ -4402,7 +4397,6 @@ static void btr_cur_trim_alter_metadata(dtuple_t* entry,
page_id_t(index->table->space->id,
mach_read_from_4(ptr + BTR_EXTERN_PAGE_NO)),
0, RW_S_LATCH, &mtr);
buf_block_dbg_add_level(block, SYNC_EXTERN_STORAGE);
ut_ad(fil_page_get_type(block->frame) == FIL_PAGE_TYPE_BLOB);
ut_ad(mach_read_from_4(&block->frame[FIL_PAGE_DATA
+ BTR_BLOB_HDR_NEXT_PAGE_NO])
Expand Down Expand Up @@ -7359,9 +7353,6 @@ btr_store_big_rec_extern_fields(
rec_block->zip_size(),
RW_X_LATCH, &mtr);

buf_block_dbg_add_level(prev_block,
SYNC_EXTERN_STORAGE);

if (page_zip) {
mtr.write<4>(*prev_block,
prev_block->frame
Expand Down Expand Up @@ -7680,12 +7671,8 @@ btr_free_externally_stored_field(
const page_id_t page_id(page_get_space_id(p),
page_get_page_no(p));

#if 0
buf_block_t* rec_block =
#endif
buf_page_get(page_id, rec_zip_size, RW_X_LATCH, &mtr);

buf_block_dbg_add_level(rec_block, SYNC_NO_ORDER_CHECK);
page_no = mach_read_from_4(field_ref + BTR_EXTERN_PAGE_NO);

if (/* There is no external storage data */
Expand All @@ -7712,7 +7699,6 @@ btr_free_externally_stored_field(
page_id_t(space_id, page_no), ext_zip_size,
RW_X_LATCH, &mtr);

buf_block_dbg_add_level(ext_block, SYNC_EXTERN_STORAGE);
page = buf_block_get_frame(ext_block);

if (ext_zip_size) {
Expand Down Expand Up @@ -7882,7 +7868,6 @@ btr_copy_blob_prefix(
mtr_start(&mtr);

block = buf_page_get(id, 0, RW_S_LATCH, &mtr);
buf_block_dbg_add_level(block, SYNC_EXTERN_STORAGE);
page = buf_block_get_frame(block);

btr_check_blob_fil_page_type(*block, true);
Expand Down
5 changes: 0 additions & 5 deletions storage/innobase/btr/btr0pcur.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,6 @@ btr_pcur_restore_position_func(
cursor->pos_state = BTR_PCUR_IS_POSITIONED;
cursor->latch_mode = latch_mode;

buf_block_dbg_add_level(
btr_pcur_get_block(cursor),
dict_index_is_ibuf(index)
? SYNC_IBUF_TREE_NODE : SYNC_TREE_NODE);

if (cursor->rel_pos == BTR_PCUR_ON) {
#ifdef UNIV_DEBUG
const rec_t* rec;
Expand Down
10 changes: 2 additions & 8 deletions storage/innobase/btr/btr0sea.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,6 @@ btr_search_guess_on_hash(

part->latch.rd_unlock();

buf_block_dbg_add_level(block, SYNC_TREE_NODE_FROM_HASH);
if (UNIV_UNLIKELY(fail)) {
goto fail_and_release_page;
}
Expand Down Expand Up @@ -1390,7 +1389,6 @@ void btr_search_drop_page_hash_when_freed(const page_id_t page_id)
{
buf_block_t* block;
mtr_t mtr;
dberr_t err = DB_SUCCESS;

mtr_start(&mtr);

Expand All @@ -1402,18 +1400,14 @@ void btr_search_drop_page_hash_when_freed(const page_id_t page_id)

block = buf_page_get_gen(page_id, 0, RW_X_LATCH, NULL,
BUF_PEEK_IF_IN_POOL, __FILE__, __LINE__,
&mtr, &err);
&mtr);

if (block) {

/* If AHI is still valid, page can't be in free state.
AHI is dropped when page is freed. */
DBUG_ASSERT(block->page.status != buf_page_t::FREED);

buf_block_dbg_add_level(block, SYNC_TREE_NODE_FROM_HASH);

dict_index_t* index = block->index;
if (index != NULL) {
if (block->index) {
/* In all our callers, the table handle should
be open, or we should be in the process of
dropping the table (preventing eviction). */
Expand Down
5 changes: 0 additions & 5 deletions storage/innobase/buf/buf0buf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2505,7 +2505,6 @@ void buf_page_free(const page_id_t page_id, mtr_t *mtr)
block->lock.x_lock();

block->page.status= buf_page_t::FREED;
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);
hash_lock->read_unlock();
}

Expand Down Expand Up @@ -3417,9 +3416,6 @@ buf_page_optimistic_get(
}

if (modify_clock != block->modify_clock) {

buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);

if (rw_latch == RW_S_LATCH) {
block->lock.s_unlock();
} else {
Expand Down Expand Up @@ -3494,7 +3490,6 @@ buf_page_try_get_func(
ut_ad(bpage->buf_fix_count());
ut_ad(bpage->state() == BUF_BLOCK_FILE_PAGE);
ut_ad(bpage->id() == page_id);
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);

buf_pool.stat.n_page_gets++;
return block;
Expand Down
10 changes: 2 additions & 8 deletions storage/innobase/buf/buf0dblwr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ buf_dblwr_t buf_dblwr;
/** @return the TRX_SYS page */
inline buf_block_t *buf_dblwr_trx_sys_get(mtr_t *mtr)
{
buf_block_t *block= buf_page_get(page_id_t(TRX_SYS_SPACE, TRX_SYS_PAGE_NO),
0, RW_X_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);
return block;
return buf_page_get(page_id_t(TRX_SYS_SPACE, TRX_SYS_PAGE_NO),
0, RW_X_LATCH, mtr);
}

/** Initialize the doublewrite buffer data structure.
Expand Down Expand Up @@ -121,10 +119,6 @@ bool buf_dblwr_t::create()
the InnoDB system tablespace file in the first place.
It could be located in separate optional file(s) in a
user-specified location. */

/* fseg_create acquires a second latch on the page,
therefore we must declare it: */
buf_block_dbg_add_level(b, SYNC_NO_ORDER_CHECK);
}

byte *fseg_header= TRX_SYS_DOUBLEWRITE + TRX_SYS_DOUBLEWRITE_FSEG +
Expand Down
9 changes: 0 additions & 9 deletions storage/innobase/dict/dict0boot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,6 @@ Created 4/18/1996 Heikki Tuuri
#include "log0recv.h"
#include "os0file.h"

/** @return the DICT_HDR block, x-latched */
buf_block_t *dict_hdr_get(mtr_t* mtr)
{
buf_block_t *block= buf_page_get(page_id_t(DICT_HDR_SPACE, DICT_HDR_PAGE_NO),
0, RW_X_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_DICT_HEADER);
return block;
}

/**********************************************************************//**
Returns a new table, index, or space id. */
void
Expand Down
12 changes: 0 additions & 12 deletions storage/innobase/fsp/fsp0fsp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ inline buf_block_t *fsp_get_header(const fil_space_t *space, mtr_t *mtr)
{
buf_block_t *block= buf_page_get(page_id_t(space->id, 0), space->zip_size(),
RW_SX_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
ut_ad(space->id == mach_read_from_4(FSP_HEADER_OFFSET + FSP_SPACE_ID +
block->frame));
return block;
Expand Down Expand Up @@ -349,8 +348,6 @@ xdes_get_descriptor_with_space_hdr(
block = buf_page_get(
page_id_t(space->id, descr_page_no), zip_size,
RW_SX_LATCH, mtr);

buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
}

if (desc_block != NULL) {
Expand Down Expand Up @@ -379,7 +376,6 @@ static xdes_t* xdes_get_descriptor(const fil_space_t *space, page_no_t offset,
{
buf_block_t *block= buf_page_get(page_id_t(space->id, 0), space->zip_size(),
RW_SX_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
return xdes_get_descriptor_with_space_hdr(block, space, offset, xdes, mtr);
}

Expand Down Expand Up @@ -412,8 +408,6 @@ xdes_get_descriptor_const(

if (buf_block_t* block = buf_page_get(page_id_t(space->id, page),
zip_size, RW_S_LATCH, mtr)) {
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);

ut_ad(page != 0 || space->free_limit == mach_read_from_4(
FSP_FREE_LIMIT + FSP_HEADER_OFFSET
+ block->frame));
Expand Down Expand Up @@ -553,8 +547,6 @@ void fsp_header_init(fil_space_t* space, uint32_t size, mtr_t* mtr)

buf_block_t* block = buf_page_create(space, 0, zip_size, mtr,
free_block);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);

if (UNIV_UNLIKELY(block != free_block)) {
buf_pool.free_block(free_block);
}
Expand Down Expand Up @@ -877,7 +869,6 @@ fsp_fill_free_list(
block= buf_page_create(
space, static_cast<uint32_t>(i),
zip_size, mtr, f);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
if (UNIV_UNLIKELY(block != f)) {
buf_pool.free_block(f);
}
Expand All @@ -894,7 +885,6 @@ fsp_fill_free_list(
static_cast<uint32_t>(
i + FSP_IBUF_BITMAP_OFFSET),
zip_size, mtr, f);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
if (UNIV_UNLIKELY(block != f)) {
buf_pool.free_block(f);
}
Expand Down Expand Up @@ -1357,7 +1347,6 @@ fsp_alloc_seg_inode_page(fil_space_t *space, buf_block_t *header, mtr_t *mtr)
if (!block)
return false;

buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
ut_ad(block->lock.not_recursive());

mtr->write<2>(*block, block->frame + FIL_PAGE_TYPE, FIL_PAGE_INODE);
Expand Down Expand Up @@ -1401,7 +1390,6 @@ fsp_alloc_seg_inode(fil_space_t *space, buf_block_t *header,
+ header->frame).page);

block = buf_page_get(page_id, space->zip_size(), RW_SX_LATCH, mtr);
buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
if (!space->full_crc32()) {
fil_block_check_type(*block, FIL_PAGE_INODE, mtr);
}
Expand Down
2 changes: 0 additions & 2 deletions storage/innobase/gis/gis0sea.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,6 @@ rtr_pcur_getnext_from_path(

if (block == NULL) {
continue;
} else if (rw_latch != RW_NO_LATCH) {
buf_block_dbg_add_level(block, SYNC_TREE_NODE);
}

rtr_info->tree_blocks[tree_idx] = block;
Expand Down
3 changes: 0 additions & 3 deletions storage/innobase/handler/ha_innodb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,6 @@ static PSI_mutex_info all_innodb_mutexes[] = {
PSI_KEY(recv_sys_mutex),
PSI_KEY(redo_rseg_mutex),
PSI_KEY(noredo_rseg_mutex),
# ifdef UNIV_DEBUG
PSI_KEY(rw_lock_debug_mutex),
# endif /* UNIV_DEBUG */
PSI_KEY(srv_innodb_monitor_mutex),
PSI_KEY(srv_misc_tmpfile_mutex),
PSI_KEY(srv_monitor_file_mutex),
Expand Down
Loading

0 comments on commit ac028ec

Please sign in to comment.