Re: updateMinRecoveryPoint bug?

Fujii Masao wrote:
> In UpdateMinRecoveryPoint() and XLogNeedsFlush(), updateMinRecoveryPoint
> is used for us to short-circuit future checks only during a crash recovery.
> But it doesn't seem to play its role in a crash recovery that follows an
> archive recovery. Because such crash recovery always starts from *valid*
> minRecoveryPoint, i.e., updateMinRecoveryPoint is never set to FALSE.

Hmm, so the problem is:

1. Restore from archive. End archive recovery, creating a new
checkpoint. But we still write the old minRecoveryPoint value to pg_control

2. Crash. minRecoveryPoint is still set in crash recovery

Yeah, that should be fixed. Otherwise we will merrily start up after
crash, even if we don't reach the end of WAL. Although that shouldn't
happen, it's a bit disconcerting.

> How about always resetting ControlFile->minRecoveryPoint to {0, 0} at the
> beginning of a crash recovery, to fix the bug?

Yeah, that would work. I think it would be better to clear it in
CreateCheckPoint(), though, when we set the pointer to the new
checkpoint. That includes the shutdown checkpoint created at the end of
archive recovery. minRecoveryPoint should never be set during normal
operation, after all.

--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6882,6 +6882,7 @@ CreateCheckPoint(int flags)
ControlFile->checkPoint = ProcLastRecPtr;
ControlFile->checkPointCopy = checkPoint;
ControlFile->time = (pg_time_t) time(NULL);
+ MemSet(&ControlFile->minRecoveryPoint, 0, sizeof(XLogRecPtr));
UpdateControlFile();
LWLockRelease(ControlFileLock);

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com