Re: The danger of deleting backup_label

On Thu, Sep 28, 2023 at 05:14:22PM -0400, David Steele wrote:
> While reading through [1] I saw there were two instances where backup_label
> was removed to achieve a "successful" restore. This might work on trivial
> test restores but is an invitation to (silent) disaster in a production
> environment where the checkpoint stored in backup_label is almost certain to
> be earlier than the one stored in pg_control.

Definitely successful.

> Recovery worked perfectly as long as backup_label was present and failed
> hard when it was not:
>
> LOG: invalid primary checkpoint record
> PANIC: could not locate a valid checkpoint record
>
> It's not a very good message, but at least the foot gun has been removed. We
> could use this as a special value to give a better message, and maybe use
> something a bit more unique like 0xFFFFFFFFFADEFADE (or whatever) as the
> value.

Why not just InvalidXLogRecPtr?

> This is all easy enough for pg_basebackup to do, but will certainly be
> non-trivial for most backup software to implement. In [2] we have discussed
> perhaps returning pg_control from pg_backup_stop() for the backup software
> to save, or it could become part of the backup_label (encoded as hex or
> base64, presumably). I prefer the latter as this means less work for the
> backup software (except for the need to exclude pg_control from the backup).
>
> I don't have a patch for this yet because I did not test this idea using
> pg_basebackup, but I'll be happy to work up a patch if there is interest.

If the contents of the control file are tweaked before sending it
through a BASE_BACKUP, it would cover more than just pg_basebackup.
Switching the way the control file is sent with new contents in
sendFileWithContent() rather than sendFile() would be one way, for
instance..
--
Michael