From 2ca19aa8165c6bec84c1d527fc5f3100c2161b1a 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 18018a4e8a..8f24b88546 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1480,6 +1480,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); @@ -1488,7 +1490,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 @@ -1498,8 +1502,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; } @@ -1518,34 +1522,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; } }