MDEV-16168: Performance regression after MDEV-12288

The function row_purge_reset_trx_id() that had been introduced in
commit 3c09f14 (MDEV-12288)
introduces some extra buffer pool and redo log activity that will
cause a significant performance regression under some workloads.

This is currently the most significant performance issue, after
commit acd071f (MDEV-21923)
fixed the InnoDB LSN allocation and MDEV-19749 the MDL bottleneck in 12.1.

The purpose of row_purge_reset_trx_id() was to ensure that we can
easily identify records for which no history exists. If DB_TRX_ID
is 0, we could avoid looking up the transaction to see if the
history is accessible or the record is implicitly locked.

To avoid trx_sys_t::find() for stale DB_TRX_ID values, we can refer
to trx_t::max_inactive_id, which was introduced in
commit 4105017 (MDEV-30357).
Instead of comparing DB_TRX_ID to 0, we may compare it to this
cached value. The cache would be updated by
trx_sys_t::find_same_or_older(), which is invoked for some operations
on secondary indexes.

row_purge_reset_trx_id(): Remove. We will no longer reset the
DB_TRX_ID to 0 after an INSERT. We will retain a single undo log
for all operations, though. Before MDEV-12288, there had been
separate insert_undo and update_undo logs.

row_check_index(): No longer warn
"InnoDB: Clustered index record with stale history in table".

lock_rec_queue_validate(), lock_rec_convert_impl_to_expl(),
row_vers_impl_x_locked_low(): Instead of comparing the DB_TRX_ID
to 0, compare it to trx_t::max_inactive_id.

In dict0load.cc we will not spend any effort to avoid extra
trx_sys.find() calls for stale DB_TRX_ID in dictionary tables.
This code does not currently use trx_t objects, and therefore
we cannot easily access trx_t::max_inactive_id. Loading table
definitions into the InnoDB data dictionary cache (dict_sys)
should be a very rare operation.

Reviewed by: Vladislav Lesin

Author: dr-m

mysql-test/suite/innodb/r/dml_purge.result

@@ -13,8 +13,8 @@ SYS_TABLES clustered index root page (8):
 N_RECS=8; LEVEL=0; INDEX_ID=0x0000000000000001
 header=0x01000003008d (NAME=0x696e66696d756d00)
 header=0x0000101500d5 (NAME='SYS_FOREIGN',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x000000000000000b,
  N_COLS=0x00000004,
  TYPE=0x00000001,
@@ -23,8 +23,8 @@ header=0x0000101500d5 (NAME='SYS_FOREIGN',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000000)
 header=0x000018150122 (NAME='SYS_FOREIGN_COLS',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x000000000000000c,
  N_COLS=0x00000004,
  TYPE=0x00000001,
@@ -33,8 +33,8 @@ header=0x000018150122 (NAME='SYS_FOREIGN_COLS',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000000)
 header=0x0000201501ae (NAME='SYS_VIRTUAL',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x000000000000000d,
  N_COLS=0x00000003,
  TYPE=0x00000001,
@@ -43,8 +43,8 @@ header=0x0000201501ae (NAME='SYS_VIRTUAL',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000000)
 header=0x0400301501f2 (NAME='test/tc',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x000000000000000f,
  N_COLS=0x80000001,
  TYPE=0x00000001,
@@ -53,8 +53,8 @@ header=0x0400301501f2 (NAME='test/tc',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000002)
 header=0x00003815027a (NAME='test/td',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x0000000000000010,
  N_COLS=0x80000001,
  TYPE=0x00000021,
@@ -63,8 +63,8 @@ header=0x00003815027a (NAME='test/td',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000003)
 header=0x00004815016a (NAME='test/tp',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x0000000000000012,
  N_COLS=0x80000001,
  TYPE=0x000009a1,
@@ -73,8 +73,8 @@ header=0x00004815016a (NAME='test/tp',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000005)
 header=0x000028150236 (NAME='test/tr',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x000000000000000e,
  N_COLS=0x00000001,
  TYPE=0x00000001,
@@ -83,8 +83,8 @@ header=0x000028150236 (NAME='test/tr',
  CLUSTER_NAME=NULL(0 bytes),
  SPACE=0x00000001)
 header=0x000040150074 (NAME='test/tz',
- DB_TRX_ID=0x000000000000,
- DB_ROLL_PTR=0x80000000000000,
+ DB_TRX_ID=0xXXXXXXXXXXXX,
+ DB_ROLL_PTR=0xXXXXXXXXXXXXXX,
  ID=0x0000000000000011,
  N_COLS=0x80000001,
  TYPE=0x00000023,
@@ -156,6 +156,7 @@ BEGIN;
 INSERT INTO tr VALUES(1);
 INSERT INTO tc VALUES(1);
 INSERT INTO td VALUES(1);
+INSERT INTO tz VALUES(1);
 INSERT INTO tp VALUES(1);
 ROLLBACK;
 SELECT * FROM tr;

mysql-test/suite/innodb/r/table_flags.result

@@ -90,7 +90,9 @@ for (my $offset= 0x65; $offset;
 	my $end= unpack("C", substr($page, $offset-7-$i, 1));
 	print ",\n " if $i;
 	print "$fields[$i]=";
-	if ($end & 0x80) {
+	if ($i == 1 || $i == 2) {
+	    print "0x", "XX" x ($end-$start);
+	} elsif ($end & 0x80) {
 	    print "NULL(", ($end & 0x7f) - $start, " bytes)"
 	} elsif ($n_fields > 1 && $i == 0) {
 	    $name= substr($page,$offset+$start,$end-$start);
@@ -211,10 +213,7 @@ BEGIN;
 INSERT INTO tr VALUES(1);
 INSERT INTO tc VALUES(1);
 INSERT INTO td VALUES(1);
-# We cannot access tz, because due to our fiddling of the NO_ROLLBACK flag,
-# it now has a record with DB_TRX_ID=0, which is invalid for
-# transactional tables until MDEV-12288 is implemented.
-# INSERT INTO tz VALUES(1);
+INSERT INTO tz VALUES(1);
 INSERT INTO tp VALUES(1);
 ROLLBACK;
 

mysql-test/suite/innodb/t/dml_purge.test

@@ -638,7 +638,8 @@ struct trx_t : ilist_node<>
   {
     /** The largest encountered transaction identifier for which no
     transaction was observed to be active. This is a cache to speed up
-    trx_sys_t::find_same_or_older().
+    trx_sys_t::find_same_or_older() as well as to elide some calls to
+    trx_sys_t::find().
 
     This will be zero-initialized in Pool::Pool() and not initialized
     when a transaction object in the pool is freed and reused. The

mysql-test/suite/innodb/t/table_flags.test

@@ -5409,8 +5409,10 @@ lock_rec_queue_validate(
 		? lock_clust_rec_some_has_impl(rec, index, offsets)
 		: 0;
 
-	if (trx_t *impl_trx = impl_trx_id
-	    ? trx_sys.find(current_trx(), impl_trx_id, false)
+	trx_t *trx= current_trx();
+
+	if (trx_t *impl_trx = impl_trx_id > (trx ? trx->max_inactive_id : 0)
+	    ? trx_sys.find(trx, impl_trx_id, false)
 	    : 0) {
 		/* impl_trx could have been committed before we
 		acquire its mutex, but not thereafter. */
@@ -5990,7 +5992,7 @@ lock_rec_convert_impl_to_expl(
 
 		trx_id = lock_clust_rec_some_has_impl(rec, index, offsets);
 
-		if (trx_id == 0) {
+		if (trx_id <= caller_trx->max_inactive_id) {
 			return nullptr;
 		}
 		if (UNIV_UNLIKELY(trx_id == caller_trx->id)) {

storage/innobase/include/trx0trx.h

@@ -2411,8 +2411,7 @@ row_merge_read_clustered_index(
 		} else if (rec_trx_id < trx->id) {
 			/* Reset the DB_TRX_ID,DB_ROLL_PTR of old rows
 			for which history is not going to be
-			available after the rebuild operation.
-			This essentially mimics row_purge_reset_trx_id(). */
+			available after the rebuild operation. */
 			row->fields[new_trx_id_col].data
 				= const_cast<byte*>(reset_trx_id);
 			row->fields[new_trx_id_col + 1].data

storage/innobase/lock/lock0lock.cc

@@ -1080,78 +1080,6 @@ row_purge_del_mark(
   return result;
 }
 
-/** Reset DB_TRX_ID, DB_ROLL_PTR of a clustered index record
-whose old history can no longer be observed.
-@param[in,out]	node	purge node
-@param[in,out]	mtr	mini-transaction (will be started and committed) */
-static void row_purge_reset_trx_id(purge_node_t* node, mtr_t* mtr)
-{
-	/* Reset DB_TRX_ID, DB_ROLL_PTR for old records. */
-	mtr->start();
-
-	if (row_purge_reposition_pcur(BTR_MODIFY_LEAF, node, mtr)) {
-		dict_index_t*	index = dict_table_get_first_index(
-			node->table);
-		ulint	trx_id_pos = index->n_uniq ? index->n_uniq : 1;
-		rec_t*	rec = btr_pcur_get_rec(&node->pcur);
-		mem_heap_t*	heap = NULL;
-		/* Reserve enough offsets for the PRIMARY KEY and 2 columns
-		so that we can access DB_TRX_ID, DB_ROLL_PTR. */
-		rec_offs offsets_[REC_OFFS_HEADER_SIZE + MAX_REF_PARTS + 3];
-		rec_offs_init(offsets_);
-		rec_offs*	offsets = rec_get_offsets(
-			rec, index, offsets_, index->n_core_fields,
-			trx_id_pos + 2, &heap);
-		ut_ad(heap == NULL);
-
-		ut_ad(dict_index_get_nth_field(index, trx_id_pos)
-		      ->col->mtype == DATA_SYS);
-		ut_ad(dict_index_get_nth_field(index, trx_id_pos)
-		      ->col->prtype == (DATA_TRX_ID | DATA_NOT_NULL));
-		ut_ad(dict_index_get_nth_field(index, trx_id_pos + 1)
-		      ->col->mtype == DATA_SYS);
-		ut_ad(dict_index_get_nth_field(index, trx_id_pos + 1)
-		      ->col->prtype == (DATA_ROLL_PTR | DATA_NOT_NULL));
-
-		/* Only update the record if DB_ROLL_PTR matches (the
-		record has not been modified after this transaction
-		became purgeable) */
-		if (node->roll_ptr
-		    == row_get_rec_roll_ptr(rec, index, offsets)) {
-			ut_ad(!rec_get_deleted_flag(
-					rec, rec_offs_comp(offsets))
-			      || rec_is_alter_metadata(rec, *index));
-			DBUG_LOG("purge", "reset DB_TRX_ID="
-				 << ib::hex(row_get_rec_trx_id(
-						    rec, index, offsets)));
-
-			index->set_modified(*mtr);
-			buf_block_t* block = btr_pcur_get_block(&node->pcur);
-			if (UNIV_LIKELY_NULL(block->page.zip.data)) {
-				page_zip_write_trx_id_and_roll_ptr(
-					block, rec, offsets, trx_id_pos,
-					0, 1ULL << ROLL_PTR_INSERT_FLAG_POS,
-					mtr);
-			} else {
-				ulint	len;
-				byte*	ptr = rec_get_nth_field(
-					rec, offsets, trx_id_pos, &len);
-				ut_ad(len == DATA_TRX_ID_LEN);
-				size_t offs = ptr - block->page.frame;
-				mtr->memset(block, offs, DATA_TRX_ID_LEN, 0);
-				offs += DATA_TRX_ID_LEN;
-				mtr->write<1,mtr_t::MAYBE_NOP>(
-					*block, block->page.frame + offs,
-					0x80U);
-				mtr->memset(block, offs + 1,
-					    DATA_ROLL_PTR_LEN - 1, 0);
-			}
-		}
-	}
-
-	mtr->commit();
-}
-
 /***********************************************************//**
 Purges an update of an existing record. Also purges an update of a delete
 marked record if that record contained an externally stored field. */
@@ -1281,8 +1209,6 @@ row_purge_upd_exist_or_extern_func(
 			mtr.commit();
 		}
 	}
-
-	row_purge_reset_trx_id(node, &mtr);
 }
 
 #ifdef UNIV_DEBUG
@@ -1578,8 +1504,6 @@ row_purge_record_func(
 		/* fall through */
 	default:
 		if (!updated_extern) {
-			mtr_t		mtr;
-			row_purge_reset_trx_id(node, &mtr);
 			break;
 		}
 		/* fall through */