Re: Add recovery to pg_control and remove backup_label

On 4/16/24 18:55, Stefan Fercot wrote: Hi, On Sunday, March 10th, 2024 at 4:47 AM, David Steele wrote: I've had a new idea which may revive this patch. The basic idea is to keep backup_label but also return a copy of pg_control from pg_stop_backup(). This copy of pg_control would be safe from

Re: Add recovery to pg_control and remove backup_label

Hi, On Sunday, March 10th, 2024 at 4:47 AM, David Steele wrote: > I've had a new idea which may revive this patch. The basic idea is to > keep backup_label but also return a copy of pg_control from > pg_stop_backup(). This copy of pg_control would be safe from tears and > have a

Re: Add recovery to pg_control and remove backup_label

On 1/29/24 12:28, David Steele wrote: On 1/28/24 19:11, Michael Paquier wrote: On Fri, Jan 26, 2024 at 06:27:30PM +0530, vignesh C wrote: Please post an updated version for the same. [1] - http://cfbot.cputube.org/patch_46_3511.log With the recent introduction of incremental backups that

Re: Add recovery to pg_control and remove backup_label

On 1/28/24 19:11, Michael Paquier wrote: On Fri, Jan 26, 2024 at 06:27:30PM +0530, vignesh C wrote: Please post an updated version for the same. [1] - http://cfbot.cputube.org/patch_46_3511.log With the recent introduction of incremental backups that depend on backup_label and the rather

Re: Add recovery to pg_control and remove backup_label

On Fri, Jan 26, 2024 at 06:27:30PM +0530, vignesh C wrote: > Please post an updated version for the same. > > [1] - http://cfbot.cputube.org/patch_46_3511.log With the recent introduction of incremental backups that depend on backup_label and the rather negative feedback received, I think that

Re: Add recovery to pg_control and remove backup_label

On Mon, 20 Nov 2023 at 06:46, Michael Paquier wrote: > > (I am not exactly sure how, but we've lost pgsql-hackers on the way > when you sent v5. Now added back in CC with the two latest patches > you've proposed attached.) > > Here is a short summary of what has been missed by the lists: > -

Re: Add recovery to pg_control and remove backup_label

Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Sun, Nov 26, 2023 at 3:42 AM Stephen Frost wrote: > > What would really be helpful would be hearing from these individuals > > directly as to what the issues are with the changes, such that perhaps > > we can do things better in the

Re: Add recovery to pg_control and remove backup_label

On Sun, Nov 26, 2023 at 3:42 AM Stephen Frost wrote: > What would really be helpful would be hearing from these individuals > directly as to what the issues are with the changes, such that perhaps > we can do things better in the future to avoid whatever the issue is > they're having with the

Re: Add recovery to pg_control and remove backup_label

Greetings, * David Steele (da...@pgmasters.net) wrote: > On 11/21/23 12:41, Andres Freund wrote: > > Sure. They also receive a backup_label today. If an external solution > > forgets > > to replace pg_control copied as part of the filesystem copy, they won't get > > an > > error after the

Re: Add recovery to pg_control and remove backup_label

On 11/21/23 16:00, Andres Freund wrote: Hi, On 2023-11-21 14:48:59 -0400, David Steele wrote: I'd not call 7.06->4.77 or 6.76->4.77 "virtually free". OK, but how does that look with compression With compression it's obviously somewhat different - but that part is done in parallel,

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-21 14:48:59 -0400, David Steele wrote: > > I'd not call 7.06->4.77 or 6.76->4.77 "virtually free". > > OK, but how does that look with compression With compression it's obviously somewhat different - but that part is done in parallel, potentially on a different machine with

Re: Add recovery to pg_control and remove backup_label

On 11/21/23 13:59, Andres Freund wrote: On 2023-11-21 13:41:15 -0400, David Steele wrote: On 11/20/23 16:41, Andres Freund wrote: On 2023-11-20 15:56:19 -0400, David Steele wrote: I understand this is an option -- but does it need to be? What is the benefit of excluding the manifest? It's

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 16:37, Andres Freund wrote: On 2023-11-20 11:11:13 -0500, Robert Haas wrote: I think we need more votes to make a change this big. I have a concern, which I think I've expressed before, that we keep whacking around the backup APIs, and that has a cost which is potentially larger

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-21 13:41:15 -0400, David Steele wrote: > On 11/20/23 16:41, Andres Freund wrote: > > > > On 2023-11-20 15:56:19 -0400, David Steele wrote: > > > I understand this is an option -- but does it need to be? What is the > > > benefit of excluding the manifest? > > > > It's not free to

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 16:41, Andres Freund wrote: On 2023-11-20 15:56:19 -0400, David Steele wrote: I understand this is an option -- but does it need to be? What is the benefit of excluding the manifest? It's not free to create the manifest, particularly if checksums are enabled. It's virtually

Re: Add recovery to pg_control and remove backup_label

On 11/21/23 12:41, Andres Freund wrote: On 2023-11-21 07:42:42 -0400, David Steele wrote: On 11/20/23 19:58, Andres Freund wrote: On 2023-11-21 08:52:08 +0900, Michael Paquier wrote: On Mon, Nov 20, 2023 at 12:37:46PM -0800, Andres Freund wrote: Given that, I wonder if what we should do is

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-21 07:42:42 -0400, David Steele wrote: > On 11/20/23 19:58, Andres Freund wrote: > > On 2023-11-21 08:52:08 +0900, Michael Paquier wrote: > > > On Mon, Nov 20, 2023 at 12:37:46PM -0800, Andres Freund wrote: > > > > Given that, I wonder if what we should do is to just add a new

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 19:58, Andres Freund wrote: On 2023-11-21 08:52:08 +0900, Michael Paquier wrote: On Mon, Nov 20, 2023 at 12:37:46PM -0800, Andres Freund wrote: Given that, I wonder if what we should do is to just add a new field to pg_control that says "error out if backup_label does not exist",

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 03:58:55PM -0800, Andres Freund wrote: > I was thinking we'd just set it in the pg_basebackup style path, and we'd > error out if it's set and backup_label is present. But we'd still use > backup_label without the pg_control flag set. > > So it'd just provide a cross-check

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-21 08:52:08 +0900, Michael Paquier wrote: > On Mon, Nov 20, 2023 at 12:37:46PM -0800, Andres Freund wrote: > > Given that, I wonder if what we should do is to just add a new field to > > pg_control that says "error out if backup_label does not exist", that we set > > when creating

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 12:37:46PM -0800, Andres Freund wrote: > Given that, I wonder if what we should do is to just add a new field to > pg_control that says "error out if backup_label does not exist", that we set > when creating a streaming base backup That would mean that one still needs to

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-20 14:18:15 -0700, David G. Johnston wrote: > On Mon, Nov 20, 2023 at 1:37 PM Andres Freund wrote: > > > > > Given that, I wonder if what we should do is to just add a new field to > > pg_control that says "error out if backup_label does not exist", that we > > set > > when

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 11:11:13AM -0500, Robert Haas wrote: > I think we need more votes to make a change this big. I have a > concern, which I think I've expressed before, that we keep whacking > around the backup APIs, and that has a cost which is potentially > larger than the benefits. The

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 1:37 PM Andres Freund wrote: > > Given that, I wonder if what we should do is to just add a new field to > pg_control that says "error out if backup_label does not exist", that we > set > when creating a streaming base backup > > I thought this was DOA since we don't want

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-20 15:56:19 -0400, David Steele wrote: > I understand this is an option -- but does it need to be? What is the > benefit of excluding the manifest? It's not free to create the manifest, particularly if checksums are enabled. Also, for external backups, there's no manifest... -

Re: Add recovery to pg_control and remove backup_label

Hi, On 2023-11-20 11:11:13 -0500, Robert Haas wrote: > I think we need more votes to make a change this big. I have a > concern, which I think I've expressed before, that we keep whacking > around the backup APIs, and that has a cost which is potentially > larger than the benefits. +1. The

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 15:47, Robert Haas wrote: On Mon, Nov 20, 2023 at 2:41 PM David Steele wrote: I can't see why a backup would continue to be valid without a manifest -- that's not very standard for backup software. If you have the critical info in backup_label, you can't afford to lose that, so why

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 2:41 PM David Steele wrote: > I can't see why a backup would continue to be valid without a manifest > -- that's not very standard for backup software. If you have the > critical info in backup_label, you can't afford to lose that, so why > should backup_manifest be any

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 14:44, Robert Haas wrote: On Mon, Nov 20, 2023 at 12:54 PM David Steele wrote: Another thing we could do is explicitly error if we see backup_label in PGDATA during recovery. That's just a few lines of code so would not be a big deal to maintain. This error would only be visible on

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 20, 2023 at 12:54 PM David Steele wrote: > Another thing we could do is explicitly error if we see backup_label in > PGDATA during recovery. That's just a few lines of code so would not be > a big deal to maintain. This error would only be visible on restore, so > it presumes that

Re: Add recovery to pg_control and remove backup_label

On 11/20/23 12:11, Robert Haas wrote: On Sun, Nov 19, 2023 at 8:16 PM Michael Paquier wrote: (I am not exactly sure how, but we've lost pgsql-hackers on the way when you sent v5. Now added back in CC with the two latest patches you've proposed attached.) Here is a short summary of what

Re: Add recovery to pg_control and remove backup_label

On Sun, Nov 19, 2023 at 8:16 PM Michael Paquier wrote: > (I am not exactly sure how, but we've lost pgsql-hackers on the way > when you sent v5. Now added back in CC with the two latest patches > you've proposed attached.) > > Here is a short summary of what has been missed by the lists: > -

Re: Add recovery to pg_control and remove backup_label

On 11/19/23 21:15, Michael Paquier wrote: (I am not exactly sure how, but we've lost pgsql-hackers on the way when you sent v5. Now added back in CC with the two latest patches you've proposed attached.) Ugh, I must have hit reply instead of reply all. It's a rookie error and you hate to see

Re: Add recovery to pg_control and remove backup_label

t_info *manifest, Oid spcoid, const char *pathname, size_t size, @@ -47,6 +48,8 @@ extern void AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr, TimeLineID starttli, XLogRecPtr endptr, TimeLineID endtli); +extern v

Re: Add recovery to pg_control and remove backup_label

On Fri, Nov 10, 2023 at 02:55:19PM -0400, David Steele wrote: > On 11/10/23 00:37, Michael Paquier wrote: >> I've done a few more dozen runs, and still nothing. I am wondering >> what this disturbance was. > > OK, hopefully it was just a blip. Still nothing on this side. So that seems really

Re: Add recovery to pg_control and remove backup_label

t struct for the backup_label file we're injecting in -* the tar. +* Construct a stat struct for the file we're injecting in the tar. */ /* Windows doesn't have the concept of uid and gid */ #ifdef WIN32 -- 2.34.1 From 79c33f300dd5bb9aabd08f27d2e6bb5857190524 Mon Sep 17 00:00

Re: Add recovery to pg_control and remove backup_label

On Tue, Nov 07, 2023 at 05:20:27PM +0900, Michael Paquier wrote: > On Mon, Nov 06, 2023 at 05:39:02PM -0400, David Steele wrote: > I've retested today, and miss the failure. I'll let you know if I see > this again. I've done a few more dozen runs, and still nothing. I am wondering what this

Re: Add recovery to pg_control and remove backup_label

On Mon, Nov 06, 2023 at 05:39:02PM -0400, David Steele wrote: > On 11/6/23 02:35, Michael Paquier wrote: >> The patch is failing the recovery test 039_end_of_wal.pl. Could you >> look at the failure? > > I'm not seeing this failure, and CI seems happy [1]. Can you give details of > the error

Re: Add recovery to pg_control and remove backup_label

On 11/6/23 02:35, Michael Paquier wrote: On Sun, Nov 05, 2023 at 01:45:39PM -0400, David Steele wrote: Rebased on 151ffcf6. I like this patch a lot. Even if the backup_label file is removed, we still have all the debug information from the backup history file, thanks to its LABEL, BACKUP

Re: Add recovery to pg_control and remove backup_label

On 11/6/23 01:05, Michael Paquier wrote: On Fri, Oct 27, 2023 at 10:10:42AM -0400, David Steele wrote: We are still planning to address this issue in the back branches. FWIW, redesigning the backend code in charge of doing base backups in the back branches is out of scope. Based on a read of

Re: Add recovery to pg_control and remove backup_label

On Sun, Nov 05, 2023 at 01:45:39PM -0400, David Steele wrote: > Rebased on 151ffcf6. I like this patch a lot. Even if the backup_label file is removed, we still have all the debug information from the backup history file, thanks to its LABEL, BACKUP METHOD and BACKUP FROM, so no information is

Re: Add recovery to pg_control and remove backup_label

On Fri, Oct 27, 2023 at 10:10:42AM -0400, David Steele wrote: > We are still planning to address this issue in the back branches. FWIW, redesigning the backend code in charge of doing base backups in the back branches is out of scope. Based on a read of the proposed patch, it includes catalog

Re: Add recovery to pg_control and remove backup_label

Rebased on 151ffcf6.diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 8cb24d6ae54..6be8fb902c5 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -935,19 +935,20 @@ SELECT * FROM pg_backup_stop(wait_for_archive => true); ready to archive.

Re: Add recovery to pg_control and remove backup_label

On 10/27/23 13:45, David G. Johnston wrote: Let me modify that to make it a bit more clear, I actually wouldn't care if pg_backup_end outputs an entire binary pg_control file as part of the SQL resultset. My proposal would be to, in addition, place in the temporary directory on the server,

Re: Add recovery to pg_control and remove backup_label

On Fri, Oct 27, 2023 at 7:10 AM David Steele wrote: > On 10/26/23 17:27, David G. Johnston wrote: > > > 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,

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

Re: Add recovery to pg_control and remove backup_label

On Thu, Oct 26, 2023 at 2:02 PM David Steele 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

Add recovery to pg_control and remove backup_label

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