Assert in init_toast_snapshot() that some snapshot registered or active.

anarazel · anarazel · commit 2776922201f7 · 2022-02-21T08:58:29.000-08:00
Commit <FIXME> fixed the bug that RemoveTempRelationsCallback() did not push/register a snapshot. That only went unnoticed because often a valid catalog snapshot exists and is returned by GetOldestSnapshot(). But due to invalidation processing that is not reliable. Thus assert in init_toast_snapshot() that there is a registered or active snapshot, using the new HaveRegisteredOrActiveSnapshot(). Author: Andres Freund Discussion: https://postgr.es/m/20220219180002.6tubjq7iw7m52bgd@alap3.anarazel.de
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
@@ -660,5 +660,14 @@ init_toast_snapshot(Snapshot toast_snapshot)
 	if (snapshot == NULL)
 		elog(ERROR, "cannot fetch toast data without an active snapshot");
 
+	/*
+	 * Catalog snapshots can be returned by GetOldestSnapshot() even if not
+	 * registered or active. That easily hides bugs around not having a
+	 * snapshot set up - most of the time there is a valid catalog
+	 * snapshot. So additionally insist that the current snapshot is
+	 * registered or active.
+	 */
+	Assert(HaveRegisteredOrActiveSnapshot());
+
 	InitToastSnapshot(*toast_snapshot, snapshot->lsn, snapshot->whenTaken);
 }
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
@@ -1625,6 +1625,32 @@ ThereAreNoPriorRegisteredSnapshots(void)
 	return false;
 }
 
+/*
+ * HaveRegisteredOrActiveSnapshots
+ *		Is there any registered or active snapshot?
+ *
+ * NB: Unless pushed or active, the cached catalog snapshot will not cause
+ * this function to return true. That allows this function to be used in
+ * checks enforcing a longer-lived snapshot.
+ */
+bool
+HaveRegisteredOrActiveSnapshot(void)
+{
+	if (ActiveSnapshot != NULL)
+		return true;
+
+	/*
+	 * The catalog snapshot is in RegisteredSnapshots when valid, but can be
+	 * removed at any time due to invalidation processing. If explicitly
+	 * registered more than one snapshot has to be in RegisteredSnapshots.
+	 */
+	if (pairingheap_is_empty(&RegisteredSnapshots) ||
+		!pairingheap_is_singular(&RegisteredSnapshots))
+		return false;
+
+	return CatalogSnapshot == NULL;
+}
+
 
 /*
  * Return a timestamp that is exactly on a minute boundary.
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
@@ -135,6 +135,7 @@ extern bool XactHasExportedSnapshots(void);
 extern void DeleteAllExportedSnapshotFiles(void);
 extern void WaitForOlderSnapshots(TransactionId limitXmin, bool progress);
 extern bool ThereAreNoPriorRegisteredSnapshots(void);
+extern bool HaveRegisteredOrActiveSnapshot(void);
 extern bool TransactionIdLimitedForOldSnapshots(TransactionId recentXmin,
 												Relation relation,
 												TransactionId *limit_xid,
