Re: Add recovery to pg_control and remove backup_label

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: David Steele <david(at)pgmasters(dot)net>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add recovery to pg_control and remove backup_label
Date: 2023-10-26 21:27:52
Message-ID: CAKFQuwYPu+PMyNvwnvs0x_7c9X57igMj1ED-kdYhnF+GEZvqJg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 26, 2023 at 2:02 PM David Steele <david(at)pgmasters(dot)net> wrote:

> Hackers,
>
> This was originally proposed in [1] but that thread went through a
> number of different proposals so it seems better to start anew.
>
> The basic idea here is to simplify and harden recovery by getting rid of
> backup_label and storing recovery information directly in pg_control.
> Instead of backup software copying pg_control from PGDATA, it stores an
> updated version that is returned from pg_backup_stop(). I believe this
> is better for the following reasons:
>
> * The user can no longer remove backup_label and get what looks like a
> successful restore (while almost certainly causing corruption). If
> pg_control is removed the cluster will not start. The user may try
> pg_resetwal, but I think that tool makes it pretty clear that corruption
> will result from its use. We could also modify pg_resetwal to complain
> if recovery info is present in pg_control.
>
> * We don't need to worry about backup software seeing a torn copy of
> pg_control, since Postgres can safely read it out of memory and provide
> a valid copy via pg_backup_stop(). This solves [2] without needing to
> write pg_control via a temp file, which may affect performance on a
> standby. Unfortunately, this solution cannot be back patched.
>

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 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.

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.

> 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)

David J.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-10-26 21:28:32 Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
Previous Message Daniel Verite 2023-10-26 21:22:24 Re: Does UCS_BASIC have the right CTYPE?