Author: Noah Misch Commit: Noah Misch 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). Reviewed by FIXME. Discussion: https://postgr.es/m/FIXME diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index b589279..a63a71c 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1611,6 +1611,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); @@ -1619,7 +1621,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 @@ -1629,8 +1633,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; } @@ -1649,34 +1653,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; } }