Refactor TrxUndoRsegsIterator for further simplification

TrxUndoRsegs::append(): Remove.

TrxUndoRsegsIterator::set_next(): Add a debug assertion that
demonstrates that the merging of rollback segments never occurs.
Since MDEV-12289 or earlier, MariaDB 10.2 will not make any
temporary undo log accessible to the purge subsystem.
(Also MySQL 5.7 would skip the purge of any undo log for
temporary tables, but not before parsing and buffering those
temporary undo log records.)

2 files changed, 31 insertions, 82 deletions

diff --git a/storage/innobase/include/trx0purge.h b/storage/innobase/include/trx0purge.h
index 124177a89ed..f0c9fa436c3 100644
--- a/storage/innobase/include/trx0purge.h
+++ b/storage/innobase/include/trx0purge.h
@@ -97,10 +97,10 @@ private:
 		trx_rsegs_t;
 public:
 	typedef trx_rsegs_t::iterator iterator;
+	typedef trx_rsegs_t::const_iterator const_iterator;
 
 	/** Default constructor */
-	TrxUndoRsegs(trx_id_t trx_no = 0) : m_trx_no(trx_no) {}
-
+	TrxUndoRsegs() {}
 	/** Constructor */
 	TrxUndoRsegs(trx_rseg_t& rseg)
 		: m_trx_no(rseg.last_trx_no), m_rsegs(1, &rseg) {}
@@ -108,28 +108,15 @@ public:
 	TrxUndoRsegs(trx_id_t trx_no, trx_rseg_t& rseg)
 		: m_trx_no(trx_no), m_rsegs(1, &rseg) {}
 
-	/** Get transaction number
-	@return trx_id_t - get transaction number. */
-	trx_id_t get_trx_no() const
-	{
-		return(m_trx_no);
-	}
+	/** Get the commit number */
+	trx_id_t get_trx_no() const { return m_trx_no; }
 
 	bool empty() const { return m_rsegs.empty(); }
 	void erase(iterator& it) { m_rsegs.erase(it); }
 	iterator begin() { return(m_rsegs.begin()); }
 	iterator end() { return(m_rsegs.end()); }
-
-	/** Append rollback segments from referred instance to current
-	instance. */
-	void append(const TrxUndoRsegs& append_from)
-	{
-		ut_ad(get_trx_no() == append_from.get_trx_no());
-
-		m_rsegs.insert(m_rsegs.end(),
-			       append_from.m_rsegs.begin(),
-			       append_from.m_rsegs.end());
-	}
+	const_iterator begin() const { return m_rsegs.begin(); }
+	const_iterator end() const { return m_rsegs.end(); }
 
 	/** Compare two TrxUndoRsegs based on trx_no.
 	@param elem1 first element to compare
@@ -141,7 +128,7 @@ public:
 	}
 
 private:
-	/** The rollback segments transaction number. */
+	/** Copy trx_rseg_t::last_trx_no */
 	trx_id_t		m_trx_no;
 
 	/** Rollback segments of a transaction, scheduled for purge. */
@@ -153,14 +140,12 @@ typedef std::priority_queue<
 	std::vector<TrxUndoRsegs, ut_allocator<TrxUndoRsegs> >,
 	TrxUndoRsegs>	purge_pq_t;
 
-/**
-Chooses the rollback segment with the smallest trx_no. */
+/** Chooses the rollback segment with the oldest committed transaction */
 struct TrxUndoRsegsIterator {
-
 	/** Constructor */
-	TrxUndoRsegsIterator();
-
+	inline TrxUndoRsegsIterator();
 	/** Sets the next rseg to purge in purge_sys.
+	Executed in the purge coordinator thread.
 	@return whether anything is to be purged */
 	inline bool set_next();
 
@@ -170,13 +155,9 @@ private:
 	TrxUndoRsegsIterator& operator=(const TrxUndoRsegsIterator&);
 
 	/** The current element to process */
-	TrxUndoRsegs			m_trx_undo_rsegs;
-
-	/** Track the current element in m_trx_undo_rseg */
-	TrxUndoRsegs::iterator		m_iter;
-
-	/** Sentinel value */
-	static const TrxUndoRsegs	NullElement;
+	TrxUndoRsegs			m_rsegs;
+	/** Track the current element in m_rsegs */
+	TrxUndoRsegs::const_iterator	m_iter;
 };
 
 /* Namespace to hold all the related functions and variables need for truncate
diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc
index 3f2f1ea0dba..ae69452ad1c 100644
--- a/storage/innobase/trx/trx0purge.cc
+++ b/storage/innobase/trx/trx0purge.cc
@@ -64,21 +64,18 @@ my_bool		srv_purge_view_update_only_debug;
 #endif /* UNIV_DEBUG */
 
 /** Sentinel value */
-const TrxUndoRsegs TrxUndoRsegsIterator::NullElement(TRX_ID_MAX);
+static const TrxUndoRsegs NullElement;
 
-/** Constructor */
-TrxUndoRsegsIterator::TrxUndoRsegsIterator()
-	:
-	m_trx_undo_rsegs(NullElement),
-	m_iter(m_trx_undo_rsegs.end())
+/** Default constructor */
+inline TrxUndoRsegsIterator::TrxUndoRsegsIterator()
+	: m_rsegs(NullElement), m_iter(m_rsegs.begin())
 {
 }
 
 /** Sets the next rseg to purge in purge_sys.
+Executed in the purge coordinator thread.
 @return whether anything is to be purged */
-inline
-bool
-TrxUndoRsegsIterator::set_next()
+inline bool TrxUndoRsegsIterator::set_next()
 {
 	mutex_enter(&purge_sys->pq_mutex);
 
@@ -87,65 +84,36 @@ TrxUndoRsegsIterator::set_next()
 
 	/* Check if there are more rsegs to process in the
 	current element. */
-	if (m_iter != m_trx_undo_rsegs.end()) {
+	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 increment of
+		number shouldn't increase. Undo the increment of
 		expected trx_no done by caller assuming rollback
 		segments from given transaction are done. */
 		purge_sys->tail.trx_no = (*m_iter)->last_trx_no;
-
 	} else if (!purge_sys->purge_queue.empty()) {
-
-		/* Read the next element from the queue.
-		Combine elements if they have same transaction number.
-		This can happen if a transaction shares redo rollback segment
-		with another transaction that has already added it to purge
-		queue and former transaction also needs to schedule non-redo
-		rollback segment for purge. */
-		m_trx_undo_rsegs = NullElement;
-
-		purge_pq_t&	purge_queue = purge_sys->purge_queue;
-
-		while (!purge_queue.empty()) {
-
-			if (m_trx_undo_rsegs.get_trx_no() == TRX_ID_MAX) {
-				m_trx_undo_rsegs = purge_queue.top();
-			} else if (purge_queue.top().get_trx_no() ==
-					m_trx_undo_rsegs.get_trx_no()) {
-				m_trx_undo_rsegs.append(
-					purge_queue.top());
-			} else {
-				break;
-			}
-
-			purge_queue.pop();
-		}
-
-		m_iter = m_trx_undo_rsegs.begin();
-
+		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());
+		m_iter = m_rsegs.begin();
 	} else {
 		/* Queue is empty, reset iterator. */
-		m_trx_undo_rsegs = NullElement;
-		m_iter = m_trx_undo_rsegs.end();
-
-		mutex_exit(&purge_sys->pq_mutex);
-
 		purge_sys->rseg = NULL;
+		mutex_exit(&purge_sys->pq_mutex);
+		m_rsegs = NullElement;
+		m_iter = m_rsegs.begin();
 		return false;
 	}
 
 	purge_sys->rseg = *m_iter++;
-
 	mutex_exit(&purge_sys->pq_mutex);
-
-	ut_a(purge_sys->rseg != NULL);
-
 	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_trx_undo_rsegs.get_trx_no());
+	ut_ad(purge_sys->rseg->last_trx_no == m_rsegs.get_trx_no());
 
 	/* We assume in purge of externally stored fields that space id is
 	in the range of UNDO tablespace space ids */