pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Andres Freund <andres@anarazel.de>
Subject: [PATCH v1 2/2] Assert in init_toast_snapshot() that some snapshot registered or active.
Date: Sat, 19 Feb 2022 12:42:37 -0800

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
---
 src/include/utils/snapmgr.h                 |  1 +
 src/backend/access/common/toast_internals.c |  9 +++++++
 src/backend/utils/time/snapmgr.c            | 26 +++++++++++++++++++++
 3 files changed, 36 insertions(+)

inline patch 1: src/include/utils/snapmgr.h, src/backend/access/common/toast_internals.c, src/backend/utils/time/snapmgr.c (+36/-0 lines) download inline-1.patch
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 293c753034a..e04018c034f 100644
--- 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,
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index de37f561cad..7052ac99780 100644
--- 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
index a0b703a5195..a0b81bf1549 100644
--- 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.
-- 
2.34.0


--msua7xqlaegyczpx--

Message-ID: <no-message-id-30864@localhost>
Permalink:  ../no-message-id-30864@localhost/
Also on:    postgresql.org/message-id/no-message-id-30864@localhost

copy link · copy postgr.es · copy git am

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: andres@anarazel.de
  Subject: Re: [PATCH v1 2/2] Assert in init_toast_snapshot() that some snapshot registered or active.
  In-Reply-To: <no-message-id-30864@localhost>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox