From 6943fb9275a50f3a9d177da1a06ea387bf490ead Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Wed, 11 Apr 2018 18:11:30 -0400 Subject: [PATCH] Ignore nextOid when replaying an ONLINE checkpoint. The nextOid value is from the start of the checkpoint and may well be stale compared to values from more recent XLOG_NEXTOID records. Previously, we adopted it anyway, allowing the OID counter to go backwards during a crash. While this should be harmless, it contributed to the severity of the bug fixed in commit 0408e1ed5, by allowing duplicate TOAST OIDs to be assigned immediately following a crash. Without this error, that issue would only have arisen when TOAST objects just younger than a multiple of 2^32 OIDs were deleted and then not vacuumed in time to avoid a conflict. Pavan Deolasee Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com --- src/backend/access/transam/xlog.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index df777a07eeb..ebd86a79ff3 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9473,11 +9473,20 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record) checkPoint.nextXid)) ShmemVariableCache->nextXid = checkPoint.nextXid; LWLockRelease(XidGenLock); - /* ... but still treat OID counter as exact */ - LWLockAcquire(OidGenLock, LW_EXCLUSIVE); - ShmemVariableCache->nextOid = checkPoint.nextOid; - ShmemVariableCache->oidCount = 0; - LWLockRelease(OidGenLock); + + /* + * We ignore the nextOid counter in an ONLINE checkpoint, preferring + * to track OID assignment through XLOG_NEXTOID records. The nextOid + * counter is from the start of the checkpoint and might well be stale + * compared to later XLOG_NEXTOID records. We could try to take the + * maximum of the nextOid counter and our latest value, but since + * there's no particular guarantee about the speed with which the OID + * counter wraps around, that's a risky thing to do. In any case, + * users of the nextOid counter are required to avoid assignment of + * duplicates, so that a somewhat out-of-date value should be safe. + */ + + /* Handle multixact */ MultiXactAdvanceNextMXact(checkPoint.nextMulti, checkPoint.nextMultiOffset); if (TransactionIdPrecedes(ShmemVariableCache->oldestXid, -- GitLab