Skip to content

Commit

Permalink
MDEV-22110 InnoDB unnecessarily writes unmodified pages
Browse files Browse the repository at this point in the history 
At least since commit 6a7be48
InnoDB appears to be invoking buf_flush_note_modification() on pages
that were exclusively latched but not modified in a mini-transaction.

MTR_MEMO_MODIFY, mtr_t::modify(): Define not only in debug code,
but also in release code. We will set the MTR_MEMO_MODIFY flag
on the earliest mtr_t::m_memo entry that we find.

MTR_LOG_NONE: Only use this mode in cases where the previous
mode will be restored before anything is modified in the mini-transaction.

MTR_MEMO_PAGE_X_MODIFY, MTR_MEMO_PAGE_SX_MODIFY: The allowed flag
combinations that include MTR_MEMO_MODIFY.

ReleaseBlocks: Only invoke buf_flush_note_modification()
on those buffer pool blocks on which mtr_t::set_modified()
and mtr_t::modify() were invoked.
  • Loading branch information
@dr-m
dr-m committed Jul 28, 2020
1 parent cf3c3cc commit 05fa455
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 141 deletions.
2 changes: 1 addition & 1 deletion storage/innobase/btr/btr0btr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,7 @@ btr_write_autoinc(dict_index_t* index, ib_uint64_t autoinc, bool reset)
static void btr_page_reorganize_low(page_cur_t *cursor, dict_index_t *index,
mtr_t *mtr)
{
const mtr_log_t log_mode= mtr->set_log_mode(MTR_LOG_NONE);
const mtr_log_t log_mode= mtr->set_log_mode(MTR_LOG_NO_REDO);

buf_block_t *const block= cursor->block;

Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/include/ibuf0ibuf.ic
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ibuf_mtr_start(
mtr->enter_ibuf();

if (high_level_read_only || srv_read_only_mode) {
mtr_set_log_mode(mtr, MTR_LOG_NONE);
mtr_set_log_mode(mtr, MTR_LOG_NO_REDO);
}

}
Expand Down
21 changes: 10 additions & 11 deletions storage/innobase/include/mtr0mtr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,15 @@ struct mtr_t {
return static_cast<mtr_log_t>(m_log_mode);
}

/** Change the logging mode.
@param mode logging mode
@return old mode */
inline mtr_log_t set_log_mode(mtr_log_t mode);
/** Change the logging mode.
@param mode logging mode
@return old mode */
mtr_log_t set_log_mode(mtr_log_t mode)
{
const mtr_log_t old_mode= get_log_mode();
m_log_mode= mode & 3;
return old_mode;
}

/** Copy the tablespaces associated with the mini-transaction
(needed for generating FILE_MODIFY records)
Expand Down Expand Up @@ -281,19 +286,13 @@ struct mtr_t {
private:
/** Note that the mini-transaction will modify data. */
void flag_modified() { m_modifications = true; }
#ifdef UNIV_DEBUG
/** Mark the given latched page as modified.
@param block page that will be modified */
void modify(const buf_block_t& block);
public:
/** Note that the mini-transaction will modify a block. */
void set_modified(const buf_block_t &block)
{ flag_modified(); if (m_log_mode == MTR_LOG_ALL) modify(block); }
#else /* UNIV_DEBUG */
public:
/** Note that the mini-transaction will modify a block. */
void set_modified(const buf_block_t &) { flag_modified(); }
#endif /* UNIV_DEBUG */
{ flag_modified(); if (m_log_mode != MTR_LOG_NONE) modify(block); }

/** Set the state to not-modified. This will not log the changes.
This is only used during redo log apply, to avoid logging the changes. */
Expand Down
36 changes: 0 additions & 36 deletions storage/innobase/include/mtr0mtr.ic
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,39 +171,3 @@ mtr_t::release_block_at_savepoint(

slot->object = NULL;
}

/**
Changes the logging mode of a mini-transaction.
@return old mode */

mtr_log_t
mtr_t::set_log_mode(mtr_log_t mode)
{
ut_ad(mode >= MTR_LOG_ALL);
ut_ad(mode <= MTR_LOG_NO_REDO);

const mtr_log_t old_mode = get_log_mode();

switch (old_mode) {
case MTR_LOG_NO_REDO:
/* Once this mode is set, it must not be changed. */
ut_ad(mode == MTR_LOG_NO_REDO || mode == MTR_LOG_NONE);
return(old_mode);
case MTR_LOG_NONE:
if (mode == old_mode) {
/* Keep MTR_LOG_NONE. */
return(old_mode);
}
ut_ad(mode == MTR_LOG_ALL);
/* fall through */
case MTR_LOG_ALL:
/* MTR_LOG_NO_REDO can only be set before generating
any redo log records. */
ut_ad(mode != MTR_LOG_NO_REDO || m_log.empty());
m_log_mode = mode & 3;
return(old_mode);
}

ut_ad(0);
return(old_mode);
}
8 changes: 4 additions & 4 deletions storage/innobase/include/mtr0types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ enum mtr_log_t {
MTR_LOG_ALL = 0,

/** Log no operations and dirty pages are not added to the flush list.
Set when applying log in crash recovery or when a modification of a
ROW_FORMAT=COMPRESSED page is attempted. */
Set for attempting modification of a ROW_FORMAT=COMPRESSED page. */
MTR_LOG_NONE,

/** Don't generate REDO log but add dirty pages to flush list */
Expand Down Expand Up @@ -329,9 +328,10 @@ enum mtr_memo_type_t {

MTR_MEMO_BUF_FIX = RW_NO_LATCH,

#ifdef UNIV_DEBUG
MTR_MEMO_MODIFY = 16,
#endif /* UNIV_DEBUG */

MTR_MEMO_PAGE_X_MODIFY = MTR_MEMO_PAGE_X_FIX | MTR_MEMO_MODIFY,
MTR_MEMO_PAGE_SX_MODIFY = MTR_MEMO_PAGE_SX_FIX | MTR_MEMO_MODIFY,

MTR_MEMO_S_LOCK = RW_S_LATCH << 5,

Expand Down
6 changes: 3 additions & 3 deletions storage/innobase/log/log0recv.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,7 @@ void recv_recover_page(fil_space_t* space, buf_page_t* bpage)
{
mtr_t mtr;
mtr.start();
mtr.set_log_mode(MTR_LOG_NONE);
mtr.set_log_mode(MTR_LOG_NO_REDO);

ut_ad(bpage->state() == BUF_BLOCK_FILE_PAGE);
buf_block_t* block = reinterpret_cast<buf_block_t*>(bpage);
Expand Down Expand Up @@ -2576,7 +2576,7 @@ inline buf_block_t *recv_sys_t::recover_low(const page_id_t page_id,
else if (fil_space_t *space= fil_space_acquire_for_io(page_id.space()))
{
mtr.start();
mtr.set_log_mode(MTR_LOG_NONE);
mtr.set_log_mode(MTR_LOG_NO_REDO);
block= buf_page_create(space, page_id.page_no(), space->zip_size(), &mtr);
p= recv_sys.pages.find(page_id);
if (p == recv_sys.pages.end())
Expand Down Expand Up @@ -2693,7 +2693,7 @@ void recv_sys_t::apply(bool last_batch)
continue;
case page_recv_t::RECV_NOT_PROCESSED:
mtr.start();
mtr.set_log_mode(MTR_LOG_NONE);
mtr.set_log_mode(MTR_LOG_NO_REDO);
if (buf_block_t *block= buf_page_get_low(page_id, 0, RW_X_LATCH,
nullptr, BUF_GET_IF_IN_POOL,
__FILE__, __LINE__,
Expand Down
178 changes: 94 additions & 84 deletions storage/innobase/mtr/mtr0mtr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,13 +205,6 @@ struct FindPage
static void memo_slot_release(mtr_memo_slot_t *slot)
{
switch (slot->type) {
#ifdef UNIV_DEBUG
default:
ut_ad("invalid type" == 0);
break;
case MTR_MEMO_MODIFY:
break;
#endif /* UNIV_DEBUG */
case MTR_MEMO_S_LOCK:
rw_lock_s_unlock(reinterpret_cast<rw_lock_t*>(slot->object));
break;
Expand All @@ -228,12 +221,21 @@ static void memo_slot_release(mtr_memo_slot_t *slot)
case MTR_MEMO_X_LOCK:
rw_lock_x_unlock(reinterpret_cast<rw_lock_t*>(slot->object));
break;
case MTR_MEMO_BUF_FIX:
case MTR_MEMO_PAGE_S_FIX:
case MTR_MEMO_PAGE_SX_FIX:
case MTR_MEMO_PAGE_X_FIX:
default:
#ifdef UNIV_DEBUG
switch (slot->type & ~MTR_MEMO_MODIFY) {
case MTR_MEMO_BUF_FIX:
case MTR_MEMO_PAGE_S_FIX:
case MTR_MEMO_PAGE_SX_FIX:
case MTR_MEMO_PAGE_X_FIX:
break;
default:
ut_ad("invalid type" == 0);
break;
}
#endif /* UNIV_DEBUG */
buf_block_t *block= reinterpret_cast<buf_block_t*>(slot->object);
buf_page_release_latch(block, slot->type);
buf_page_release_latch(block, slot->type & ~MTR_MEMO_MODIFY);
block->unfix();
break;
}
Expand All @@ -248,13 +250,6 @@ struct ReleaseLatches {
if (!slot->object)
return true;
switch (slot->type) {
#ifdef UNIV_DEBUG
default:
ut_ad("invalid type" == 0);
break;
case MTR_MEMO_MODIFY:
break;
#endif /* UNIV_DEBUG */
case MTR_MEMO_S_LOCK:
rw_lock_s_unlock(reinterpret_cast<rw_lock_t*>(slot->object));
break;
Expand All @@ -271,12 +266,21 @@ struct ReleaseLatches {
case MTR_MEMO_SX_LOCK:
rw_lock_sx_unlock(reinterpret_cast<rw_lock_t*>(slot->object));
break;
case MTR_MEMO_BUF_FIX:
case MTR_MEMO_PAGE_S_FIX:
case MTR_MEMO_PAGE_SX_FIX:
case MTR_MEMO_PAGE_X_FIX:
default:
#ifdef UNIV_DEBUG
switch (slot->type & ~MTR_MEMO_MODIFY) {
case MTR_MEMO_BUF_FIX:
case MTR_MEMO_PAGE_S_FIX:
case MTR_MEMO_PAGE_SX_FIX:
case MTR_MEMO_PAGE_X_FIX:
break;
default:
ut_ad("invalid type" == 0);
break;
}
#endif /* UNIV_DEBUG */
buf_block_t *block= reinterpret_cast<buf_block_t*>(slot->object);
buf_page_release_latch(block, slot->type);
buf_page_release_latch(block, slot->type & ~MTR_MEMO_MODIFY);
block->unfix();
break;
}
Expand Down Expand Up @@ -308,50 +312,42 @@ struct DebugCheck {
};
#endif

/** Release a resource acquired by the mini-transaction. */
struct ReleaseBlocks {
/** Release specific object */
ReleaseBlocks(lsn_t start_lsn, lsn_t end_lsn)
:
m_end_lsn(end_lsn),
m_start_lsn(start_lsn)
{
/* Do nothing */
}

/** Add the modified page to the buffer flush list. */
void add_dirty_page_to_flush_list(mtr_memo_slot_t* slot) const
{
ut_ad(m_end_lsn > 0);
ut_ad(m_start_lsn > 0);

buf_block_t* block;

block = reinterpret_cast<buf_block_t*>(slot->object);

buf_flush_note_modification(block, m_start_lsn, m_end_lsn);
}

/** @return true always. */
bool operator()(mtr_memo_slot_t* slot) const
{
if (slot->object != NULL) {

if (slot->type == MTR_MEMO_PAGE_X_FIX
|| slot->type == MTR_MEMO_PAGE_SX_FIX) {

add_dirty_page_to_flush_list(slot);
}
}
/** Release page latches held by the mini-transaction. */
struct ReleaseBlocks
{
const lsn_t start, end;
#ifdef UNIV_DEBUG
const mtr_buf_t &memo;

return(true);
}
ReleaseBlocks(lsn_t start, lsn_t end, const mtr_buf_t &memo) :
start(start), end(end), memo(memo)
#else /* UNIV_DEBUG */
ReleaseBlocks(lsn_t start, lsn_t end, const mtr_buf_t&) :
start(start), end(end)
#endif /* UNIV_DEBUG */
{
ut_ad(start);
ut_ad(end);
}

/** Mini-transaction REDO start LSN */
lsn_t m_end_lsn;
/** @return true always */
bool operator()(mtr_memo_slot_t* slot) const
{
if (!slot->object)
return true;
switch (slot->type) {
case MTR_MEMO_PAGE_X_MODIFY:
case MTR_MEMO_PAGE_SX_MODIFY:
break;
default:
ut_ad(!(slot->type & MTR_MEMO_MODIFY));
return true;
}

/** Mini-transaction REDO end LSN */
lsn_t m_start_lsn;
buf_flush_note_modification(static_cast<buf_block_t*>(slot->object),
start, end);
return true;
}
};

/** Write the block contents to the REDO log */
Expand Down Expand Up @@ -457,7 +453,8 @@ void mtr_t::commit()
}

m_memo.for_each_block_in_reverse(CIterate<const ReleaseBlocks>
(ReleaseBlocks(start_lsn, m_commit_lsn)));
(ReleaseBlocks(start_lsn, m_commit_lsn,
m_memo)));
if (m_made_dirty)
log_flush_order_mutex_exit();

Expand Down Expand Up @@ -834,40 +831,53 @@ mtr_t::memo_contains_page_flagged(
? NULL : iteration.functor.get_block();
}

/** Print info of an mtr handle. */
void
mtr_t::print() const
{
ib::info() << "Mini-transaction handle: memo size "
<< m_memo.size() << " bytes log size "
<< get_log()->size() << " bytes";
}

#endif /* UNIV_DEBUG */


/** Find a block, preferrably in MTR_MEMO_MODIFY state */
struct FindModified
{
const mtr_memo_slot_t *found= nullptr;
mtr_memo_slot_t *found= nullptr;
const buf_block_t& block;

FindModified(const buf_block_t &block) : block(block) {}
bool operator()(const mtr_memo_slot_t* slot)
bool operator()(mtr_memo_slot_t *slot)
{
if (slot->object != &block)
return true;
found= slot;
return slot->type != MTR_MEMO_MODIFY;
return !(slot->type & (MTR_MEMO_MODIFY |
MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX));
}
};

/** Mark the given latched page as modified.
@param block page that will be modified */
void mtr_t::modify(const buf_block_t &block)
{
Iterate<FindModified> iteration(block);
m_memo.for_each_block_in_reverse(iteration);
ut_ad(iteration.functor.found);
if (iteration.functor.found->type != MTR_MEMO_MODIFY)
memo_push(const_cast<buf_block_t*>(&block), MTR_MEMO_MODIFY);
}
if (UNIV_UNLIKELY(m_memo.empty()))
{
/* This must be PageConverter::update_page() in IMPORT TABLESPACE. */
ut_ad(!block.page.in_LRU_list);
ut_ad(!buf_pool.is_uncompressed(&block));
return;
}

/** Print info of an mtr handle. */
void
mtr_t::print() const
{
ib::info() << "Mini-transaction handle: memo size "
<< m_memo.size() << " bytes log size "
<< get_log()->size() << " bytes";
Iterate<FindModified> iteration((FindModified(block)));
if (UNIV_UNLIKELY(m_memo.for_each_block(iteration)))
{
ut_ad("modifying an unlatched page" == 0);
return;
}
iteration.functor.found->type= static_cast<mtr_memo_type_t>
(iteration.functor.found->type | MTR_MEMO_MODIFY);
}

#endif /* UNIV_DEBUG */
Loading

0 comments on commit 05fa455

Please sign in to comment.