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 75b0ca9534707dbf4a93fc3dcedf7e9a014e90a5..329c73d226111ae212554dc30f9780e1d735d3dd 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1532,6 +1532,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);
 
@@ -1540,7 +1542,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
@@ -1550,8 +1554,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;
                }
 
@@ -1570,34 +1574,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;
                }
        }