Re: The danger of deleting backup_label

On 10/12/23 10:19, David Steele wrote:
> On 10/11/23 18:10, Thomas Munro wrote:
>
>> As Stephen mentioned[1], we could perhaps also complain if both backup
>> label and control file exist, and then hint that the user should
>> remove the *control file* (not the backup label!).  I had originally
>> suggested we would just overwrite the control file, but by explicitly
>> complaining about it we would also bring the matter to tool/script
>> authors' attention, ie that they shouldn't be backing that file up, or
>> should be removing it in a later step if they copy everything.  He
>> also mentions that there doesn't seem to be anything stopping us from
>> back-patching changes to the backup label contents if we go this way.
>> I don't have a strong opinion on that and we could leave the question
>> for later.
>
> I'm worried about the possibility of back patching this unless the
> solution comes out to be simpler than I think and that rarely comes to
> pass. Surely throwing errors on something that is currently valid (i.e.
> backup_label and pg_control both present).
>
> But perhaps there is a simpler, acceptable solution we could back patch
> (transparent to all parties except Postgres) and then a more advanced
> solution we could go forward with.
>
> I guess I had better get busy on this.

Attached is a very POC attempt at something along these lines that could
be back patched. I stopped when I realized excluding pg_control from the
backup is a necessity to make this work (else we still could end up with
a torn copy of pg_control even if there is a good copy elsewhere). To
enumerate the back patch issues as I see them:

1) We still need a copy of pg_control in order to get Postgres to start
and that copy might be torn (pretty much where we are now). We can
handle this easily in pg_basebackup but most backup software will not be
able to do so. In my opinion teaching Postgres to start without
pg_control is too big a change to possibly back patch.

2) We need to deal with backups made with a prior *minor* version that
did not include pg_control in the backup_label. Doable, but...

3) We need to move backup_label to the end of the main pg_basebackup
tar, which could cause unforeseen breakage for tools.

4) This patch is less efficient for backups taken from standby because
it will overwrite pg_control on restart and force replay back to the
original MinRecoveryPoint.

5) We still need a solution for exclusive backup (still valid in PG <=
14). Doable but it would still have the weakness of 1.

All of this is fixable in HEAD, but seems incredibly dangerous to back
patch. Even so, I have attached the patch in case somebody sees an
opportunity that I do not.

Regards,
-David