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;
                }
        }