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

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index affe3896056d4519d79e73e0268bb6f4f86f2999..a435d58a4f1a164e3b23521bf62c3d367cd62425 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9751,11 +9751,20 @@ xlog_redo(XLogReaderState *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);