0007-assert-checking-sequence-hash-20230730.patch

From e1f153e405bbc105271bbd98bc08aa662c602b37 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 29 Jul 2023 18:54:45 +0200
Subject: [PATCH 7/8] assert checking sequence hash

---
 .../replication/logical/reorderbuffer.c       | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 34e8590528..83ac3a6a4c 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -266,6 +266,7 @@ static void ReorderBufferTransferSnapToParent(ReorderBufferTXN *txn,
 											  ReorderBufferTXN *subtxn);
 
 static void AssertTXNLsnOrder(ReorderBuffer *rb);
+static void AssertCheckSequences(ReorderBuffer *rb);
 
 /* ---------------------------------------
  * support functions for lsn-order iterating over the ->changes of a
@@ -1009,6 +1010,8 @@ ReorderBufferSequenceIsTransactional(ReorderBuffer *rb,
 	bool		found = false;
 	dlist_iter	iter;
 
+	AssertCheckSequences(rb);
+
 	/*
 	 * Walk all top-level transactions (some of which may be subxacts, except
 	 * that we haven't processed the assignments yet), and check if any of
@@ -1056,6 +1059,8 @@ ReorderBufferSequenceGetXid(ReorderBuffer *rb,
 	bool		found = true;
 	dlist_iter	iter;
 
+	AssertCheckSequences(rb);
+
 	/*
 	 * Walk all top-level transactions (some of which may be subxacts, except
 	 * that we haven't processed the assignments yet), and check if any of
@@ -1183,6 +1188,8 @@ ReorderBufferQueueSequence(ReorderBuffer *rb, TransactionId xid,
 						   RelFileLocator rlocator, bool transactional,
 						   ReorderBufferTupleBuf *tuplebuf)
 {
+	AssertCheckSequences(rb);
+
 	/*
 	 * Change needs to be handled as transactional, because the sequence was
 	 * created in a transaction that is still running. In that case all the
@@ -1336,6 +1343,8 @@ ReorderBufferAddRelFileLocator(ReorderBuffer *rb, TransactionId xid,
 	ReorderBufferSequenceEnt   *entry;
 	ReorderBufferTXN   *txn;
 
+	AssertCheckSequences(rb);
+
 	/*
 	 * We only care about sequence relfilenodes for now, and those always have
 	 * a XID. So if there's no XID, don't bother adding them to the hash.
@@ -1389,6 +1398,8 @@ ReorderBufferAddRelFileLocator(ReorderBuffer *rb, TransactionId xid,
 	Assert(!found);
 
 	entry->txn = txn;
+
+	AssertCheckSequences(rb);
 }
 
 /*
@@ -1465,6 +1476,123 @@ AssertTXNLsnOrder(ReorderBuffer *rb)
 #endif
 }
 
+static void
+AssertCheckSequences(ReorderBuffer *rb)
+{
+#ifdef USE_ASSERT_CHECKING
+	LogicalDecodingContext *ctx = rb->private_data;
+	dlist_iter	iter;
+
+	/*
+	 * Skip the verification if we don't reach the LSN at which we start
+	 * decoding the contents of transactions yet because until we reach the
+	 * LSN, we could have transactions that don't have the association between
+	 * the top-level transaction and subtransaction yet and consequently have
+	 * the same LSN.  We don't guarantee this association until we try to
+	 * decode the actual contents of transaction. The ordering of the records
+	 * prior to the start_decoding_at LSN should have been checked before the
+	 * restart.
+	 */
+	if (SnapBuildXactNeedsSkip(ctx->snapshot_builder, ctx->reader->EndRecPtr))
+		return;
+
+	/*
+	 * Make sure the relfilenodes from subxacts are properly recorded in the
+	 * parent transaction hash table.
+	 */
+	dlist_foreach(iter, &rb->toplevel_by_lsn)
+	{
+		int			nentries = 0,
+					nsubentries = 0;
+		dlist_iter	subiter;
+		ReorderBufferTXN *txn = dlist_container(ReorderBufferTXN, node,
+													iter.cur);
+
+		if (txn->sequences_hash)
+			nentries = hash_get_num_entries(txn->sequences_hash);
+
+		/* walk all subxacts */
+		dlist_foreach(subiter, &txn->subtxns)
+		{
+			HASH_SEQ_STATUS scan_status;
+			ReorderBufferSequenceEnt *entry;
+
+			ReorderBufferTXN *subtxn = dlist_container(ReorderBufferTXN, node,
+													   subiter.cur);
+
+			if (!subtxn->sequences_hash)
+				continue;
+
+			nsubentries += hash_get_num_entries(subtxn->sequences_hash);
+
+			/*
+			 * Check that all subxact relfilenodes are in the parent too, and
+			 * are pointing to this subtransaction.
+			 */
+			hash_seq_init(&scan_status, subtxn->sequences_hash);
+			while ((entry = (ReorderBufferSequenceEnt *) hash_seq_search(&scan_status)) != NULL)
+			{
+				bool	found = false;
+				ReorderBufferSequenceEnt *entry2;
+
+				/* search for the same relfilenode in the parent */
+				entry2 = hash_search(txn->sequences_hash,
+									 (void *) &entry->rlocator,
+									 HASH_FIND,
+									 &found);
+
+				/*
+				 * The parent hash should have the relfilenode too, and it should
+				 * point to this subxact.
+				 */
+				Assert(found);
+				Assert(entry2->txn == subtxn);
+			}
+		}
+
+		Assert(nentries >= nsubentries);
+
+		/*
+		 * Now do the check in the opposite direction - check that every entry in
+		 * the parent hash (except those pointing to the parent txn) point to one
+		 * of the subxacts, and there's an entry in the subxact hash.
+		 */
+		if (txn->sequences_hash)
+		{
+			HASH_SEQ_STATUS scan_status;
+			ReorderBufferSequenceEnt *entry;
+
+			hash_seq_init(&scan_status, txn->sequences_hash);
+			while ((entry = (ReorderBufferSequenceEnt *) hash_seq_search(&scan_status)) != NULL)
+			{
+				bool	found = false;
+				ReorderBufferSequenceEnt *entry2;
+
+				/* skip entries for the parent txn itself */
+				if (entry->txn == txn)
+					continue;
+
+				/* is it a subxact of this txn? */
+				Assert(rbtxn_is_known_subxact(entry->txn));
+				Assert(entry->txn->toptxn == txn);
+
+				/*
+				 * Search for the same relfilenode in the subxact (it should be
+				 * initialized, as we expect it to contain the relfilenode).
+				 */
+				entry2 = hash_search(entry->txn->sequences_hash,
+									 (void *) &entry->rlocator,
+									 HASH_FIND,
+									 &found);
+
+				Assert(found);
+				Assert(entry2->txn = entry->txn);
+			}
+		}
+	}
+#endif
+}
+
 /*
  * AssertChangeLsnOrder
  *
@@ -3479,6 +3607,8 @@ ReorderBufferAbort(ReorderBuffer *rb, TransactionId xid, XLogRecPtr lsn,
 
 	/* remove potential on-disk data, and deallocate */
 	ReorderBufferCleanupTXN(rb, txn);
+
+	AssertCheckSequences(rb);
 }
 
 /*
@@ -3516,6 +3646,8 @@ ReorderBufferAbortOld(ReorderBuffer *rb, TransactionId oldestRunningXid)
 
 			/* remove potential on-disk data, and deallocate this tx */
 			ReorderBufferCleanupTXN(rb, txn);
+
+			AssertCheckSequences(rb);
 		}
 		else
 			return;
-- 
2.41.0