From 70cadfba0c70b25922d0739a9e09d24f0efd7f46 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 29 Apr 2024 10:24:56 -0700 Subject: [PATCH] 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 --- src/backend/commands/vacuum.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index b92e6c1c6a..9a4428a885 100644 --- 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; } }