From f1b8a84dec30b44e6a0b306f95961f5426cb8368 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 6 Feb 2012 13:14:46 -0500 Subject: [PATCH] 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. --- src/backend/access/transam/xlog.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 575cfbafff..1ec5b51554 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8387,12 +8387,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) { @@ -8480,15 +8483,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); if (TransactionIdPrecedes(ShmemVariableCache->oldestXid, -- 2.40.0