Hi all,
As of now UpdateMinRecoveryPoint() is using two arguments:
- lsn, to check if the minimum recovery point should be updated to that
- force, a boolean flag to decide if the update should be enforced or not.
However those two arguments are correlated. If lsn is
InvalidXlogRecPtr, the minimum recovery point update will be enforced.
Hence why not simplifying its interface and remove the force flag? See
attached.
Thanks,
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e4645a3..5d69bd3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -827,7 +827,7 @@ static void RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRec
static void UpdateLastRemovedPtr(char *filename);
static void ValidateXLOGDirectoryStructure(void);
static void CleanupBackupHistory(void);
-static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force);
+static void UpdateMinRecoveryPoint(XLogRecPtr lsn);
static XLogRecord *ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr,
int emode, bool fetching_ckpt);
static void CheckRecoveryConsistency(void);
@@ -2489,14 +2489,16 @@ XLogGetReplicationSlotMinimumLSN(void)
* If we crash during recovery, we must reach this point again before the
* database is consistent.
*
- * If 'force' is true, 'lsn' argument is ignored. Otherwise, minRecoveryPoint
- * is only updated if it's not already greater than or equal to 'lsn'.
+ * If 'lsn' is InvalidXLogRecPtr, enforce its advance. Otherwise,
+ * minRecoveryPoint is only updated if it's not already greater than or
+ * equal to 'lsn'.
*/
static void
-UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
+UpdateMinRecoveryPoint(XLogRecPtr lsn)
{
/* Quick check using our local copy of the variable */
- if (!updateMinRecoveryPoint || (!force && lsn <= minRecoveryPoint))
+ if (!updateMinRecoveryPoint ||
+ (!XLogRecPtrIsInvalid(lsn) && lsn <= minRecoveryPoint))
return;
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
@@ -2512,7 +2514,7 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
*/
if (minRecoveryPoint == 0)
updateMinRecoveryPoint = false;
- else if (force || minRecoveryPoint < lsn)
+ else if (XLogRecPtrIsInvalid(lsn) || minRecoveryPoint < lsn)
{
XLogRecPtr newMinRecoveryPoint;
TimeLineID newMinRecoveryPointTLI;
@@ -2520,8 +2522,7 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
/*
* To avoid having to update the control file too often, we update it
* all the way to the last record being replayed, even though 'lsn'
- * would suffice for correctness. This also allows the 'force' case
- * to not need a valid 'lsn' value.
+ * would suffice for correctness.
*
* Another important reason for doing it this way is that the passed
* 'lsn' value could be bogus, i.e., past the end of available WAL, if
@@ -2535,7 +2536,7 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
newMinRecoveryPointTLI = XLogCtl->replayEndTLI;
SpinLockRelease(&XLogCtl->info_lck);
- if (!force && newMinRecoveryPoint < lsn)
+ if (!XLogRecPtrIsInvalid(lsn) && newMinRecoveryPoint < lsn)
elog(WARNING,
"xlog min recovery request %X/%X is past current point %X/%X",
(uint32) (lsn >> 32), (uint32) lsn,
@@ -2582,7 +2583,8 @@ XLogFlush(XLogRecPtr record)
*/
if (!XLogInsertAllowed())
{
- UpdateMinRecoveryPoint(record, false);
+ Assert(!XLogRecPtrIsInvalid(record));
+ UpdateMinRecoveryPoint(record);
return;
}
@@ -5244,7 +5246,7 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
/*
* Update min recovery point one last time.
*/
- UpdateMinRecoveryPoint(InvalidXLogRecPtr, true);
+ UpdateMinRecoveryPoint(InvalidXLogRecPtr);
/*
* If the ending log segment is still open, close it (to avoid problems on
@@ -8750,7 +8752,7 @@ CreateRestartPoint(int flags)
(uint32) (lastCheckPoint.redo >> 32),
(uint32) lastCheckPoint.redo)));
- UpdateMinRecoveryPoint(InvalidXLogRecPtr, true);
+ UpdateMinRecoveryPoint(InvalidXLogRecPtr);
if (flags & CHECKPOINT_IS_SHUTDOWN)
{
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
