Close race condition between datfrozen and relfrozen updates.

vac_update_datfrozenxid() did multiple loads of relfrozenxid and
relminmxid from buffer memory, and it assumed each would get the same
value.  Not so if a concurrent vac_update_relstats() did an inplace
update.  Commit 2d2e40e3befd8b9e0d2757554537345b15fa6ea2 fixed the same
kind of bug in vac_truncate_clog().  Today's bug could cause the
rel-level field and XIDs in the rel's rows to precede the db-level
field.  A cluster having such values should VACUUM affected tables.
Back-patch to v12 (all supported versions).

Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index b92e6c1c6aaa563be7c3009214016728272d9476..9a4428a8851723cbcc348eb1f30e09bd211df78f 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1379,6 +1379,8 @@ vac_update_datfrozenxid(void)
    /*
     * We must seqscan pg_class to find the minimum Xid, because there is no
     * index that can help us here.
+    *
+    * See vac_truncate_clog() for the race condition to prevent.
     */
    relation = table_open(RelationRelationId, AccessShareLock);
 
@@ -1387,7 +1389,9 @@ vac_update_datfrozenxid(void)
 
    while ((classTup = systable_getnext(scan)) != NULL)
    {
-       Form_pg_class classForm = (Form_pg_class) GETSTRUCT(classTup);
+       volatile FormData_pg_class *classForm = (Form_pg_class) GETSTRUCT(classTup);
+       TransactionId relfrozenxid = classForm->relfrozenxid;
+       TransactionId relminmxid = classForm->relminmxid;
 
        /*
         * Only consider relations able to hold unfrozen XIDs (anything else
@@ -1397,8 +1401,8 @@ vac_update_datfrozenxid(void)
            classForm->relkind != RELKIND_MATVIEW &&
            classForm->relkind != RELKIND_TOASTVALUE)
        {
-           Assert(!TransactionIdIsValid(classForm->relfrozenxid));
-           Assert(!MultiXactIdIsValid(classForm->relminmxid));
+           Assert(!TransactionIdIsValid(relfrozenxid));
+           Assert(!MultiXactIdIsValid(relminmxid));
            continue;
        }
 
@@ -1417,34 +1421,34 @@ vac_update_datfrozenxid(void)
         * before those relations have been scanned and cleaned up.
         */
 
-       if (TransactionIdIsValid(classForm->relfrozenxid))
+       if (TransactionIdIsValid(relfrozenxid))
        {
-           Assert(TransactionIdIsNormal(classForm->relfrozenxid));
+           Assert(TransactionIdIsNormal(relfrozenxid));
 
            /* check for values in the future */
-           if (TransactionIdPrecedes(lastSaneFrozenXid, classForm->relfrozenxid))
+           if (TransactionIdPrecedes(lastSaneFrozenXid, relfrozenxid))
            {
                bogus = true;
                break;
            }
 
            /* determine new horizon */
-           if (TransactionIdPrecedes(classForm->relfrozenxid, newFrozenXid))
-               newFrozenXid = classForm->relfrozenxid;
+           if (TransactionIdPrecedes(relfrozenxid, newFrozenXid))
+               newFrozenXid = relfrozenxid;
        }
 
-       if (MultiXactIdIsValid(classForm->relminmxid))
+       if (MultiXactIdIsValid(relminmxid))
        {
            /* check for values in the future */
-           if (MultiXactIdPrecedes(lastSaneMinMulti, classForm->relminmxid))
+           if (MultiXactIdPrecedes(lastSaneMinMulti, relminmxid))
            {
                bogus = true;
                break;
            }
 
            /* determine new horizon */
-           if (MultiXactIdPrecedes(classForm->relminmxid, newMinMulti))
-               newMinMulti = classForm->relminmxid;
+           if (MultiXactIdPrecedes(relminmxid, newMinMulti))
+               newMinMulti = relminmxid;
        }
    }