Use an MVCC snapshot, rather than SnapshotNow, for catalog scans.

SnapshotNow scans have the undesirable property that, in the face of
concurrent updates, the scan can fail to see either the old or the new
versions of the row.  In many cases, we work around this by requiring
DDL operations to hold AccessExclusiveLock on the object being
modified; in some cases, the existing locking is inadequate and random
failures occur as a result.  This commit doesn't change anything
related to locking, but will hopefully pave the way to allowing lock
strength reductions in the future.

The major issue has held us back from making this change in the past
is that taking an MVCC snapshot is significantly more expensive than
using a static special snapshot such as SnapshotNow.  However, testing
of various worst-case scenarios reveals that this problem is not
severe except under fairly extreme workloads.  To mitigate those
problems, we avoid retaking the MVCC snapshot for each new scan;
instead, we take a new snapshot only when invalidation messages have
been processed.  The catcache machinery already requires that
invalidation messages be sent before releasing the related heavyweight
lock; else other backends might rely on locally-cached data rather
than scanning the catalog at all.  Thus, making snapshot reuse
dependent on the same guarantees shouldn't break anything that wasn't
already subtly broken.

Patch by me.  Review by Michael Paquier and Andres Freund.

1 files changed, 16 insertions, 5 deletions

diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h
index 9e833ca0f88..7e70e57a7eb 100644
--- a/src/include/storage/sinval.h
+++ b/src/include/storage/sinval.h
@@ -24,6 +24,7 @@
  *	* invalidate a relcache entry for a specific logical relation
  *	* invalidate an smgr cache entry for a specific physical relation
  *	* invalidate the mapped-relation mapping for a given database
+ *	* invalidate any saved snapshot that might be used to scan a given relation
  * More types could be added if needed.  The message type is identified by
  * the first "int8" field of the message struct.  Zero or positive means a
  * specific-catcache inval message (and also serves as the catcache ID field).
@@ -43,11 +44,11 @@
  * catcache inval messages must be generated for each of its caches, since
  * the hash keys will generally be different.
  *
- * Catcache and relcache invalidations are transactional, and so are sent
- * to other backends upon commit.  Internally to the generating backend,
- * they are also processed at CommandCounterIncrement so that later commands
- * in the same transaction see the new state.  The generating backend also
- * has to process them at abort, to flush out any cache state it's loaded
+ * Catcache, relcache, and snapshot invalidations are transactional, and so
+ * are sent to other backends upon commit.  Internally to the generating
+ * backend, they are also processed at CommandCounterIncrement so that later
+ * commands in the same transaction see the new state.  The generating backend
+ * also has to process them at abort, to flush out any cache state it's loaded
  * from no-longer-valid entries.
  *
  * smgr and relation mapping invalidations are non-transactional: they are
@@ -98,6 +99,15 @@ typedef struct
 	Oid			dbId;			/* database ID, or 0 for shared catalogs */
 } SharedInvalRelmapMsg;
 
+#define SHAREDINVALSNAPSHOT_ID	(-5)
+
+typedef struct
+{
+	int8		id;				/* type field --- must be first */
+	Oid			dbId;			/* database ID, or 0 if a shared relation */
+	Oid			relId;			/* relation ID */
+} SharedInvalSnapshotMsg;
+
 typedef union
 {
 	int8		id;				/* type field --- must be first */
@@ -106,6 +116,7 @@ typedef union
 	SharedInvalRelcacheMsg rc;
 	SharedInvalSmgrMsg sm;
 	SharedInvalRelmapMsg rm;
+	SharedInvalSnapshotMsg sn;
 } SharedInvalidationMessage;