MDEV-15370 Upgrade fails when both insert_undo and update_undo exist

Before MDEV-12288 in MariaDB 10.3.1, InnoDB used to partition
the persistent transaction undo log into insert_undo and update_undo.

MDEV-12288 repurposes the update_undo as the single undo log.
In order to support an upgrade from earlier MariaDB versions,
the insert_undo is recovered in data structures, called old_insert.

An assertion failure occurred in TrxUndoRsegsIterator::set_next()
when an incomplete transaction was recovered with both insert_undo
and update_undo log. This could be easily demonstrated by starting
./mysql-test-run --manual-gdb innodb.read_only_recovery
in MariaDB 10.2, and after the first kill, start up the MariaDB 10.3
server with the same parameters.

The problem is that MariaDB 10.3 would roll back the recovered
transaction, and finally "commit" it twice (with all changes to
data rolled back), both insert_undo and update_undo with the same
commit end identifier (trx->no).

Our fix is to introduce a "commit number" that comprises two components:
(trx->no << 1 | !old_insert). In this way, the assertion in the purge
subsystem can be relaxed so that only the trx->no component must match.

storage/innobase/include/trx0purge.h

@@ -103,14 +103,16 @@ public:
 	TrxUndoRsegs() {}
 	/** Constructor */
 	TrxUndoRsegs(trx_rseg_t& rseg)
-		: m_trx_no(rseg.last_trx_no), m_rsegs(1, &rseg) {}
+		: m_commit(rseg.last_commit), m_rsegs(1, &rseg) {}
 	/** Constructor */
 	TrxUndoRsegs(trx_id_t trx_no, trx_rseg_t& rseg)
-		: m_trx_no(trx_no), m_rsegs(1, &rseg) {}
+		: m_commit(trx_no << 1), m_rsegs(1, &rseg) {}
 
-	/** Get the commit number */
-	trx_id_t get_trx_no() const { return m_trx_no; }
+	/** @return the transaction commit identifier */
+	trx_id_t trx_no() const { return m_commit >> 1; }
 
+	bool operator!=(const TrxUndoRsegs& other) const
+	{ return m_commit != other.m_commit; }
 	bool empty() const { return m_rsegs.empty(); }
 	void erase(iterator& it) { m_rsegs.erase(it); }
 	iterator begin() { return(m_rsegs.begin()); }
@@ -124,13 +126,12 @@ public:
 	@return true if elem1 > elem2 else false.*/
 	bool operator()(const TrxUndoRsegs& lhs, const TrxUndoRsegs& rhs)
 	{
-		return(lhs.m_trx_no > rhs.m_trx_no);
+		return(lhs.m_commit > rhs.m_commit);
 	}
 
 private:
-	/** Copy trx_rseg_t::last_trx_no */
-	trx_id_t		m_trx_no;
-
+	/** Copy trx_rseg_t::last_commit */
+	trx_id_t		m_commit;
 	/** Rollback segments of a transaction, scheduled for purge. */
 	trx_rsegs_t		m_rsegs;
 };
@@ -438,13 +439,17 @@ public:
 	{
 		bool operator<=(const iterator& other) const
 		{
-			if (trx_no < other.trx_no) return true;
-			if (trx_no > other.trx_no) return false;
+			if (commit < other.commit) return true;
+			if (commit > other.commit) return false;
 			return undo_no <= other.undo_no;
 		}
 
-		/** The trx_t::no of the committed transaction */
-		trx_id_t	trx_no;
+		/** @return the commit number of the transaction */
+		trx_id_t trx_no() const { return commit >> 1; }
+		void reset_trx_no(trx_id_t trx_no) { commit = trx_no << 1; }
+
+		/** 2 * trx_t::no + old_insert of the committed transaction */
+		trx_id_t	commit;
 		/** The record number within the committed transaction's undo
 		log, increasing, purged from from 0 onwards */
 		undo_no_t	undo_no;

storage/innobase/include/trx0rseg.h

@@ -164,8 +164,8 @@ struct trx_rseg_t {
 	/** Byte offset of the last not yet purged log header */
 	ulint				last_offset;
 
-	/** Transaction number of the last not yet purged log */
-	trx_id_t			last_trx_no;
+	/** trx_t::no * 2 + old_insert of the last not yet purged log */
+	trx_id_t			last_commit;
 
 	/** Whether the log segment needs purge */
 	bool				needs_purge;
@@ -177,6 +177,14 @@ struct trx_rseg_t {
 	UNDO-tablespace marked for truncate. */
 	bool				skip_allocation;
 
+	/** @return the commit ID of the last committed transaction */
+	trx_id_t last_trx_no() const { return last_commit >> 1; }
+
+	void set_last_trx_no(trx_id_t trx_no, bool is_update)
+	{
+		last_commit = trx_no << 1 | trx_id_t(is_update);
+	}
+
 	/** @return whether the rollback segment is persistent */
 	bool is_persistent() const
 	{

storage/innobase/lock/lock0lock.cc

@@ -5258,7 +5258,7 @@ lock_print_info_summary(
 	fprintf(file,
 		"Purge done for trx's n:o < " TRX_ID_FMT
 		" undo n:o < " TRX_ID_FMT " state: ",
-		purge_sys->tail.trx_no,
+		purge_sys->tail.trx_no(),
 		purge_sys->tail.undo_no);
 
 	/* Note: We are reading the state without the latch. One because it

storage/innobase/trx/trx0purge.cc

@@ -85,19 +85,17 @@ inline bool TrxUndoRsegsIterator::set_next()
 	/* Check if there are more rsegs to process in the
 	current element. */
 	if (m_iter != m_rsegs.end()) {
-
 		/* We are still processing rollback segment from
 		the same transaction and so expected transaction
 		number shouldn't increase. Undo the increment of
-		expected trx_no done by caller assuming rollback
+		expected commit done by caller assuming rollback
 		segments from given transaction are done. */
-		purge_sys->tail.trx_no = (*m_iter)->last_trx_no;
+		purge_sys->tail.commit = (*m_iter)->last_commit;
 	} else if (!purge_sys->purge_queue.empty()) {
 		m_rsegs = purge_sys->purge_queue.top();
 		purge_sys->purge_queue.pop();
 		ut_ad(purge_sys->purge_queue.empty()
-		      || purge_sys->purge_queue.top().get_trx_no()
-		      != m_rsegs.get_trx_no());
+		      || purge_sys->purge_queue.top() != m_rsegs);
 		m_iter = m_rsegs.begin();
 	} else {
 		/* Queue is empty, reset iterator. */
@@ -113,16 +111,16 @@ inline bool TrxUndoRsegsIterator::set_next()
 	mutex_enter(&purge_sys->rseg->mutex);
 
 	ut_a(purge_sys->rseg->last_page_no != FIL_NULL);
-	ut_ad(purge_sys->rseg->last_trx_no == m_rsegs.get_trx_no());
+	ut_ad(purge_sys->rseg->last_trx_no() == m_rsegs.trx_no());
 
 	/* We assume in purge of externally stored fields that space id is
 	in the range of UNDO tablespace space ids */
 	ut_a(purge_sys->rseg->space == TRX_SYS_SPACE
 	     || srv_is_undo_tablespace(purge_sys->rseg->space));
 
-	ut_a(purge_sys->tail.trx_no <= purge_sys->rseg->last_trx_no);
+	ut_a(purge_sys->tail.commit <= purge_sys->rseg->last_commit);
 
-	purge_sys->tail.trx_no = purge_sys->rseg->last_trx_no;
+	purge_sys->tail.commit = purge_sys->rseg->last_commit;
 	purge_sys->hdr_offset = purge_sys->rseg->last_offset;
 	purge_sys->hdr_page_no = purge_sys->rseg->last_page_no;
 
@@ -306,7 +304,7 @@ trx_purge_add_undo_to_history(const trx_t* trx, trx_undo_t*& undo, mtr_t* mtr)
 	if (rseg->last_page_no == FIL_NULL) {
 		rseg->last_page_no = undo->hdr_page_no;
 		rseg->last_offset = undo->hdr_offset;
-		rseg->last_trx_no = trx->no;
+		rseg->set_last_trx_no(trx->no, undo == trx->rsegs.m_redo.undo);
 		rseg->needs_purge = true;
 	}
 
@@ -462,8 +460,8 @@ func_exit:
 
 	undo_trx_no = mach_read_from_8(log_hdr + TRX_UNDO_TRX_NO);
 
-	if (undo_trx_no >= limit.trx_no) {
-		if (undo_trx_no == limit.trx_no) {
+	if (undo_trx_no >= limit.trx_no()) {
+		if (undo_trx_no == limit.trx_no()) {
 			trx_undo_truncate_start(
 				&rseg, hdr_addr.page,
 				hdr_addr.boffset, limit.undo_no);
@@ -933,7 +931,7 @@ trx_purge_initiate_truncate(
 			     undo != NULL && all_free;
 			     undo = UT_LIST_GET_NEXT(undo_list, undo)) {
 
-				if (limit.trx_no < undo->trx_id) {
+				if (limit.trx_no() < undo->trx_id) {
 					all_free = false;
 				} else {
 					cached_undo_size += undo->size;
@@ -1040,12 +1038,12 @@ function is called, the caller must not have any latches on undo log pages!
 static void trx_purge_truncate_history()
 {
 	ut_ad(purge_sys->head <= purge_sys->tail);
-	purge_sys_t::iterator& head = purge_sys->head.trx_no
+	purge_sys_t::iterator& head = purge_sys->head.commit
 		? purge_sys->head : purge_sys->tail;
 
-	if (head.trx_no >= purge_sys->view.low_limit_no()) {
+	if (head.trx_no() >= purge_sys->view.low_limit_no()) {
 		/* This is sometimes necessary. TODO: find out why. */
-		head.trx_no = purge_sys->view.low_limit_no();
+		head.reset_trx_no(purge_sys->view.low_limit_no());
 		head.undo_no = 0;
 	}
 
@@ -1086,7 +1084,7 @@ trx_purge_rseg_get_next_history_log(
 
 	ut_a(rseg->last_page_no != FIL_NULL);
 
-	purge_sys->tail.trx_no = rseg->last_trx_no + 1;
+	purge_sys->tail.commit = rseg->last_commit + 1;
 	purge_sys->tail.undo_no = 0;
 	purge_sys->next_stored = false;
 
@@ -1136,7 +1134,7 @@ trx_purge_rseg_get_next_history_log(
 
 	rseg->last_page_no = prev_log_addr.page;
 	rseg->last_offset = prev_log_addr.boffset;
-	rseg->last_trx_no = trx_no;
+	rseg->set_last_trx_no(trx_no, purge != 0);
 	rseg->needs_purge = purge != 0;
 
 	/* Purge can also produce events, however these are already ordered
@@ -1235,7 +1233,7 @@ trx_purge_get_next_rec(
 	mtr_t		mtr;
 
 	ut_ad(purge_sys->next_stored);
-	ut_ad(purge_sys->tail.trx_no < purge_sys->view.low_limit_no());
+	ut_ad(purge_sys->tail.trx_no() < purge_sys->view.low_limit_no());
 
 	space = purge_sys->rseg->space;
 	page_no = purge_sys->page_no;
@@ -1330,7 +1328,7 @@ trx_purge_fetch_next_rec(
 		}
 	}
 
-	if (purge_sys->tail.trx_no >= purge_sys->view.low_limit_no()) {
+	if (purge_sys->tail.trx_no() >= purge_sys->view.low_limit_no()) {
 
 		return(NULL);
 	}

storage/innobase/trx/trx0rseg.cc

@@ -470,13 +470,13 @@ trx_rseg_mem_restore(
 			max_trx_id = id;
 		}
 		id = mach_read_from_8(undo_log_hdr + TRX_UNDO_TRX_NO);
-		rseg->last_trx_no = id;
 		if (id > max_trx_id) {
 			max_trx_id = id;
 		}
 		unsigned purge = mach_read_from_2(
 			undo_log_hdr + TRX_UNDO_NEEDS_PURGE);
 		ut_ad(purge <= 1);
+		rseg->set_last_trx_no(id, purge != 0);
 		rseg->needs_purge = purge != 0;
 
 		if (rseg->last_page_no != FIL_NULL) {

storage/innobase/trx/trx0undo.cc

@@ -1761,7 +1761,7 @@ trx_undo_truncate_tablespace(
 		rseg->trx_ref_count = 0;
 		rseg->last_page_no = FIL_NULL;
 		rseg->last_offset = 0;
-		rseg->last_trx_no = 0;
+		rseg->last_commit = 0;
 		rseg->needs_purge = false;
 	}
 	mtr_commit(&mtr);