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

Attachment: signature.asc
Description: PGP signature

Reply via email to