On Wed, Jan 26, 2022 at 09:57:59AM +0900, Michael Paquier wrote:
> Yeah, that sounds like a good compromise. At least, I find the whole
> a bit easier to follow.
So, I have been checking this idea in details, and spotted what looks
like one issue in CreateRestartPoint(), as of:
/*
* Update pg_control, using current time. Check that it still shows
* DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
* this is a quick hack to make sure nothing really bad happens if somehow
* we get here after the end-of-recovery checkpoint.
*/
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
This change increases the window making this code path reachable if an
end-of-recovery checkpoint is triggered but not finished at the end of
recovery (possible of course at the end of crash recovery, but
DB_IN_ARCHIVE_RECOVERY is also possible when !IsUnderPostmaster with a
promotion request), before updating ControlFile->checkPointCopy at the
end of the checkpoint because the state could still be
DB_IN_ARCHIVE_RECOVERY. The window is wider the longer the
end-of-recovery checkpoint. And this would be the case of an instance
restarted, when a restart point is created.
> Heikki was planning to commit a large refactoring of xlog.c, so we'd
> better wait for that to happen before concluding here.
I have checked that as well, and both are independent.
--
Michael