MDEV-12353: Exclusively use page_zip_reorganize() for ROW_FORMAT=COMPRESSED

page_zip_reorganize(): Restore the page on failure.
In callers, omit now-redundant calls to page_zip_decompress().

btr_page_reorganize_low(): Define in static scope only, and
remove the z_level parameter. Assert that ROW_FORMAT is not COMPRESSED.

btr_page_reorganize_block(), btr_page_reorganize(): Invoke
page_zip_reorganize() for ROW_FORMAT=COMPRESSED.

storage/innobase/btr/btr0btr.cc

@@ -1365,44 +1365,30 @@ btr_write_autoinc(dict_index_t* index, ib_uint64_t autoinc, bool reset)
 	mtr.commit();
 }
 
-/*************************************************************//**
+/** Reorganize an index page. */
-Reorganizes an index page.
+static void
-
-IMPORTANT: On success, the caller will have to update IBUF_BITMAP_FREE
-if this is a compressed leaf page in a secondary index. This has to
-be done either within the same mini-transaction, or by invoking
-ibuf_reset_free_bits() before mtr_commit(). On uncompressed pages,
-IBUF_BITMAP_FREE is unaffected by reorganization.
-
-@retval true if the operation was successful
-@retval false if it is a compressed page, and recompression failed */
-bool
 btr_page_reorganize_low(
-/*====================*/
 	bool		recovery,/*!< in: true if called in recovery:
 				locks should not be updated, i.e.,
 				there cannot exist locks on the
 				page, and a hash index should not be
 				dropped: it cannot exist */
-	ulint		z_level,/*!< in: compression level to be used
-				if dealing with compressed page */
 	page_cur_t*	cursor,	/*!< in/out: page cursor */
 	dict_index_t*	index,	/*!< in: the index tree of the page */
 	mtr_t*		mtr)	/*!< in/out: mini-transaction */
 {
 	buf_block_t*	block		= page_cur_get_block(cursor);
 	page_t*		page		= buf_block_get_frame(block);
-	page_zip_des_t*	page_zip	= buf_block_get_page_zip(block);
 	buf_block_t*	temp_block;
 	ulint		data_size1;
 	ulint		data_size2;
 	ulint		max_ins_size1;
 	ulint		max_ins_size2;
-	bool		success		= false;
 	ulint		pos;
 	bool		is_spatial;
 
 	ut_ad(mtr_memo_contains(mtr, block, MTR_MEMO_PAGE_X_FIX));
+	ut_ad(!is_buf_block_get_page_zip(block));
 	btr_assert_not_corrupted(block, index);
 	ut_ad(fil_page_index_page_check(block->frame));
 	ut_ad(index->is_dummy
@@ -1410,9 +1396,6 @@ btr_page_reorganize_low(
 	ut_ad(index->is_dummy
 	      || block->page.id.page_no() != index->page
 	      || !page_has_siblings(page));
-#ifdef UNIV_ZIP_DEBUG
-	ut_a(!page_zip || page_zip_validate(page_zip, page, index));
-#endif /* UNIV_ZIP_DEBUG */
 	data_size1 = page_get_data_size(page);
 	max_ins_size1 = page_get_max_insert_size_after_reorganize(page, 1);
 	/* Turn logging off */
@@ -1476,50 +1459,16 @@ btr_page_reorganize_low(
 		  ? page_is_leaf(temp_block->frame)
 		  : block->page.id.page_no() == index->page));
 
-	if (page_zip
-	    && !page_zip_compress(block, index, z_level, mtr)) {
-
-		/* Restore the old page and exit. */
-#if defined UNIV_DEBUG || defined UNIV_ZIP_DEBUG
-		/* Check that the bytes that we skip are identical. */
-		ut_a(!memcmp(page, temp_block->frame, PAGE_HEADER));
-		ut_a(!memcmp(PAGE_HEADER + PAGE_N_RECS + block->frame,
-			     PAGE_HEADER + PAGE_N_RECS + temp_block->frame,
-			     PAGE_DATA - (PAGE_HEADER + PAGE_N_RECS)));
-		ut_a(!memcmp(srv_page_size - FIL_PAGE_DATA_END
-			     + block->frame,
-			     srv_page_size - FIL_PAGE_DATA_END
-			     + temp_block->frame, FIL_PAGE_DATA_END));
-#endif /* UNIV_DEBUG || UNIV_ZIP_DEBUG */
-
-		memcpy_aligned<2>(PAGE_HEADER + block->frame,
-				  PAGE_HEADER + temp_block->frame,
-				  PAGE_N_RECS - PAGE_N_DIR_SLOTS);
-		memcpy(PAGE_DATA + block->frame, PAGE_DATA + temp_block->frame,
-		       srv_page_size - PAGE_DATA - FIL_PAGE_DATA_END);
-
-#if defined UNIV_DEBUG || defined UNIV_ZIP_DEBUG
-		ut_a(!memcmp(block->frame, temp_block->frame, srv_page_size));
-#endif /* UNIV_DEBUG || UNIV_ZIP_DEBUG */
-
-		goto func_exit;
-	}
-
 	data_size2 = page_get_data_size(block->frame);
 	max_ins_size2 = page_get_max_insert_size_after_reorganize(block->frame,
 								  1);
 
 	if (data_size1 != data_size2 || max_ins_size1 != max_ins_size2) {
-		ib::error()
+		ib::fatal()
 			<< "Page old data size " << data_size1
 			<< " new data size " << data_size2
 			<< ", page old max ins size " << max_ins_size1
 			<< " new max ins size " << max_ins_size2;
-
-		ib::error() << BUG_REPORT_MSG;
-		ut_ad(0);
-	} else {
-		success = true;
 	}
 
 	/* Restore the cursor position. */
@@ -1529,16 +1478,11 @@ btr_page_reorganize_low(
 		ut_ad(cursor->rec == page_get_infimum_rec(block->frame));
 	}
 
-#ifdef UNIV_ZIP_DEBUG
-	ut_a(!page_zip || page_zip_validate(page_zip, block->frame, index));
-#endif /* UNIV_ZIP_DEBUG */
-
 	if (!recovery) {
 		if (block->page.id.page_no() == index->page
 		    && fil_page_get_type(temp_block->frame)
 		    == FIL_PAGE_TYPE_INSTANT) {
 			/* Preserve the PAGE_INSTANT information. */
-			ut_ad(!page_zip);
 			ut_ad(index->is_instant());
 			static_assert(!(FIL_PAGE_TYPE % 2), "alignment");
 			memcpy_aligned<2>(FIL_PAGE_TYPE + block->frame,
@@ -1568,38 +1512,19 @@ btr_page_reorganize_low(
 		}
 	}
 
-func_exit:
 	buf_block_free(temp_block);
 
 	/* Restore logging mode */
 	mtr_set_log_mode(mtr, log_mode);
 
-	if (success) {
+	mlog_open_and_write_index(mtr, page, index, page_is_comp(page)
-		mlog_id_t	type;
+				  ? MLOG_COMP_PAGE_REORGANIZE
-
+				  : MLOG_PAGE_REORGANIZE, 0);
-		/* Write the log record */
+	MONITOR_INC(MONITOR_INDEX_REORG_SUCCESSFUL);
-		if (page_zip) {
-			ut_ad(page_is_comp(page));
-			type = MLOG_ZIP_PAGE_REORGANIZE;
-		} else if (page_is_comp(page)) {
-			type = MLOG_COMP_PAGE_REORGANIZE;
-		} else {
-			type = MLOG_PAGE_REORGANIZE;
-		}
-
-		if (byte* log_ptr = mlog_open_and_write_index(
-			    mtr, page, index, type, page_zip ? 1 : 0)) {
-			*log_ptr++ = z_level;
-			mlog_close(mtr, log_ptr);
-		}
-
-		MONITOR_INC(MONITOR_INDEX_REORG_SUCCESSFUL);
-	}
 
 	if (UNIV_UNLIKELY(fil_page_get_type(block->frame)
 			  == FIL_PAGE_TYPE_INSTANT)) {
 		/* Log the PAGE_INSTANT information. */
-		ut_ad(!page_zip);
 		ut_ad(index->is_instant());
 		ut_ad(!recovery);
 		mtr->write<2,mtr_t::FORCED>(*block, FIL_PAGE_TYPE
@@ -1617,8 +1542,6 @@ func_exit:
 			mtr->memcpy(*block, PAGE_OLD_SUPREMUM, 8);
 		}
 	}
-
-	return(success);
 }
 
 /*************************************************************//**
@@ -1646,10 +1569,15 @@ btr_page_reorganize_block(
 	dict_index_t*	index,	/*!< in: the index tree of the page */
 	mtr_t*		mtr)	/*!< in/out: mini-transaction */
 {
+	if (buf_block_get_page_zip(block)) {
+		return page_zip_reorganize(block, index, z_level, mtr, true);
+	}
+
 	page_cur_t	cur;
 	page_cur_set_before_first(block, &cur);
 
-	return(btr_page_reorganize_low(recovery, z_level, &cur, index, mtr));
+	btr_page_reorganize_low(recovery, &cur, index, mtr);
+	return true;
 }
 
 /*************************************************************//**
@@ -1670,8 +1598,24 @@ btr_page_reorganize(
 	dict_index_t*	index,	/*!< in: the index tree of the page */
 	mtr_t*		mtr)	/*!< in/out: mini-transaction */
 {
-	return(btr_page_reorganize_low(false, page_zip_level,
+	if (!buf_block_get_page_zip(cursor->block)) {
-				       cursor, index, mtr));
+		btr_page_reorganize_low(false, cursor, index, mtr);
+		return true;
+	}
+
+	ulint pos = page_rec_get_n_recs_before(cursor->rec);
+	if (!page_zip_reorganize(cursor->block, index, page_zip_level, mtr,
+				 true)) {
+		return false;
+	}
+	if (pos) {
+		cursor->rec = page_rec_get_nth(cursor->block->frame, pos);
+	} else {
+		ut_ad(cursor->rec == page_get_infimum_rec(
+			      cursor->block->frame));
+	}
+
+	return true;
 }
 
 /***********************************************************//**

storage/innobase/btr/btr0scrub.cc

@@ -349,7 +349,7 @@ btr_scrub_skip_page(
 }
 
 /****************************************************************
-Try to scrub a page using btr_page_reorganize_low
+Try to scrub a page.
 return DB_SUCCESS on success or DB_OVERFLOW on failure */
 static
 dberr_t
@@ -369,12 +369,8 @@ btr_optimistic_scrub(
 		return DB_OVERFLOW;
 	}
 #endif
-
+	if (!btr_page_reorganize_block(false, scrub_compression_level, block,
-	page_cur_t cur;
+				       index, mtr)) {
-	page_cur_set_before_first(block, &cur);
-	bool recovery = false;
-	if (!btr_page_reorganize_low(recovery, scrub_compression_level,
-				     &cur, index, mtr)) {
 		return DB_OVERFLOW;
 	}
 

storage/innobase/gis/gis0rtree.cc

@@ -892,7 +892,8 @@ rtr_split_page_move_rec_list(
 			ret_pos == 0. */
 
 			if (UNIV_UNLIKELY
-			    (!page_zip_reorganize(new_block, index, mtr))) {
+			    (!page_zip_reorganize(new_block, index,
+						  page_zip_level, mtr))) {
 
 				if (UNIV_UNLIKELY
 				    (!page_zip_decompress(new_page_zip,

storage/innobase/include/btr0btr.h

@@ -434,31 +434,6 @@ be done either within the same mini-transaction, or by invoking
 ibuf_reset_free_bits() before mtr_commit(). On uncompressed pages,
 IBUF_BITMAP_FREE is unaffected by reorganization.
 
-@retval true if the operation was successful
-@retval false if it is a compressed page, and recompression failed */
-bool
-btr_page_reorganize_low(
-/*====================*/
-	bool		recovery,/*!< in: true if called in recovery:
-				locks should not be updated, i.e.,
-				there cannot exist locks on the
-				page, and a hash index should not be
-				dropped: it cannot exist */
-	ulint		z_level,/*!< in: compression level to be used
-				if dealing with compressed page */
-	page_cur_t*	cursor,	/*!< in/out: page cursor */
-	dict_index_t*	index,	/*!< in: the index tree of the page */
-	mtr_t*		mtr)	/*!< in/out: mini-transaction */
-	MY_ATTRIBUTE((warn_unused_result));
-/*************************************************************//**
-Reorganizes an index page.
-
-IMPORTANT: On success, the caller will have to update IBUF_BITMAP_FREE
-if this is a compressed leaf page in a secondary index. This has to
-be done either within the same mini-transaction, or by invoking
-ibuf_reset_free_bits() before mtr_commit(). On uncompressed pages,
-IBUF_BITMAP_FREE is unaffected by reorganization.
-
 @retval true if the operation was successful
 @retval false if it is a compressed page, and recompression failed */
 bool

storage/innobase/include/page0zip.h

@@ -415,17 +415,18 @@ IMPORTANT: if page_zip_reorganize() is invoked on a leaf page of a
 non-clustered index, the caller must update the insert buffer free
 bits in the same mini-transaction in such a way that the modification
 will be redo-logged.
-@return TRUE on success, FALSE on failure; page_zip will be left
+@retval true on success
-intact on failure, but page will be overwritten. */
+@retval false on failure; the block_zip will be left intact */
-ibool
+bool
 page_zip_reorganize(
-/*================*/
 	buf_block_t*	block,	/*!< in/out: page with compressed page;
 				on the compressed page, in: size;
 				out: data, n_blobs,
 				m_start, m_end, m_nonempty */
 	dict_index_t*	index,	/*!< in: index of the B-tree node */
-	mtr_t*		mtr)	/*!< in: mini-transaction */
+	ulint		z_level,/*!< in: compression level */
+	mtr_t*		mtr,	/*!< in: mini-transaction */
+	bool		restore = false)/*!< whether to restore on failure */
 	MY_ATTRIBUTE((nonnull));
 
 /**********************************************************************//**

storage/innobase/page/page0cur.cc

@@ -1663,9 +1663,22 @@ page_cur_insert_rec_zip(
 		} else if (!page_zip->m_nonempty && !page_has_garbage(page)) {
 			/* The page has been freshly compressed, so
 			reorganizing it will not help. */
-		} else if (btr_page_reorganize_low(
+		} else {
-				   recv_recovery_is_on(), level,
+			ulint pos = page_rec_get_n_recs_before(cursor->rec);
-				   cursor, index, mtr)) {
+
+			if (!page_zip_reorganize(page_cur_get_block(cursor),
+						 index, level, mtr, true)) {
+				ut_ad(cursor->rec == cursor_rec);
+				return NULL;
+			}
+
+			if (pos) {
+				cursor->rec = page_rec_get_nth(page, pos);
+			} else {
+				ut_ad(cursor->rec == page_get_infimum_rec(
+					      page));
+			}
+
 			ut_ad(!page_header_get_ptr(page, PAGE_FREE));
 
 			if (page_zip_available(
@@ -1675,9 +1688,6 @@ page_cur_insert_rec_zip(
 				available. */
 				goto use_heap;
 			}
-		} else {
-			ut_ad(cursor->rec == cursor_rec);
-			return(NULL);
 		}
 
 		/* Try compressing the whole page afterwards. */
@@ -1723,7 +1733,8 @@ page_cur_insert_rec_zip(
 			/* We are writing entire page images to the
 			log.  Reduce the redo log volume by
 			reorganizing the page at the same time. */
-			if (page_zip_reorganize(cursor->block, index, mtr)) {
+			if (page_zip_reorganize(cursor->block, index,
+						level, mtr)) {
 				/* The page was reorganized: Seek to pos. */
 				if (pos > 1) {
 					cursor->rec = page_rec_get_nth(

storage/innobase/page/page0page.cc

@@ -662,7 +662,8 @@ page_copy_rec_list_end(
 			that is smaller than "ret"). */
 			ut_a(ret_pos > 0);
 
-			if (!page_zip_reorganize(new_block, index, mtr)) {
+			if (!page_zip_reorganize(new_block, index,
+						 page_zip_level, mtr)) {
 
 				if (!page_zip_decompress(new_page_zip,
 							 new_page, FALSE)) {
@@ -678,11 +679,7 @@ page_copy_rec_list_end(
 			} else {
 				/* The page was reorganized:
 				Seek to ret_pos. */
-				ret = new_page + PAGE_NEW_INFIMUM;
+				ret = page_rec_get_nth(new_page, ret_pos);
-
-				do {
-					ret = rec_get_next_ptr(ret, TRUE);
-				} while (--ret_pos);
 			}
 		}
 	}
@@ -832,7 +829,8 @@ zip_reorganize:
 			ret_pos == 0. */
 
 			if (UNIV_UNLIKELY
-			    (!page_zip_reorganize(new_block, index, mtr))) {
+			    (!page_zip_reorganize(new_block, index,
+						  page_zip_level, mtr))) {
 
 				if (UNIV_UNLIKELY
 				    (!page_zip_decompress(new_page_zip,

storage/innobase/page/page0zip.cc

@@ -4682,17 +4682,18 @@ IMPORTANT: if page_zip_reorganize() is invoked on a leaf page of a
 non-clustered index, the caller must update the insert buffer free
 bits in the same mini-transaction in such a way that the modification
 will be redo-logged.
-@return TRUE on success, FALSE on failure; page_zip will be left
+@retval true on success
-intact on failure, but page will be overwritten. */
+@retval false on failure; the block will be left intact */
-ibool
+bool
 page_zip_reorganize(
-/*================*/
 	buf_block_t*	block,	/*!< in/out: page with compressed page;
 				on the compressed page, in: size;
 				out: data, n_blobs,
 				m_start, m_end, m_nonempty */
 	dict_index_t*	index,	/*!< in: index of the B-tree node */
-	mtr_t*		mtr)	/*!< in: mini-transaction */
+	ulint		z_level,/*!< in: compression level */
+	mtr_t*		mtr,	/*!< in: mini-transaction */
+	bool		restore)/*!< whether to restore on failure */
 {
 	page_t*		page		= buf_block_get_frame(block);
 	buf_block_t*	temp_block;
@@ -4746,15 +4747,39 @@ page_zip_reorganize(
 	/* Restore logging. */
 	mtr_set_log_mode(mtr, log_mode);
 
-	if (!page_zip_compress(block, index, page_zip_level, mtr)) {
+	if (!page_zip_compress(block, index, z_level, mtr)) {
+		if (restore) {
+			/* Restore the old page and exit. */
+#if defined UNIV_DEBUG || defined UNIV_ZIP_DEBUG
+			/* Check that the bytes that we skip are identical. */
+			ut_a(!memcmp(page, temp_page, PAGE_HEADER));
+			ut_a(!memcmp(PAGE_HEADER + PAGE_N_RECS + page,
+				     PAGE_HEADER + PAGE_N_RECS + temp_page,
+				     PAGE_DATA - (PAGE_HEADER + PAGE_N_RECS)));
+			ut_a(!memcmp(srv_page_size - FIL_PAGE_DATA_END + page,
+				     srv_page_size - FIL_PAGE_DATA_END
+				     + temp_page,
+				     FIL_PAGE_DATA_END));
+#endif /* UNIV_DEBUG || UNIV_ZIP_DEBUG */
+
+			memcpy(PAGE_HEADER + page, PAGE_HEADER + temp_page,
+			       PAGE_N_RECS - PAGE_N_DIR_SLOTS);
+			memcpy(PAGE_DATA + page, PAGE_DATA + temp_page,
+			       srv_page_size - PAGE_DATA - FIL_PAGE_DATA_END);
+
+#if defined UNIV_DEBUG || defined UNIV_ZIP_DEBUG
+			ut_a(!memcmp(page, temp_page, srv_page_size));
+#endif /* UNIV_DEBUG || UNIV_ZIP_DEBUG */
+		}
+
 		buf_block_free(temp_block);
-		return(FALSE);
+		return false;
 	}
 
 	lock_move_reorganize_page(block, temp_block);
 
 	buf_block_free(temp_block);
-	return(TRUE);
+	return true;
 }
 
 /**********************************************************************//**