Re: Add checkpoint and redo LSN to LogCheckpointEnd log message - Mailing list pgsql-hackers

On 2022/02/14 14:40, Kyotaro Horiguchi wrote:
> For backbranches, the attached for pg14 does part of the full patch.

Thanks for updating the patch!


> Of the following, I think we should do (a) and (b) to make future
> backpatchings easier.
> 
> a) Use RedoRecPtr and PriorRedoPtr after they are assigned.
> 
> b) Move assignment to PriorRedoPtr into the ControlFileLock section.

I failed to understand how (a) and (b) can make the backpatching easier. How easy to backpatch seems the same whether
weapply (a) and (b) or not...
 


> c) Skip udpate of minRecoveryPoint only when the checkpoint gets old.

Yes.


> d) Skip call to UpdateCheckPointDistanceEstimate() when RedoRecPtr <=
>    PriorRedoPtr.

But "RedoRecPtr <= PriorRedoPtr" will never happen, will it? Because a restartpoint is skipped at the beginning of
CreateRestartPoint()in that case. If this understanding is right, the check of "RedoRecPtr <= PriorRedoPtr" is not
necessarybefore calling UpdateCheckPointDistanceEstimate().
 


+        ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+        ControlFile->minRecoveryPointTLI = 0;

Don't we need to update LocalMinRecoveryPoint and LocalMinRecoveryPointTLI after this? Maybe it's not necessary, but
ISTMthat it's safer and better to always update them whether the state is DB_IN_ARCHIVE_RECOVERY or not.
 


          if (flags & CHECKPOINT_IS_SHUTDOWN)
              ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;

Same as above. IMO it's safer and better to always update the state (whether the state is DB_IN_ARCHIVE_RECOVERY or
not)if CHECKPOINT_IS_SHUTDOWN flag is passed.
 

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION