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
signature.asc
Description: PGP signature