Remove unnecessary GetTransactionSnapshot() calls

In get_database_list() and get_subscription_list(), the
GetTransactionSnapshot() call is not required because the catalog
table scans use the catalog snapshot, which is held until the end of
the scan. See table_beginscan_catalog(), which calls
RegisterSnapshot(GetCatalogSnapshot(relid)).

In InitPostgres, it's a little less obvious that it's not required,
but still true I believe. All the catalog lookups in InitPostgres()
also use the catalog snapshot, and the looked up values are copied
while still holding the snapshot.

Furthermore, as the removed FIXME comments said, calling
GetTransactionSnapshot() didn't really prevent MyProc->xmin from being
reset anyway.

Discussion: https://www.postgresql.org/message-id/7c56f180-b9e1-481e-8c1d-efa63de3ecbb@iki.fi

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index dc3cf87abab8d18a6d078d6f5d9cf2aa1ba0a018..8078eeef62e71fae623e904c026b68c228cac7bf 100644 (file)
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -1799,18 +1799,9 @@ get_database_list(void)
    resultcxt = CurrentMemoryContext;
 
    /*
-    * Start a transaction so we can access pg_database, and get a snapshot.
-    * We don't have a use for the snapshot itself, but we're interested in
-    * the secondary effect that it sets RecentGlobalXmin.  (This is critical
-    * for anything that reads heap pages, because HOT may decide to prune
-    * them even if the process doesn't attempt to modify any tuples.)
-    *
-    * FIXME: This comment is inaccurate / the code buggy. A snapshot that is
-    * not pushed/active does not reliably prevent HOT pruning (->xmin could
-    * e.g. be cleared when cache invalidations are processed).
+    * Start a transaction so we can access pg_database.
     */
    StartTransactionCommand();
-   (void) GetTransactionSnapshot();
 
    rel = table_open(DatabaseRelationId, AccessShareLock);
    scan = table_beginscan_catalog(rel, 0, NULL);

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index e5fdca8bbf6eed23eb4abb7ffa7aa93c2534851d..8b1964204453f6584f303e8b49c3bd8b35bebcdc 100644 (file)
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -121,18 +121,9 @@ get_subscription_list(void)
    resultcxt = CurrentMemoryContext;
 
    /*
-    * Start a transaction so we can access pg_database, and get a snapshot.
-    * We don't have a use for the snapshot itself, but we're interested in
-    * the secondary effect that it sets RecentGlobalXmin.  (This is critical
-    * for anything that reads heap pages, because HOT may decide to prune
-    * them even if the process doesn't attempt to modify any tuples.)
-    *
-    * FIXME: This comment is inaccurate / the code buggy. A snapshot that is
-    * not pushed/active does not reliably prevent HOT pruning (->xmin could
-    * e.g. be cleared when cache invalidations are processed).
+    * Start a transaction so we can access pg_subscription.
     */
    StartTransactionCommand();
-   (void) GetTransactionSnapshot();
 
    rel = table_open(SubscriptionRelationId, AccessShareLock);
    scan = table_beginscan_catalog(rel, 0, NULL);

diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 5b657a3f135ccc127dbafcba3b9c7981fb7081b7..770ab6906e7efd0fd1f820ddbfad21595ece6b2d 100644 (file)
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -813,16 +813,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
    }
 
    /*
-    * Start a new transaction here before first access to db, and get a
-    * snapshot.  We don't have a use for the snapshot itself, but we're
-    * interested in the secondary effect that it sets RecentGlobalXmin. (This
-    * is critical for anything that reads heap pages, because HOT may decide
-    * to prune them even if the process doesn't attempt to modify any
-    * tuples.)
-    *
-    * FIXME: This comment is inaccurate / the code buggy. A snapshot that is
-    * not pushed/active does not reliably prevent HOT pruning (->xmin could
-    * e.g. be cleared when cache invalidations are processed).
+    * Start a new transaction here before first access to db.
     */
    if (!bootstrap)
    {
@@ -837,8 +828,6 @@ InitPostgres(const char *in_dbname, Oid dboid,
         * Fortunately, "read committed" is plenty good enough.
         */
        XactIsoLevel = XACT_READ_COMMITTED;
-
-       (void) GetTransactionSnapshot();
    }
 
    /*