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