MDEV-32489 Change buffer index fails to delete the records

When the change buffer records for a page span across multiple change
buffer leaf pages or the starting record is at the beginning of a page
with a left sibling, ibuf_delete_recs deletes only the records in first
page and fails to move to subsequent pages.

Subsequently a slow shutdown hangs trying to delete those left over
records.

Fix-A: Position the cursor to an user record in B-tree and exit only
when all records are exhausted.

Fix-B: Make sure we call ibuf_delete_recs during slow shutdown for
pages with IBUF entries to cleanup any previously left over records.

storage/innobase/btr/btr0pcur.cc

@@ -657,8 +657,9 @@ user record satisfying the search condition, in the case PAGE_CUR_L or
 PAGE_CUR_LE, on the last user record. If no such user record exists, then
 in the first case sets the cursor after last in tree, and in the latter case
 before first in tree. The latching mode must be BTR_SEARCH_LEAF or
-BTR_MODIFY_LEAF. */
-void
+BTR_MODIFY_LEAF.
+@return DB_SUCCESS or error code */
+dberr_t
 btr_pcur_open_on_user_rec_func(
 /*===========================*/
 	dict_index_t*	index,		/*!< in: index */
@@ -672,8 +673,12 @@ btr_pcur_open_on_user_rec_func(
 	unsigned	line,		/*!< in: line where called */
 	mtr_t*		mtr)		/*!< in: mtr */
 {
-	btr_pcur_open_low(index, 0, tuple, mode, latch_mode, cursor,
-			  file, line, 0, mtr);
+	auto err = btr_pcur_open_low(index, 0, tuple, mode, latch_mode, cursor,
+				     file, line, 0, mtr);
+
+	if (UNIV_UNLIKELY(err != DB_SUCCESS)) {
+		return err;
+	}
 
 	if ((mode == PAGE_CUR_GE) || (mode == PAGE_CUR_G)) {
 
@@ -688,4 +693,5 @@ btr_pcur_open_on_user_rec_func(
 
 		ut_error;
 	}
+	return DB_SUCCESS;
 }

storage/innobase/ibuf/ibuf0ibuf.cc

@@ -24,6 +24,7 @@ Insert buffer
 Created 7/19/1997 Heikki Tuuri
 *******************************************************/
 
+#include <tuple>
 #include "ibuf0ibuf.h"
 #include "sync0sync.h"
 #include "btr0sea.h"
@@ -2331,9 +2332,11 @@ static void ibuf_delete_recs(const page_id_t page_id)
 loop:
   btr_pcur_t pcur;
   ibuf_mtr_start(&mtr);
-  if (btr_pcur_open(ibuf.index, &tuple, PAGE_CUR_GE, BTR_MODIFY_LEAF,
-                    &pcur, &mtr) != DB_SUCCESS)
+  if (btr_pcur_open_on_user_rec(ibuf.index, &tuple, PAGE_CUR_GE,
+                                BTR_MODIFY_LEAF, &pcur, &mtr) != DB_SUCCESS)
+  {
     goto func_exit;
+  }
   if (!btr_pcur_is_on_user_rec(&pcur))
   {
     ut_ad(btr_pcur_is_after_last_on_page(&pcur));
@@ -2370,7 +2373,8 @@ static void ibuf_delete_recs(const page_id_t page_id)
 
 /** Merge the change buffer to some pages. */
 static void ibuf_read_merge_pages(const uint32_t* space_ids,
-				  const uint32_t* page_nos, ulint n_stored)
+				  const uint32_t* page_nos, ulint n_stored,
+				  bool slow_shutdown_cleanup)
 {
 	for (ulint i = 0; i < n_stored; i++) {
 		const ulint space_id = space_ids[i];
@@ -2394,26 +2398,25 @@ static void ibuf_read_merge_pages(const uint32_t* space_ids,
 		if (UNIV_LIKELY(page_nos[i] < size)) {
 			mtr.start();
 			dberr_t err;
-			buf_block_t *block =
+			/* Load the page and apply change buffers. */
+			std::ignore =
 			buf_page_get_gen(page_id_t(space_id, page_nos[i]),
 					 zip_size, RW_X_LATCH, nullptr,
 					 BUF_GET_POSSIBLY_FREED,
 					 __FILE__, __LINE__, &mtr, &err, true);
-			bool remove = !block
-				|| fil_page_get_type(block->frame)
-				!= FIL_PAGE_INDEX
-				|| !page_is_leaf(block->frame);
 			mtr.commit();
 			if (err == DB_TABLESPACE_DELETED) {
 				goto tablespace_deleted;
 			}
-			if (!remove) {
-				continue;
-			}
 		}
 
-		if (srv_shutdown_state == SRV_SHUTDOWN_NONE
-		    || srv_fast_shutdown) {
+		/* During slow shutdown cleanup, we apply all pending IBUF
+		changes and need to cleanup any left-over IBUF records. There
+		are a few cases when the changes are already discarded and IBUF
+		bitmap is cleaned but we miss to delete the record. Even after
+		the issues are fixed, we need to keep this safety measure for
+		upgraded DBs with such left over records. */
+		if (!slow_shutdown_cleanup) {
 			continue;
 		}
 
@@ -2445,10 +2448,11 @@ static void ibuf_read_merge_pages(const uint32_t* space_ids,
 }
 
 /** Contract the change buffer by reading pages to the buffer pool.
+@param slow_shutdown true, if called during slow shutdown
 @return a lower limit for the combined size in bytes of entries which
 will be merged from ibuf trees to the pages read
 @retval 0 if ibuf.empty */
-ulint ibuf_contract()
+ulint ibuf_contract(bool slow_shutdown)
 {
 	if (UNIV_UNLIKELY(!ibuf.index)) return 0;
 	mtr_t		mtr;
@@ -2492,7 +2496,7 @@ ulint ibuf_contract()
 	ibuf_mtr_commit(&mtr);
 	btr_pcur_close(&pcur);
 
-	ibuf_read_merge_pages(space_ids, page_nos, n_pages);
+	ibuf_read_merge_pages(space_ids, page_nos, n_pages, slow_shutdown);
 
 	return(sum_sizes + 1);
 }
@@ -2574,7 +2578,7 @@ ibuf_merge_space(
 		}
 #endif /* UNIV_DEBUG */
 
-		ibuf_read_merge_pages(spaces, pages, n_pages);
+		ibuf_read_merge_pages(spaces, pages, n_pages, false);
 	}
 
 	return(n_pages);
@@ -2599,7 +2603,7 @@ ibuf_contract_after_insert(
 	ulint size;
 
 	do {
-		size = ibuf_contract();
+		size = ibuf_contract(false);
 		sum_sizes += size;
 	} while (size > 0 && sum_sizes < entry_size);
 }
@@ -3228,7 +3232,7 @@ ibuf_insert_low(
 #ifdef UNIV_IBUF_DEBUG
 		fputs("Ibuf too big\n", stderr);
 #endif
-		ibuf_contract();
+		ibuf_contract(false);
 
 		return(DB_STRONG_FAIL);
 	}
@@ -3478,7 +3482,7 @@ ibuf_insert_low(
 #ifdef UNIV_IBUF_DEBUG
 		ut_a(n_stored <= IBUF_MAX_N_PAGES_MERGED);
 #endif
-		ibuf_read_merge_pages(space_ids, page_nos, n_stored);
+		ibuf_read_merge_pages(space_ids, page_nos, n_stored, false);
 	}
 
 	return(err);

storage/innobase/include/btr0pcur.h

@@ -181,8 +181,9 @@ user record satisfying the search condition, in the case PAGE_CUR_L or
 PAGE_CUR_LE, on the last user record. If no such user record exists, then
 in the first case sets the cursor after last in tree, and in the latter case
 before first in tree. The latching mode must be BTR_SEARCH_LEAF or
-BTR_MODIFY_LEAF. */
-void
+BTR_MODIFY_LEAF.
+@return DB_SUCCESS or error code */
+dberr_t
 btr_pcur_open_on_user_rec_func(
 /*===========================*/
 	dict_index_t*	index,		/*!< in: index */

storage/innobase/include/ibuf0ibuf.h

@@ -370,10 +370,11 @@ in DISCARD TABLESPACE, IMPORT TABLESPACE, or read-ahead.
 void ibuf_delete_for_discarded_space(ulint space);
 
 /** Contract the change buffer by reading pages to the buffer pool.
+@param slow_shutdown true, if called during slow shutdown
 @return a lower limit for the combined size in bytes of entries which
 will be merged from ibuf trees to the pages read
 @retval 0 if ibuf.empty */
-ulint ibuf_contract();
+ulint ibuf_contract(bool slow_shutdown);
 
 /** Contracts insert buffer trees by reading pages referring to space_id
 to the buffer pool.

storage/innobase/srv/srv0srv.cc

@@ -1665,7 +1665,7 @@ void srv_shutdown(bool ibuf_merge)
 			ibuf_read_merge_pages() */
 			ibuf_max_size_update(0);
 			log_free_check();
-			n_read = ibuf_contract();
+			n_read = ibuf_contract(true);
 		}
 
 		if (n_tables_to_drop || ibuf_merge) {