Re: Add recovery to pg_control and remove backup_label

On 10/26/23 17:27, David G. Johnston wrote:
> On Thu, Oct 26, 2023 at 2:02 PM David Steele <david(at)pgmasters(dot)net
> <mailto:david(at)pgmasters(dot)net>> wrote:
>
> Are we planning on dealing with torn writes in the back branches in some
> way or are we just throwing in the towel and saying the old method is
> too error-prone to exist/retain

We are still planning to address this issue in the back branches.

> and therefore the goal of the v17
> changes is to not only provide a better way but also to ensure the old
> way no longer works?  It seems sufficient to change the output signature
> of pg_backup_stop to accomplish that goal though I am pondering whether
> an explicit check and error for seeing the backup_label file would be
> warranted.

Well, if the backup tool is just copying the second column of output to
the backup_label, then it won't break. Of course in that case, restores
won't work correctly but you would not get an error. Testing would show
that it is not working properly and backup tools should certainly be tested.

Even so, I'm OK with an explicit check for backup_label. Let's see what
others think.

> If we are going to solve the torn writes problem completely then while I
> agree the new way is superior, implementing it doesn't have to mean
> existing tools built to produce backup_label and rely upon the
> pg_control in the data directory need to be forcibly broken.

It is a pretty easy update to any backup software that supports
non-exclusive backup. I was able to make the changes to pgBackRest in
less than an hour. We've made major changes to backup and restore in
almost every major version of PostgreSQL for a while: non-exlusive
backup in 9.6, dir renames in 10, variable WAL size in 11, new recovery
location in 12, hard recovery target errors in 13, and changes to
non-exclusive backup and removal of exclusive backup in 15. In 17 we are
already looking at new page and segment sizes.

> I know that outputting pg_control as bytea is going to be a bit
> controversial. Software that is using psql get run pg_backup_stop()
> could use encode() to get pg_control as text and then decode it later.
> Alternately, we could update ReadControlFile() to recognize a
> base64-encoded pg_control file. I'm not sure dealing with binary
> data is
> that much of a problem, though, and if the backup software gets it
> wrong
> then recovery with fail on an invalid pg_control file.
>
> Can we not figure out some way to place the relevant files onto the
> server somewhere so that a simple "cp" command would work?  Have
> pg_backup_stop return paths instead of contents, those paths being
> "$TEMP_DIR"/<random unique new directory>/pg_control.conf (and
> tablespace_map)

Nobody has been able to figure this out, and some of us have been
thinking about it for years. It just doesn't seem possible to reliably
tell the difference between a cluster that was copied and one that
simply crashed.

If cp is really the backup tool being employed, I would recommend using
pg_basebackup. cp has flaws that could lead to corruption, and of course
does not at all take into account the archive required to make a backup
consistent, directories to be excluded, the order of copying pg_control
on backup from standy, etc., etc.

Backup/restore is not a simple endeavor and we don't do anyone favors
pretending that it is.

Regards,
-David