On 1/25/24 04:12, Michael Paquier wrote:
On Mon, Jan 22, 2024 at 04:36:27PM +0900, Michael Paquier wrote:
+ if (ControlFile->backupStartPoint != InvalidXLogRecPtr)
Nit 1: I would use XLogRecPtrIsInvalid here.
+ ereport(LOG,
+ (errmsg("completed backup recovery with redo LSN %X/%X",
+ LSN_FORMAT_ARGS(oldBackupStartPoint))));
Nit 2: How about adding backupEndPoint in this LOG? That would give:
"completed backup recovery with redo LSN %X/%X and end LSN %X/%X".
Hearing nothing, I've just applied a version of the patch with these
two modifications on HEAD. If this needs tweaks, just let me know.
I had planned to update the patch this morning -- so thanks for doing
that. I think having the end point in the message makes perfect sense.
I would still advocate for a back patch here. It is frustrating to get
logs from users that just say:
LOG: invalid checkpoint record
PANIC: could not locate a valid checkpoint record
It would be very helpful to know what the checkpoint record LSN was in
this case.
Regards,
-David
