Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fetch XIDs atomically during vac_truncate_clog().
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 24 May 2016 19:47:51 +0000 (15:47 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 24 May 2016 19:47:51 +0000 (15:47 -0400)
Because vac_update_datfrozenxid() updates datfrozenxid and datminmxid
in-place, it's unsafe to assume that successive reads of those values will
give consistent results.  Fetch each one just once to ensure sane behavior
in the minimum calculation.  Noted while reviewing Alexander Korotkov's
patch in the same area.

Discussion: <8564.1464116473@sss.pgh.pa.us>

src/backend/commands/vacuum.c

index c019b7230bfb3f394e90be23b91731b0c6402313..7c4f53cab9adb5287c6a890d026d487c512a7ec2 100644 (file)
@@ -815,6 +815,12 @@ vac_truncate_clog(TransactionId frozenXID)
    /*
     * Scan pg_database to compute the minimum datfrozenxid
     *
+    * Since vac_update_datfrozenxid updates datfrozenxid in-place,
+    * the values could change while we look at them.  Fetch each one just
+    * once to ensure sane behavior of the comparison logic.  (Here, as in
+    * many other places, we assume that fetching or updating an XID in shared
+    * storage is atomic.)
+    *
     * Note: we need not worry about a race condition with new entries being
     * inserted by CREATE DATABASE.  Any such entry will have a copy of some
     * existing DB's datfrozenxid, and that source DB cannot be ours because
@@ -830,15 +836,16 @@ vac_truncate_clog(TransactionId frozenXID)
 
    while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
    {
-       Form_pg_database dbform = (Form_pg_database) GETSTRUCT(tuple);
+       volatile FormData_pg_database *dbform = (Form_pg_database) GETSTRUCT(tuple);
+       TransactionId datfrozenxid = dbform->datfrozenxid;
 
-       Assert(TransactionIdIsNormal(dbform->datfrozenxid));
+       Assert(TransactionIdIsNormal(datfrozenxid));
 
-       if (TransactionIdPrecedes(nextXID, dbform->datfrozenxid))
+       if (TransactionIdPrecedes(nextXID, datfrozenxid))
            frozenAlreadyWrapped = true;
-       else if (TransactionIdPrecedes(dbform->datfrozenxid, frozenXID))
+       else if (TransactionIdPrecedes(datfrozenxid, frozenXID))
        {
-           frozenXID = dbform->datfrozenxid;
+           frozenXID = datfrozenxid;
            oldest_datoid = HeapTupleGetOid(tuple);
        }
    }