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.

Author: dr-m

storage/innobase/include/trx0purge.h

@@ -103,14 +103,16 @@ class TrxUndoRsegs {
 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 @@ class TrxUndoRsegs {
 @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_tm_trx_no;
-
+/** Copy trx_rseg_t::last_commit */
+trx_id_tm_commit;
 /** Rollback segments of a transaction, scheduled for purge. */
 trx_rsegs_tm_rsegs;
 };
@@ -438,13 +439,17 @@ class purge_sys_t
 {
 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_ttrx_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_tcommit;
 /** The record number within the committed transaction's undo
 log, increasing, purged from from 0 onwards */
 undo_no_tundo_no;

storage/innobase/include/trx0rseg.h

@@ -164,8 +164,8 @@ struct trx_rseg_t {
 /** Byte offset of the last not yet purged log header */
 ulintlast_offset;
 
-/** Transaction number of the last not yet purged log */
-trx_id_tlast_trx_no;
+/** trx_t::no * 2 + old_insert of the last not yet purged log */
+trx_id_tlast_commit;
 
 /** Whether the log segment needs purge */
 boolneeds_purge;
@@ -177,6 +177,14 @@ struct trx_rseg_t {
 UNDO-tablespace marked for truncate. */
 boolskip_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 @@ trx_purge_truncate_rseg_history(
 
 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_tmtr;
 
 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);