xlog.c: Remove global variables ReadRecPtr and EndRecPtr.

In most places, the variables necessarily store the same value as the
eponymous members of the XLogReaderState that we use during WAL
replay, because ReadRecord() assigns the values from the structure
members to the global variables just after XLogReadRecord() returns.
However, XLogBeginRead() adjusts the structure members but not the
global variables, so after XLogBeginRead() and before the completion
of XLogReadRecord() the values can differ. Otherwise, they must be
identical.  According to my analysis, the only place where either
variable is referenced at a point where it might not have the same
value as the structure member is the refrence to EndRecPtr within
XLogPageRead.

Therefore, at every other place where we are using the global
variable, we can just switch to using the structure member instead,
and remove the global variable. However, we can, and in fact should,
do this in XLogPageRead() as well, because at that point in the code,
the global variable will actually store the start of the record we
want to read - either because it's where the last WAL record ended, or
because the read position has been changed using XLogBeginRead since
the last record was read. The structure member, on the other hand,
will already have been updated to point to the end of the record we
just read. Elsewhere, the latter is what we use as an argument to
emode_for_corrupt_record(), so we should do the same here.

This part of the patch is perhaps a bug fix, but I don't think it has
any important consequences, so no back-patch. The point here is just
to continue to whittle down the entirely excessive use of global
variables in xlog.c.

Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
This commit is contained in:
Robert Haas 2021-11-24 11:27:39 -05:00
parent e7ea2fa342
commit d2ddfa681d
1 changed files with 24 additions and 29 deletions

View File

@ -850,10 +850,6 @@ typedef struct XLogPageReadPrivate
static TimestampTz XLogReceiptTime = 0; static TimestampTz XLogReceiptTime = 0;
static XLogSource XLogReceiptSource = XLOG_FROM_ANY; static XLogSource XLogReceiptSource = XLOG_FROM_ANY;
/* State information for XLOG reading */
static XLogRecPtr ReadRecPtr; /* start of last record read */
static XLogRecPtr EndRecPtr; /* end+1 of last record read */
/* /*
* Local copies of equivalent fields in the control file. When running * Local copies of equivalent fields in the control file. When running
* crash recovery, minRecoveryPoint is set to InvalidXLogRecPtr as we * crash recovery, minRecoveryPoint is set to InvalidXLogRecPtr as we
@ -4475,8 +4471,6 @@ ReadRecord(XLogReaderState *xlogreader, int emode,
char *errormsg; char *errormsg;
record = XLogReadRecord(xlogreader, &errormsg); record = XLogReadRecord(xlogreader, &errormsg);
ReadRecPtr = xlogreader->ReadRecPtr;
EndRecPtr = xlogreader->EndRecPtr;
if (record == NULL) if (record == NULL)
{ {
/* /*
@ -4505,7 +4499,7 @@ ReadRecord(XLogReaderState *xlogreader, int emode,
* shouldn't loop anymore in that case. * shouldn't loop anymore in that case.
*/ */
if (errormsg) if (errormsg)
ereport(emode_for_corrupt_record(emode, EndRecPtr), ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
(errmsg_internal("%s", errormsg) /* already translated */ )); (errmsg_internal("%s", errormsg) /* already translated */ ));
} }
@ -4523,7 +4517,7 @@ ReadRecord(XLogReaderState *xlogreader, int emode,
wal_segment_size); wal_segment_size);
XLogFileName(fname, xlogreader->seg.ws_tli, segno, XLogFileName(fname, xlogreader->seg.ws_tli, segno,
wal_segment_size); wal_segment_size);
ereport(emode_for_corrupt_record(emode, EndRecPtr), ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
(errmsg("unexpected timeline ID %u in log segment %s, offset %u", (errmsg("unexpected timeline ID %u in log segment %s, offset %u",
xlogreader->latestPageTLI, xlogreader->latestPageTLI,
fname, fname,
@ -4565,9 +4559,9 @@ ReadRecord(XLogReaderState *xlogreader, int emode,
/* initialize minRecoveryPoint to this record */ /* initialize minRecoveryPoint to this record */
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE); LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
ControlFile->state = DB_IN_ARCHIVE_RECOVERY; ControlFile->state = DB_IN_ARCHIVE_RECOVERY;
if (ControlFile->minRecoveryPoint < EndRecPtr) if (ControlFile->minRecoveryPoint < xlogreader->EndRecPtr)
{ {
ControlFile->minRecoveryPoint = EndRecPtr; ControlFile->minRecoveryPoint = xlogreader->EndRecPtr;
ControlFile->minRecoveryPointTLI = replayTLI; ControlFile->minRecoveryPointTLI = replayTLI;
} }
/* update local copy */ /* update local copy */
@ -7501,7 +7495,7 @@ StartupXLOG(void)
if (checkPoint.redo < RecPtr) if (checkPoint.redo < RecPtr)
XLogCtl->replayEndRecPtr = checkPoint.redo; XLogCtl->replayEndRecPtr = checkPoint.redo;
else else
XLogCtl->replayEndRecPtr = EndRecPtr; XLogCtl->replayEndRecPtr = xlogreader->EndRecPtr;
XLogCtl->replayEndTLI = replayTLI; XLogCtl->replayEndTLI = replayTLI;
XLogCtl->lastReplayedEndRecPtr = XLogCtl->replayEndRecPtr; XLogCtl->lastReplayedEndRecPtr = XLogCtl->replayEndRecPtr;
XLogCtl->lastReplayedTLI = XLogCtl->replayEndTLI; XLogCtl->lastReplayedTLI = XLogCtl->replayEndTLI;
@ -7557,7 +7551,7 @@ StartupXLOG(void)
ereport(LOG, ereport(LOG,
(errmsg("redo starts at %X/%X", (errmsg("redo starts at %X/%X",
LSN_FORMAT_ARGS(ReadRecPtr)))); LSN_FORMAT_ARGS(xlogreader->ReadRecPtr))));
/* Prepare to report progress of the redo phase. */ /* Prepare to report progress of the redo phase. */
if (!StandbyMode) if (!StandbyMode)
@ -7572,7 +7566,7 @@ StartupXLOG(void)
if (!StandbyMode) if (!StandbyMode)
ereport_startup_progress("redo in progress, elapsed time: %ld.%02d s, current LSN: %X/%X", ereport_startup_progress("redo in progress, elapsed time: %ld.%02d s, current LSN: %X/%X",
LSN_FORMAT_ARGS(ReadRecPtr)); LSN_FORMAT_ARGS(xlogreader->ReadRecPtr));
#ifdef WAL_DEBUG #ifdef WAL_DEBUG
if (XLOG_DEBUG || if (XLOG_DEBUG ||
@ -7686,7 +7680,8 @@ StartupXLOG(void)
if (newReplayTLI != replayTLI) if (newReplayTLI != replayTLI)
{ {
/* Check that it's OK to switch to this TLI */ /* Check that it's OK to switch to this TLI */
checkTimeLineSwitch(EndRecPtr, newReplayTLI, checkTimeLineSwitch(xlogreader->EndRecPtr,
newReplayTLI,
prevReplayTLI, replayTLI); prevReplayTLI, replayTLI);
/* Following WAL records should be run with new TLI */ /* Following WAL records should be run with new TLI */
@ -7700,7 +7695,7 @@ StartupXLOG(void)
* so that XLogFlush will update minRecoveryPoint correctly. * so that XLogFlush will update minRecoveryPoint correctly.
*/ */
SpinLockAcquire(&XLogCtl->info_lck); SpinLockAcquire(&XLogCtl->info_lck);
XLogCtl->replayEndRecPtr = EndRecPtr; XLogCtl->replayEndRecPtr = xlogreader->EndRecPtr;
XLogCtl->replayEndTLI = replayTLI; XLogCtl->replayEndTLI = replayTLI;
SpinLockRelease(&XLogCtl->info_lck); SpinLockRelease(&XLogCtl->info_lck);
@ -7732,7 +7727,7 @@ StartupXLOG(void)
* successfully replayed. * successfully replayed.
*/ */
SpinLockAcquire(&XLogCtl->info_lck); SpinLockAcquire(&XLogCtl->info_lck);
XLogCtl->lastReplayedEndRecPtr = EndRecPtr; XLogCtl->lastReplayedEndRecPtr = xlogreader->EndRecPtr;
XLogCtl->lastReplayedTLI = replayTLI; XLogCtl->lastReplayedTLI = replayTLI;
SpinLockRelease(&XLogCtl->info_lck); SpinLockRelease(&XLogCtl->info_lck);
@ -7748,7 +7743,7 @@ StartupXLOG(void)
} }
/* Remember this record as the last-applied one */ /* Remember this record as the last-applied one */
LastRec = ReadRecPtr; LastRec = xlogreader->ReadRecPtr;
/* Allow read-only connections if we're consistent now */ /* Allow read-only connections if we're consistent now */
CheckRecoveryConsistency(); CheckRecoveryConsistency();
@ -7761,7 +7756,7 @@ StartupXLOG(void)
* (possibly bogus) future WAL segments on the old * (possibly bogus) future WAL segments on the old
* timeline. * timeline.
*/ */
RemoveNonParentXlogFiles(EndRecPtr, replayTLI); RemoveNonParentXlogFiles(xlogreader->EndRecPtr, replayTLI);
/* /*
* Wake up any walsenders to notice that we are on a new * Wake up any walsenders to notice that we are on a new
@ -7829,7 +7824,7 @@ StartupXLOG(void)
ereport(LOG, ereport(LOG,
(errmsg("redo done at %X/%X system usage: %s", (errmsg("redo done at %X/%X system usage: %s",
LSN_FORMAT_ARGS(ReadRecPtr), LSN_FORMAT_ARGS(xlogreader->ReadRecPtr),
pg_rusage_show(&ru0)))); pg_rusage_show(&ru0))));
xtime = GetLatestXTime(); xtime = GetLatestXTime();
if (xtime) if (xtime)
@ -7904,7 +7899,7 @@ StartupXLOG(void)
*/ */
XLogBeginRead(xlogreader, LastRec); XLogBeginRead(xlogreader, LastRec);
record = ReadRecord(xlogreader, PANIC, false, replayTLI); record = ReadRecord(xlogreader, PANIC, false, replayTLI);
EndOfLog = EndRecPtr; EndOfLog = xlogreader->EndRecPtr;
/* /*
* EndOfLogTLI is the TLI in the filename of the XLOG segment containing * EndOfLogTLI is the TLI in the filename of the XLOG segment containing
@ -8013,7 +8008,7 @@ StartupXLOG(void)
* between here and writing the end-of-recovery record. * between here and writing the end-of-recovery record.
*/ */
writeTimeLineHistory(newTLI, recoveryTargetTLI, writeTimeLineHistory(newTLI, recoveryTargetTLI,
EndRecPtr, reason); xlogreader->EndRecPtr, reason);
/* /*
* Since there might be a partial WAL segment named RECOVERYXLOG, get * Since there might be a partial WAL segment named RECOVERYXLOG, get
@ -9647,7 +9642,7 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags)
* startup process.) * startup process.)
*/ */
static void static void
RecoveryRestartPoint(const CheckPoint *checkPoint) RecoveryRestartPoint(const CheckPoint *checkPoint, XLogReaderState *record)
{ {
/* /*
* Also refrain from creating a restartpoint if we have seen any * Also refrain from creating a restartpoint if we have seen any
@ -9670,8 +9665,8 @@ RecoveryRestartPoint(const CheckPoint *checkPoint)
* work out the next time it wants to perform a restartpoint. * work out the next time it wants to perform a restartpoint.
*/ */
SpinLockAcquire(&XLogCtl->info_lck); SpinLockAcquire(&XLogCtl->info_lck);
XLogCtl->lastCheckPointRecPtr = ReadRecPtr; XLogCtl->lastCheckPointRecPtr = record->ReadRecPtr;
XLogCtl->lastCheckPointEndPtr = EndRecPtr; XLogCtl->lastCheckPointEndPtr = record->EndRecPtr;
XLogCtl->lastCheckPoint = *checkPoint; XLogCtl->lastCheckPoint = *checkPoint;
SpinLockRelease(&XLogCtl->info_lck); SpinLockRelease(&XLogCtl->info_lck);
} }
@ -10468,7 +10463,7 @@ xlog_redo(XLogReaderState *record)
(errmsg("unexpected timeline ID %u (should be %u) in checkpoint record", (errmsg("unexpected timeline ID %u (should be %u) in checkpoint record",
checkPoint.ThisTimeLineID, replayTLI))); checkPoint.ThisTimeLineID, replayTLI)));
RecoveryRestartPoint(&checkPoint); RecoveryRestartPoint(&checkPoint, record);
} }
else if (info == XLOG_CHECKPOINT_ONLINE) else if (info == XLOG_CHECKPOINT_ONLINE)
{ {
@ -10524,7 +10519,7 @@ xlog_redo(XLogReaderState *record)
(errmsg("unexpected timeline ID %u (should be %u) in checkpoint record", (errmsg("unexpected timeline ID %u (should be %u) in checkpoint record",
checkPoint.ThisTimeLineID, replayTLI))); checkPoint.ThisTimeLineID, replayTLI)));
RecoveryRestartPoint(&checkPoint); RecoveryRestartPoint(&checkPoint, record);
} }
else if (info == XLOG_OVERWRITE_CONTRECORD) else if (info == XLOG_OVERWRITE_CONTRECORD)
{ {
@ -10690,8 +10685,8 @@ xlog_redo(XLogReaderState *record)
if (!fpw) if (!fpw)
{ {
SpinLockAcquire(&XLogCtl->info_lck); SpinLockAcquire(&XLogCtl->info_lck);
if (XLogCtl->lastFpwDisableRecPtr < ReadRecPtr) if (XLogCtl->lastFpwDisableRecPtr < record->ReadRecPtr)
XLogCtl->lastFpwDisableRecPtr = ReadRecPtr; XLogCtl->lastFpwDisableRecPtr = record->ReadRecPtr;
SpinLockRelease(&XLogCtl->info_lck); SpinLockRelease(&XLogCtl->info_lck);
} }
@ -12586,7 +12581,7 @@ retry:
* errmsg_internal() because the message was already translated. * errmsg_internal() because the message was already translated.
*/ */
if (xlogreader->errormsg_buf[0]) if (xlogreader->errormsg_buf[0])
ereport(emode_for_corrupt_record(emode, EndRecPtr), ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
(errmsg_internal("%s", xlogreader->errormsg_buf))); (errmsg_internal("%s", xlogreader->errormsg_buf)));
/* reset any error XLogReaderValidatePageHeader() might have set */ /* reset any error XLogReaderValidatePageHeader() might have set */