MDEV-22110 InnoDB unnecessarily writes unmodified pagesbb-10.2-MDEV-22110-dont-log-unmodified-pages

This is backport from 10.5. Strictly speaking this is not a backport because
in 10.5 all the code which writes to mtr was moved to mtr_t class,
and the corresponding mtr_t functions have block_t parameter,
and that is why it's quite easy to mark the block as modified for mtr
in 10.5, it's enough just to call the correcponding function from the
corredponding mtr_t functions, which modify the block.

In 10.2 the code which writes in mtr_t is spread among the code base,
and that is why we can't backport MDEV-22110 as is. The common idea is
to find all mlog_write_initial_log_record_fast() and
mlog_write_initial_log_record_low() calls, and mark blocks as modified
there. We can't do this inside of mlog_write_initial_log_record_fast()
or mlog_write_initial_log_record_low() because when log mode is
MTR_LOG_NONE or MTR_LOG_NO_REDO, the functions are not invoked(see also
mlog_open() and it's invocation).

The downside of such approach is performace impact due to frequent
mtr_t::memo_modify_page() calls from mlog_write_*() and mlog_log_string()
functions.

17 files changed, 158 insertions, 72 deletions

diff --git a/storage/innobase/.clang-format b/storage/innobase/.clang-format
deleted file mode 100644
index 54f7b47bc88..00000000000
--- a/storage/innobase/.clang-format
+++ /dev/null
@@ -1,11 +0,0 @@
-UseTab: Always
-TabWidth: 8
-IndentWidth: 8
-ContinuationIndentWidth: 8
-BreakBeforeBinaryOperators: All
-PointerAlignment: Left
-BreakBeforeBraces: Custom
-ColumnLimit: 79
-BraceWrapping:
-  AfterFunction: true
-AccessModifierOffset: -8
diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
index 3d03c55bf15..20e4ee3c9e5 100644
--- a/storage/innobase/btr/btr0cur.cc
+++ b/storage/innobase/btr/btr0cur.cc
@@ -4952,6 +4952,7 @@ btr_cur_del_mark_set_sec_rec_log(
 	ut_ad(val <= 1);
 
 	log_ptr = mlog_open(mtr, 11 + 1 + 2);
+	mtr->memo_modify_page(rec);
 
 	if (!log_ptr) {
 		/* Logging in mtr is switched off during crash recovery:
diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index 68a8a9be261..a11334a7329 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -410,6 +410,7 @@ fil_space_crypt_t::write_page0(
 	DBUG_EXECUTE_IF("ib_do_not_log_crypt_data", return;);
 
 	byte* log_ptr = mlog_open(mtr, 11 + 17 + len);
+	mtr->memo_modify_page(page);
 
 	if (log_ptr != NULL) {
 		log_ptr = mlog_write_initial_log_record_fast(
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 328cd104e5d..934e5b60d4d 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -2160,6 +2160,8 @@ fil_op_write_log(
 	      || !strcmp(&path[strlen(path) - strlen(DOT_IBD)], DOT_IBD));
 
 	log_ptr = mlog_open(mtr, 11 + 4 + 2 + 1);
+	/* There is no need to mark page 0 as modified for file operations
+	because they do not really modify the page */
 
 	if (log_ptr == NULL) {
 		/* Logging in mtr is switched off during crash recovery:
diff --git a/storage/innobase/include/buf0buf.ic b/storage/innobase/include/buf0buf.ic
index 49b741ab5c8..5a4a21a883b 100644
--- a/storage/innobase/include/buf0buf.ic
+++ b/storage/innobase/include/buf0buf.ic
@@ -1372,6 +1372,10 @@ buf_page_release_latch(
 	} else if (rw_latch == RW_X_LATCH) {
 		rw_lock_x_unlock(&block->lock);
 	}
+#ifdef UNIV_DEBUG
+	else
+		ut_a(rw_latch == RW_NO_LATCH);
+#endif
 }
 
 #ifdef UNIV_DEBUG
diff --git a/storage/innobase/include/dyn0buf.h b/storage/innobase/include/dyn0buf.h
index da8d4b7de26..6c144dc4f1b 100644
--- a/storage/innobase/include/dyn0buf.h
+++ b/storage/innobase/include/dyn0buf.h
@@ -417,6 +417,12 @@ public:
 		return(m_heap == NULL);
 	}
 
+	/** @return whether the buffer is empty */
+	bool empty()
+	{
+		return !(const_cast<const block_t*>(back()))->m_used;
+	}
+
 private:
 	// Disable copying
 	dyn_buf_t(const dyn_buf_t&);
diff --git a/storage/innobase/include/mtr0log.ic b/storage/innobase/include/mtr0log.ic
index 5cfc08622d5..c18ba94d875 100644
--- a/storage/innobase/include/mtr0log.ic
+++ b/storage/innobase/include/mtr0log.ic
@@ -223,7 +223,6 @@ mlog_write_initial_log_record_fast(
 	ulint		offset;
 
 	ut_ad(log_ptr);
-	ut_d(mtr->memo_modify_page(ptr));
 
 	page = (const byte*) ut_align_down(ptr, UNIV_PAGE_SIZE);
 	space = mach_read_from_4(page + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
diff --git a/storage/innobase/include/mtr0mtr.h b/storage/innobase/include/mtr0mtr.h
index b57a38f8eab..9307120df27 100644
--- a/storage/innobase/include/mtr0mtr.h
+++ b/storage/innobase/include/mtr0mtr.h
@@ -389,10 +389,6 @@ struct mtr_t {
 		const byte*	ptr,
 		ulint		flags) const;
 
-	/** Mark the given latched page as modified.
-	@param[in]	ptr	pointer to within buffer frame */
-	void memo_modify_page(const byte* ptr);
-
 	/** Print info of an mtr handle. */
 	void print() const;
 
@@ -409,6 +405,15 @@ struct mtr_t {
 	mtr_buf_t* get_memo() { return &m_memo; }
 #endif /* UNIV_DEBUG */
 
+	/** Mark the given latched page as modified.
+	@param[in]	ptr	pointer to within buffer frame */
+	void memo_modify_page(const byte* ptr);
+
+	/** Mark the given latched page as modified.
+	@param[in]	space	page's space id
+	@param[in]	page	page number*/
+	void memo_modify_page(ulint space, ulint page);
+
 	/** @return true if a record was added to the mini-transaction */
 	bool is_dirty() const { return m_made_dirty; }
 
diff --git a/storage/innobase/include/mtr0mtr.ic b/storage/innobase/include/mtr0mtr.ic
index a45d088d5d7..cf9756fb8c2 100644
--- a/storage/innobase/include/mtr0mtr.ic
+++ b/storage/innobase/include/mtr0mtr.ic
@@ -170,7 +170,7 @@ mtr_t::release_block_at_savepoint(
 
 	ut_a(slot->object == block);
 
-	buf_page_release_latch(block, slot->type);
+	buf_page_release_latch(block, slot->type & ~MTR_MEMO_MODIFY);
 
 	buf_block_unfix(reinterpret_cast<buf_block_t*>(block));
 
diff --git a/storage/innobase/include/mtr0types.h b/storage/innobase/include/mtr0types.h
index 2d4cd7b97ac..9791eda6d5f 100644
--- a/storage/innobase/include/mtr0types.h
+++ b/storage/innobase/include/mtr0types.h
@@ -255,9 +255,7 @@ enum mtr_memo_type_t {
 
 	MTR_MEMO_BUF_FIX = RW_NO_LATCH,
 
-#ifdef UNIV_DEBUG
 	MTR_MEMO_MODIFY = 16,
-#endif /* UNIV_DEBUG */
 
 	MTR_MEMO_S_LOCK = RW_S_LATCH << 5,
 
diff --git a/storage/innobase/mtr/mtr0log.cc b/storage/innobase/mtr/mtr0log.cc
index 0e6a80cb363..430c2291d05 100644
--- a/storage/innobase/mtr/mtr0log.cc
+++ b/storage/innobase/mtr/mtr0log.cc
@@ -68,6 +68,7 @@ mlog_write_initial_log_record(
 	ut_ad(type > MLOG_8BYTES);
 
 	log_ptr = mlog_open(mtr, 11);
+	mtr->memo_modify_page(ptr);
 
 	/* If no logging is requested, we may return now */
 	if (log_ptr == NULL) {
@@ -255,7 +256,7 @@ mlog_write_ulint(
 
 	if (mtr != 0) {
 		byte*	log_ptr = mlog_open(mtr, 11 + 2 + 5);
-
+		mtr->memo_modify_page(ptr);
 		/* If no logging is requested, we may return now */
 
 		if (log_ptr != 0) {
@@ -287,6 +288,7 @@ mlog_write_ull(
 
 	if (mtr != 0) {
 		byte*	log_ptr = mlog_open(mtr, 11 + 2 + 9);
+		mtr->memo_modify_page(ptr);
 
 		/* If no logging is requested, we may return now */
 		if (log_ptr != 0) {
@@ -339,6 +341,7 @@ mlog_log_string(
 	ut_ad(len <= UNIV_PAGE_SIZE);
 
 	log_ptr = mlog_open(mtr, 30);
+	mtr->memo_modify_page(ptr);
 
 	/* If no logging is requested, we may return now */
 	if (log_ptr == NULL) {
@@ -432,6 +435,7 @@ mlog_open_and_write_index(
 
 	if (!page_rec_is_comp(rec)) {
 		log_start = log_ptr = mlog_open(mtr, 11 + size);
+		mtr->memo_modify_page(rec);
 		if (!log_ptr) {
 			return(NULL); /* logging is disabled */
 		}
@@ -457,6 +461,7 @@ mlog_open_and_write_index(
 		}
 
 		log_start = log_ptr = mlog_open(mtr, alloc);
+		mtr->memo_modify_page(rec);
 
 		if (!log_ptr) {
 			return(NULL); /* logging is disabled */
diff --git a/storage/innobase/mtr/mtr0mtr.cc b/storage/innobase/mtr/mtr0mtr.cc
index fefc0687ddb..f8d4608cdcb 100644
--- a/storage/innobase/mtr/mtr0mtr.cc
+++ b/storage/innobase/mtr/mtr0mtr.cc
@@ -114,7 +114,8 @@ struct Find {
 	/** @return false if the object was found. */
 	bool operator()(mtr_memo_slot_t* slot)
 	{
-		if (m_object == slot->object && m_type == slot->type) {
+		if (m_object == slot->object
+		    && m_type == (slot->type & ~MTR_MEMO_MODIFY)) {
 			m_slot = slot;
 			return(false);
 		}
@@ -159,7 +160,8 @@ struct FindPage
 	{
 		ut_ad(m_slot == NULL);
 
-		if (!(m_flags & slot->type) || slot->object == NULL) {
+		if (!(m_flags & (slot->type & ~MTR_MEMO_MODIFY))
+		    || slot->object == NULL) {
 			return(true);
 		}
 
@@ -204,14 +206,10 @@ private:
 @param slot	memo slot */
 static void memo_slot_release(mtr_memo_slot_t *slot)
 {
-  switch (slot->type) {
-#ifdef UNIV_DEBUG
+  switch (slot->type & ~MTR_MEMO_MODIFY) {
   default:
     ut_ad(!"invalid type");
     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;
@@ -233,7 +231,7 @@ static void memo_slot_release(mtr_memo_slot_t *slot)
   case MTR_MEMO_PAGE_SX_FIX:
   case MTR_MEMO_PAGE_X_FIX:
     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);
     buf_block_unfix(block);
     break;
   }
@@ -247,14 +245,10 @@ struct ReleaseLatches {
   {
     if (!slot->object)
       return true;
-    switch (slot->type) {
-#ifdef UNIV_DEBUG
+    switch (slot->type & ~MTR_MEMO_MODIFY) {
     default:
       ut_ad(!"invalid type");
       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;
@@ -276,7 +270,7 @@ struct ReleaseLatches {
     case MTR_MEMO_PAGE_SX_FIX:
     case MTR_MEMO_PAGE_X_FIX:
       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);
       buf_block_unfix(block);
       break;
     }
@@ -320,31 +314,27 @@ struct ReleaseBlocks {
 		/* 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, m_flush_observer);
-	}
-
 	/** @return true always. */
 	bool operator()(mtr_memo_slot_t* slot) const
 	{
-		if (slot->object != NULL) {
+		if (!slot->object)
+			return true;
+
+		switch (slot->type) {
+		case (MTR_MEMO_PAGE_X_FIX | MTR_MEMO_MODIFY):
+		case (MTR_MEMO_PAGE_SX_FIX | MTR_MEMO_MODIFY):
+			break;
+		default:
+			ut_ad(!(slot->type & MTR_MEMO_MODIFY));
+			return true;
+		}
 
-			if (slot->type == MTR_MEMO_PAGE_X_FIX
-			    || slot->type == MTR_MEMO_PAGE_SX_FIX) {
+		ut_ad(m_end_lsn > 0);
+		ut_ad(m_start_lsn > 0);
 
-				add_dirty_page_to_flush_list(slot);
-			}
-		}
+		buf_flush_note_modification(
+			reinterpret_cast<buf_block_t*>(slot->object),
+			m_start_lsn, m_end_lsn, m_flush_observer);
 
 		return(true);
 	}
@@ -814,7 +804,8 @@ struct FindBlockX
   /** @return whether the block was not found x-latched */
   bool operator()(const mtr_memo_slot_t *slot) const
   {
-    return slot->object != &block || slot->type != MTR_MEMO_PAGE_X_FIX;
+    return slot->object != &block ||
+           (slot->type & ~MTR_MEMO_MODIFY) != MTR_MEMO_PAGE_X_FIX;
   }
 };
 
@@ -952,6 +943,17 @@ mtr_t::memo_contains_flagged(const void* ptr, ulint flags) const
 		CIterate<FlaggedCheck>(FlaggedCheck(ptr, flags)));
 }
 
+/** 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 */
+
 /** Check if memo contains the given page.
 @param[in]	ptr	pointer to within buffer frame
 @param[in]	flags	specify types of object with OR of
@@ -968,27 +970,90 @@ mtr_t::memo_contains_page_flagged(
 		? NULL : iteration.functor.get_block();
 }
 
+
+/** Find a block in MTR_MEMO_MODIFY state */
+struct FindModifiedSpacePage
+{
+  mtr_memo_slot_t *found= nullptr;
+  ulint space;
+  ulint page;
+
+  FindModifiedSpacePage(ulint space, ulint page) : space(space), page(page) {}
+
+  bool operator()(mtr_memo_slot_t *slot)
+  {
+    if (!(slot->type & (MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX)))
+      return true;
+
+    const byte *frame= static_cast<buf_block_t *>(slot->object)->frame;
+    uint32_t page_space=
+        mach_read_from_4(frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
+    uint32_t page_no= mach_read_from_4(frame + FIL_PAGE_OFFSET);
+
+    if (page_space != space || page_no != page)
+      return true;
+
+    found= slot;
+    return !(slot->type &
+             (MTR_MEMO_MODIFY | MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX));
+  }
+};
+
 /** Mark the given latched page as modified.
-@param[in]	ptr	pointer to within buffer frame */
-void
-mtr_t::memo_modify_page(const byte* ptr)
+@param[in]	space	page's space id
+@param[in]	page	page number*/
+void mtr_t::memo_modify_page(ulint space, ulint page)
 {
-	buf_block_t*	block = memo_contains_page_flagged(
-		ptr, MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX);
-	ut_ad(block != NULL);
 
-	if (!memo_contains(get_memo(), block, MTR_MEMO_MODIFY)) {
-		memo_push(block, MTR_MEMO_MODIFY);
-	}
+  if (m_memo.empty())
+    return;
+  Iterate<FindModifiedSpacePage> iteration(
+      (FindModifiedSpacePage(space, page)));
+  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);
 }
 
-/** Print info of an mtr handle. */
-void
-mtr_t::print() const
+/** Find a block in MTR_MEMO_MODIFY state */
+struct FindModifiedPtr
 {
-	ib::info() << "Mini-transaction handle: memo size "
-		<< m_memo.size() << " bytes log size "
-		<< get_log()->size() << " bytes";
-}
+  mtr_memo_slot_t *found= nullptr;
+  const byte *ptr;
 
-#endif /* UNIV_DEBUG */
+  FindModifiedPtr(const byte *ptr) : ptr(ptr) {}
+
+  bool operator()(mtr_memo_slot_t *slot)
+  {
+    if (!slot->object ||
+        !(slot->type & (MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX)) ||
+        ptr < static_cast<buf_block_t *>(slot->object)->frame ||
+        ptr >= (static_cast<buf_block_t *>(slot->object)->frame +
+                static_cast<buf_block_t *>(slot->object)->page.size.logical()))
+      return true;
+    found= slot;
+    return !(slot->type &
+             (MTR_MEMO_MODIFY | MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX));
+  }
+};
+
+/** Mark the given latched page as modified.
+@param[in]	ptr	pointer to within buffer frame */
+void mtr_t::memo_modify_page(const byte *ptr)
+{
+  if (m_memo.empty())
+    return;
+
+  Iterate<FindModifiedPtr> iteration((FindModifiedPtr(ptr)));
+  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);
+}
diff --git a/storage/innobase/page/page0cur.cc b/storage/innobase/page/page0cur.cc
index 6810edf6c33..58ec5f58198 100644
--- a/storage/innobase/page/page0cur.cc
+++ b/storage/innobase/page/page0cur.cc
@@ -816,6 +816,7 @@ page_cur_insert_rec_write_log(
 	const byte* log_end;
 	ulint	i;
 
+	mtr->memo_modify_page(insert_rec);
 	if (dict_table_is_temporary(index->table)) {
 		mtr->set_modified();
 		ut_ad(mtr->get_log_mode() == MTR_LOG_NO_REDO);
diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc
index 9664bda6fea..93ee6252177 100644
--- a/storage/innobase/page/page0zip.cc
+++ b/storage/innobase/page/page0zip.cc
@@ -388,6 +388,7 @@ page_zip_compress_write_log(
 	ut_ad(!dict_index_is_ibuf(index));
 
 	log_ptr = mlog_open(mtr, 11 + 2 + 2);
+	mtr->memo_modify_page(page);
 
 	if (!log_ptr) {
 
@@ -4001,6 +4002,8 @@ page_zip_write_blob_ptr(
 	if (mtr) {
 		byte*	log_ptr	= mlog_open(
 			mtr, 11 + 2 + 2 + BTR_EXTERN_FIELD_REF_SIZE);
+		mtr->memo_modify_page((byte *)field);
+
 		if (UNIV_UNLIKELY(!log_ptr)) {
 			return;
 		}
@@ -4139,6 +4142,8 @@ page_zip_write_node_ptr(
 	if (mtr) {
 		byte*	log_ptr	= mlog_open(mtr,
 					    11 + 2 + 2 + REC_NODE_PTR_SIZE);
+		mtr->memo_modify_page(field);
+
 		if (UNIV_UNLIKELY(!log_ptr)) {
 			return;
 		}
@@ -4654,6 +4659,7 @@ page_zip_write_header_log(
 	ut_ad(length > 0);
 	ut_ad(length < 256);
 
+	mtr->memo_modify_page(data);
 	/* If no logging is requested, we may return now */
 	if (UNIV_UNLIKELY(!log_ptr)) {
 
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 6571a7fbfec..0a2db9487a5 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -4480,6 +4480,7 @@ void row_merge_write_redo(const dict_index_t* index)
 	mtr_t mtr;
 	mtr.start();
 	byte* log_ptr = mlog_open(&mtr, 11 + 8);
+	mtr.memo_modify_page(index->space, index->page);
 	log_ptr = mlog_write_initial_log_record_low(
 		MLOG_INDEX_LOAD,
 		index->space, index->page, log_ptr, &mtr);
diff --git a/storage/innobase/row/row0trunc.cc b/storage/innobase/row/row0trunc.cc
index 618e161bee4..db8cca77d25 100644
--- a/storage/innobase/row/row0trunc.cc
+++ b/storage/innobase/row/row0trunc.cc
@@ -446,6 +446,8 @@ public:
 			mtr_start(&mtr);
 
 			log_ptr = mlog_open(&mtr, 11 + 8);
+			mtr.memo_modify_page(m_table->space, 0);
+
 			log_ptr = mlog_write_initial_log_record_low(
 				MLOG_TRUNCATE, m_table->space, 0,
 				log_ptr, &mtr);
diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc
index e3e1c33b305..fd39a273dad 100644
--- a/storage/innobase/trx/trx0rec.cc
+++ b/storage/innobase/trx/trx0rec.cc
@@ -59,6 +59,7 @@ trx_undof_page_add_undo_rec_log(
 	ulint		len;
 
 	log_ptr = mlog_open(mtr, 11 + 13 + MLOG_BUF_MARGIN);
+	mtr->memo_modify_page(undo_page);
 
 	if (log_ptr == NULL) {