On 3/18/26 15:26, Michael Paquier wrote:
> On Wed, Mar 18, 2026 at 07:35:47AM +0000, David Steele wrote:
>> You are correct -- the copy of pg_control needs to happen before
>> do_pg_backup_stop(). An older version of this patch saved pg_control in
>> backup_state which made the prior location safe. However, I missed moving
>> this code when I moved pg_control out of backup_state. Code review to the
>> rescue.
>
> Right. I am wondering also if the final result would not be better
> without 0002, actually, focusing only on the "simpler" base backup
> case through the replication protocol, and you are making a good case
> in mentioning it as not absolutely mandatory for base backups that are
> taken through the SQL functions. One could always tweak the flag
> manually in the control file based on the contents taken from the data
> folder. That's more hairy than writing the entire file, for sure,
> still possible.
Getting even 01 into PG19 would be a great outcome. This would solve the
problem of torn pg_control and deleted backup labels for any backups
made with pg_basebackup and that's going to cover a *lot* of cases.
Established third-party backup solutions that are not based on
pg_basebackup are generally able to manipulate pg_control so that's not
as much of a concern, perhaps. It does raise the barrier of entry for
new backup software if they need to learn to read and validate
pg_control to avoid a torn copy and set the flag. Patch 02 solves that
problem in a general way so I still think it adds value for the
ecosystem -- but we could always discuss that in the PG20 cycle.
Whatever gets committed for PG19 I'll write a followup patch to describe
the hazards of reading pg_control and generally how to get a good copy.
However, this will be complicated enough that the best answer will
likely be to use pg_basebackup or some other reputable backup software.
I don't love this -- I feel like the low-level interface should be
usable with such hazards.
Regards,
-David
