From 076c589dff7e08f0a6b562b185f179da4fbfc13a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 6 Apr 2020 21:28:55 -0700
Subject: [PATCH v7 02/11] Improve and extend asserts for a snapshot being set.

---
 src/include/utils/snapmgr.h        |  2 ++
 src/backend/access/heap/heapam.c   |  6 ++++--
 src/backend/access/index/indexam.c |  8 +++++++-
 src/backend/catalog/indexing.c     | 11 +++++++++++
 src/backend/utils/time/snapmgr.c   | 19 +++++++++++++++++++
 contrib/amcheck/verify_nbtree.c    |  6 +++---
 6 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index b28d13ce841..7738d6a8e01 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -116,6 +116,8 @@ extern void PopActiveSnapshot(void);
 extern Snapshot GetActiveSnapshot(void);
 extern bool ActiveSnapshotSet(void);
 
+extern bool SnapshotSet(void);
+
 extern Snapshot RegisterSnapshot(Snapshot snapshot);
 extern void UnregisterSnapshot(Snapshot snapshot);
 extern Snapshot RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner);
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index c4a5aa616a3..0af51880ccc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1136,6 +1136,8 @@ heap_beginscan(Relation relation, Snapshot snapshot,
 {
 	HeapScanDesc scan;
 
+	Assert(SnapshotSet());
+
 	/*
 	 * increment relation ref count while scanning relation
 	 *
@@ -1545,7 +1547,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
 	at_chain_start = first_call;
 	skip = !first_call;
 
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 	Assert(BufferGetBlockNumber(buffer) == blkno);
 
 	/* Scan through possible multiple members of HOT-chain */
@@ -5633,7 +5635,7 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
 	 * if so (vacuum can't subsequently move relfrozenxid to beyond
 	 * TransactionXmin, so there's no race here).
 	 */
-	Assert(TransactionIdIsValid(TransactionXmin));
+	Assert(SnapshotSet() && TransactionIdIsValid(TransactionXmin));
 	if (TransactionIdPrecedes(TransactionXmin, relation->rd_rel->relfrozenxid))
 		prune_xid = relation->rd_rel->relfrozenxid;
 	else
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index a3f77169a79..5d6354dedf5 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -184,6 +184,8 @@ index_insert(Relation indexRelation,
 	RELATION_CHECKS;
 	CHECK_REL_PROCEDURE(aminsert);
 
+	Assert(SnapshotSet());
+
 	if (!(indexRelation->rd_indam->ampredlocks))
 		CheckForSerializableConflictIn(indexRelation,
 									   (ItemPointer) NULL,
@@ -256,6 +258,8 @@ index_beginscan_internal(Relation indexRelation,
 {
 	IndexScanDesc scan;
 
+	Assert(SnapshotSet());
+
 	RELATION_CHECKS;
 	CHECK_REL_PROCEDURE(ambeginscan);
 
@@ -519,7 +523,7 @@ index_getnext_tid(IndexScanDesc scan, ScanDirection direction)
 	SCAN_CHECKS;
 	CHECK_SCAN_PROCEDURE(amgettuple);
 
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 
 	/*
 	 * The AM's amgettuple proc finds the next index entry matching the scan
@@ -574,6 +578,8 @@ index_fetch_heap(IndexScanDesc scan, TupleTableSlot *slot)
 	bool		all_dead = false;
 	bool		found;
 
+	Assert(SnapshotSet());
+
 	found = table_index_fetch_tuple(scan->xs_heapfetch, &scan->xs_heaptid,
 									scan->xs_snapshot, slot,
 									&scan->xs_heap_continue, &all_dead);
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index d63fcf58cf1..8ba6b3dfa5e 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -22,6 +22,7 @@
 #include "catalog/indexing.h"
 #include "executor/executor.h"
 #include "utils/rel.h"
+#include "utils/snapmgr.h"
 
 
 /*
@@ -184,6 +185,8 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
 {
 	CatalogIndexState indstate;
 
+	Assert(SnapshotSet());
+
 	indstate = CatalogOpenIndexes(heapRel);
 
 	simple_heap_insert(heapRel, tup);
@@ -204,6 +207,8 @@ void
 CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
 						   CatalogIndexState indstate)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_insert(heapRel, tup);
 
 	CatalogIndexInsert(indstate, tup);
@@ -225,6 +230,8 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
 {
 	CatalogIndexState indstate;
 
+	Assert(SnapshotSet());
+
 	indstate = CatalogOpenIndexes(heapRel);
 
 	simple_heap_update(heapRel, otid, tup);
@@ -245,6 +252,8 @@ void
 CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
 						   CatalogIndexState indstate)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_update(heapRel, otid, tup);
 
 	CatalogIndexInsert(indstate, tup);
@@ -268,5 +277,7 @@ CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
 void
 CatalogTupleDelete(Relation heapRel, ItemPointer tid)
 {
+	Assert(SnapshotSet());
+
 	simple_heap_delete(heapRel, tid);
 }
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index b5cff157bf6..3b148ae30a6 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -857,6 +857,25 @@ ActiveSnapshotSet(void)
 	return ActiveSnapshot != NULL;
 }
 
+/*
+ * Does this transaction have a snapshot.
+ */
+bool
+SnapshotSet(void)
+{
+	/* can't be safe, because somehow xmin is not set */
+	if (!TransactionIdIsValid(MyPgXact->xmin) && HistoricSnapshot == NULL)
+		return false;
+
+	/*
+	 * Can't be safe because no snapshot being active/registered means that
+	 * e.g. invalidation processing could change xmin horizon.
+	 */
+	return ActiveSnapshot != NULL ||
+		!pairingheap_is_empty(&RegisteredSnapshots) ||
+		HistoricSnapshot != NULL;
+}
+
 /*
  * RegisterSnapshot
  *		Register a snapshot as being in use by the current resource owner
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index ceaaa271680..8f43f3e9dfb 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -412,10 +412,10 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace,
 	Snapshot	snapshot = SnapshotAny;
 
 	/*
-	 * RecentGlobalXmin assertion matches index_getnext_tid().  See note on
-	 * RecentGlobalXmin/B-Tree page deletion.
+	 * This assertion matches the one in index_getnext_tid().  See page
+	 * recycling/RecentGlobalXmin notes in nbtree README.
 	 */
-	Assert(TransactionIdIsValid(RecentGlobalXmin));
+	Assert(SnapshotSet());
 
 	/*
 	 * Initialize state for entire verification operation
-- 
2.25.0.114.g5b0ca878e0

