]> granicus.if.org Git - postgresql/commitdiff
Initialize the minimum frozen Xid in vac_update_datfrozenxid using
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Thu, 11 Sep 2008 14:01:35 +0000 (14:01 +0000)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Thu, 11 Sep 2008 14:01:35 +0000 (14:01 +0000)
GetOldestXmin() instead of RecentGlobalXmin; this is safer because we do not
depend on the latter being correctly set elsewhere, and while it is more
expensive, this code path is not performance-critical.  This is a real
risk for autovacuum, because it can execute whole cycles without doing
a single vacuum, which would mean that RecentGlobalXmin would stay at its
initialization value, FirstNormalTransactionId, causing a bogus value to be
inserted in pg_database.  This bug could explain some recent reports of
failure to truncate pg_clog.

At the same time, change the initialization of RecentGlobalXmin to
InvalidTransactionId, and ensure that it's set to something else whenever
it's going to be used.  Using it as FirstNormalTransactionId in HOT page
pruning could incur in data loss.  InitPostgres takes care of setting it
to a valid value, but the extra checks are there to prevent "special"
backends from behaving in unusual ways.

Per Tom Lane's detailed problem dissection in 29544.1221061979@sss.pgh.pa.us

src/backend/access/heap/heapam.c
src/backend/access/index/indexam.c
src/backend/commands/vacuum.c
src/backend/executor/nodeBitmapHeapscan.c
src/backend/utils/init/postinit.c
src/backend/utils/time/tqual.c

index 690d139fb28e4263c33473d816fb2833c1fa4062..5994329dcc74b81371d03bb885ba32230825874d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.249.2.2 2008/03/08 21:58:07 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.249.2.3 2008/09/11 14:01:35 alvherre Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -212,6 +212,7 @@ heapgetpage(HeapScanDesc scan, BlockNumber page)
        /*
         * Prune and repair fragmentation for the whole page, if possible.
         */
+       Assert(TransactionIdIsValid(RecentGlobalXmin));
        heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
 
        /*
@@ -1488,6 +1489,8 @@ heap_hot_search_buffer(ItemPointer tid, Buffer buffer, Snapshot snapshot,
        if (all_dead)
                *all_dead = true;
 
+       Assert(TransactionIdIsValid(RecentGlobalXmin));
+
        Assert(ItemPointerGetBlockNumber(tid) == BufferGetBlockNumber(buffer));
        offnum = ItemPointerGetOffsetNumber(tid);
        at_chain_start = true;
index e4da00e868a03c34c7d5fc418ca24d11bff6d048..719f52450158c1bccf36b141c9c6084b5a53e8e4 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.101 2008/01/01 19:45:46 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.101.2.1 2008/09/11 14:01:35 alvherre Exp $
  *
  * INTERFACE ROUTINES
  *             index_open              - open an index relation by relation OID
@@ -419,6 +419,8 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
        SCAN_CHECKS;
        GET_SCAN_PROCEDURE(amgettuple);
 
+       Assert(TransactionIdIsValid(RecentGlobalXmin));
+
        /*
         * We always reset xs_hot_dead; if we are here then either we are just
         * starting the scan, or we previously returned a visible tuple, and in
index 1a0eecd9490d517e2ad854cb010270049195cdb4..e3ec8772ce2291ad655819223936591a59dad749 100644 (file)
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.1 2008/03/14 17:26:00 alvherre Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.2 2008/09/11 14:01:35 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -781,14 +781,12 @@ vac_update_datfrozenxid(void)
        bool            dirty = false;
 
        /*
-        * Initialize the "min" calculation with RecentGlobalXmin.      Any
-        * not-yet-committed pg_class entries for new tables must have
-        * relfrozenxid at least this high, because any other open xact must have
-        * RecentXmin >= its PGPROC.xmin >= our RecentGlobalXmin; see
-        * AddNewRelationTuple().  So we cannot produce a wrong minimum by
-        * starting with this.
+        * Initialize the "min" calculation with GetOldestXmin, which is a
+        * reasonable approximation to the minimum relfrozenxid for not-yet-
+        * committed pg_class entries for new tables; see AddNewRelationTuple().
+        * Se we cannot produce a wrong minimum by starting with this.
         */
-       newFrozenXid = RecentGlobalXmin;
+       newFrozenXid = GetOldestXmin(true, true);
 
        /*
         * We must seqscan pg_class to find the minimum Xid, because there is no
@@ -982,18 +980,16 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
        /* Begin a transaction for vacuuming this relation */
        StartTransactionCommand();
 
-       if (vacstmt->full)
-       {
-               /* functions in indexes may want a snapshot set */
-               ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
-       }
-       else
+       /*
+        * Functions in indexes may want a snapshot set. Also, setting
+        * a snapshot ensures that RecentGlobalXmin is kept truly recent.
+        */
+       ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
+
+       if (!vacstmt->full)
        {
                /*
-                * During a lazy VACUUM we do not run any user-supplied functions, and
-                * so it should be safe to not create a transaction snapshot.
-                *
-                * We can furthermore set the PROC_IN_VACUUM flag, which lets other
+                * During a lazy VACUUM we can set the PROC_IN_VACUUM flag, which lets other
                 * concurrent VACUUMs know that they can ignore this one while
                 * determining their OldestXmin.  (The reason we don't set it during a
                 * full VACUUM is exactly that we may have to run user- defined
index e53982d6a9854ff34c880d0539da4a077d8ab7e8..64aea38130351dcae726e16eb324c6a620acfbeb 100644 (file)
@@ -21,7 +21,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.22 2008/01/01 19:45:49 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.22.2.1 2008/09/11 14:01:35 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,6 +36,7 @@
 #include "postgres.h"
 
 #include "access/heapam.h"
+#include "access/transam.h"
 #include "executor/execdebug.h"
 #include "executor/nodeBitmapHeapscan.h"
 #include "pgstat.h"
@@ -258,6 +259,7 @@ bitgetpage(HeapScanDesc scan, TBMIterateResult *tbmres)
        /*
         * Prune and repair fragmentation for the whole page, if possible.
         */
+       Assert(TransactionIdIsValid(RecentGlobalXmin));
        heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
 
        /*
index dd510f2fc840ff8e134b9dc8e3f7915410e4d934..a46f7835ab2a4373ab53b7cfca9a868d86273163 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/init/postinit.c,v 1.180 2008/01/01 19:45:53 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/init/postinit.c,v 1.180.2.1 2008/09/11 14:01:35 alvherre Exp $
  *
  *
  *-------------------------------------------------------------------------
@@ -44,6 +44,7 @@
 #include "utils/plancache.h"
 #include "utils/portal.h"
 #include "utils/relcache.h"
+#include "utils/tqual.h"
 #include "utils/syscache.h"
 
 
@@ -457,10 +458,15 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
        on_shmem_exit(ShutdownPostgres, 0);
 
        /*
-        * Start a new transaction here before first access to db
+        * 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.
         */
        if (!bootstrap)
+       {
                StartTransactionCommand();
+               (void) GetTransactionSnapshot();
+       }
 
        /*
         * Now that we have a transaction, we can take locks.  Take a writer's
index 6fbc935a6bc2d05ed6ca318022b2119db4eabf0b..db016abd09acbc22cc0391e4bdf4bd5202fe4668 100644 (file)
@@ -31,7 +31,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.109 2008/01/01 19:45:55 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.109.2.1 2008/09/11 14:01:35 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -75,10 +75,14 @@ Snapshot    ActiveSnapshot = NULL;
  * These are updated by GetSnapshotData.  We initialize them this way
  * for the convenience of TransactionIdIsInProgress: even in bootstrap
  * mode, we don't want it to say that BootstrapTransactionId is in progress.
+ *
+ * RecentGlobalXmin is initialized to InvalidTransactionId, to ensure that no
+ * one tries to use a stale value.  Readers should ensure that it has been set
+ * to something else before using it.
  */
 TransactionId TransactionXmin = FirstNormalTransactionId;
 TransactionId RecentXmin = FirstNormalTransactionId;
-TransactionId RecentGlobalXmin = FirstNormalTransactionId;
+TransactionId RecentGlobalXmin = InvalidTransactionId;
 
 /* local functions */
 static bool XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot);