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
