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 2d2e40e3be 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
This commit is contained in:
Noah Misch 2024-04-29 10:24:56 -07:00
parent 617a239272
commit 2ca19aa816
1 changed files with 16 additions and 12 deletions

View File

@ -1480,6 +1480,8 @@ vac_update_datfrozenxid(void)
/* /*
* We must seqscan pg_class to find the minimum Xid, because there is no * We must seqscan pg_class to find the minimum Xid, because there is no
* index that can help us here. * index that can help us here.
*
* See vac_truncate_clog() for the race condition to prevent.
*/ */
relation = table_open(RelationRelationId, AccessShareLock); relation = table_open(RelationRelationId, AccessShareLock);
@ -1488,7 +1490,9 @@ vac_update_datfrozenxid(void)
while ((classTup = systable_getnext(scan)) != NULL) 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 * Only consider relations able to hold unfrozen XIDs (anything else
@ -1498,8 +1502,8 @@ vac_update_datfrozenxid(void)
classForm->relkind != RELKIND_MATVIEW && classForm->relkind != RELKIND_MATVIEW &&
classForm->relkind != RELKIND_TOASTVALUE) classForm->relkind != RELKIND_TOASTVALUE)
{ {
Assert(!TransactionIdIsValid(classForm->relfrozenxid)); Assert(!TransactionIdIsValid(relfrozenxid));
Assert(!MultiXactIdIsValid(classForm->relminmxid)); Assert(!MultiXactIdIsValid(relminmxid));
continue; continue;
} }
@ -1518,34 +1522,34 @@ vac_update_datfrozenxid(void)
* before those relations have been scanned and cleaned up. * 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 */ /* check for values in the future */
if (TransactionIdPrecedes(lastSaneFrozenXid, classForm->relfrozenxid)) if (TransactionIdPrecedes(lastSaneFrozenXid, relfrozenxid))
{ {
bogus = true; bogus = true;
break; break;
} }
/* determine new horizon */ /* determine new horizon */
if (TransactionIdPrecedes(classForm->relfrozenxid, newFrozenXid)) if (TransactionIdPrecedes(relfrozenxid, newFrozenXid))
newFrozenXid = classForm->relfrozenxid; newFrozenXid = relfrozenxid;
} }
if (MultiXactIdIsValid(classForm->relminmxid)) if (MultiXactIdIsValid(relminmxid))
{ {
/* check for values in the future */ /* check for values in the future */
if (MultiXactIdPrecedes(lastSaneMinMulti, classForm->relminmxid)) if (MultiXactIdPrecedes(lastSaneMinMulti, relminmxid))
{ {
bogus = true; bogus = true;
break; break;
} }
/* determine new horizon */ /* determine new horizon */
if (MultiXactIdPrecedes(classForm->relminmxid, newMinMulti)) if (MultiXactIdPrecedes(relminmxid, newMinMulti))
newMinMulti = classForm->relminmxid; newMinMulti = relminmxid;
} }
} }