Avoid problems with OID wraparound during WAL replay.

Fix a longstanding thinko in replay of NEXTOID and checkpoint records: we
tried to advance nextOid only if it was behind the value in the WAL record,
but the comparison would draw the wrong conclusion if OID wraparound had
occurred since the previous value.  Better to just unconditionally assign
the new value, since OID assignment shouldn't be happening during replay
anyway.

The consequences of a failure to update nextOid would be pretty minimal,
since we have long had the code set up to obtain another OID and try again
if the generated value is already in use.  But in the worst case there
could be significant performance glitches while such loops iterate through
many already-used OIDs before finding a free one.

The odds of a wraparound happening during WAL replay would be small in a
crash-recovery scenario, and the length of any ensuing OID-assignment stall
quite limited anyway.  But neither of these statements hold true for a
replication slave that follows a WAL stream for a long period; its behavior
upon going live could be almost unboundedly bad.  Hence it seems worth
back-patching this fix into all supported branches.

Already fixed in HEAD in commit c6d76d7c82ebebb7210029f7382c0ebe2c558bca.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3da118b14ed9669a92e71a908ecc8b82a0fe93e7..b8df42567d265a8efd758ba04f724c1a9927a483 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7017,12 +7017,15 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record)
    {
        Oid         nextOid;
 
+       /*
+        * We used to try to take the maximum of ShmemVariableCache->nextOid
+        * and the recorded nextOid, but that fails if the OID counter wraps
+        * around.  Since no OID allocation should be happening during replay
+        * anyway, better to just believe the record exactly.
+        */
        memcpy(&nextOid, XLogRecGetData(record), sizeof(Oid));
-       if (ShmemVariableCache->nextOid < nextOid)
-       {
-           ShmemVariableCache->nextOid = nextOid;
-           ShmemVariableCache->oidCount = 0;
-       }
+       ShmemVariableCache->nextOid = nextOid;
+       ShmemVariableCache->oidCount = 0;
    }
    else if (info == XLOG_CHECKPOINT_SHUTDOWN)
    {
@@ -7062,15 +7065,13 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record)
        CheckPoint  checkPoint;
 
        memcpy(&checkPoint, XLogRecGetData(record), sizeof(CheckPoint));
-       /* In an ONLINE checkpoint, treat the counters like NEXTOID */
+       /* In an ONLINE checkpoint, treat the XID counter as a minimum */
        if (TransactionIdPrecedes(ShmemVariableCache->nextXid,
                                  checkPoint.nextXid))
            ShmemVariableCache->nextXid = checkPoint.nextXid;
-       if (ShmemVariableCache->nextOid < checkPoint.nextOid)
-       {
-           ShmemVariableCache->nextOid = checkPoint.nextOid;
-           ShmemVariableCache->oidCount = 0;
-       }
+       /* ... but still treat OID counter as exact */
+       ShmemVariableCache->nextOid = checkPoint.nextOid;
+       ShmemVariableCache->oidCount = 0;
        MultiXactAdvanceNextMXact(checkPoint.nextMulti,
                                  checkPoint.nextMultiOffset);