Re: Add system identifier to backup manifest - Mailing list pgsql-hackers

From Amul Sul
Subject Re: Add system identifier to backup manifest
Date
Msg-id CAAJ_b97_9nJCD3kAmNfSnN1s7m9n1h97OP=APwts=g-rWGztxQ@mail.gmail.com
Whole thread Raw
In response to Re: Add system identifier to backup manifest  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Add system identifier to backup manifest
Re: Add system identifier to backup manifest
List pgsql-hackers

On Mon, Feb 19, 2024 at 4:22 AM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Feb 15, 2024 at 05:41:46PM +0530, Robert Haas wrote:
> On Thu, Feb 15, 2024 at 3:05 PM Amul Sul <sulamul@gmail.com> wrote:
> > Kindly have a look at the attached version.
>
> IMHO, 0001 looks fine, except probably the comment could be phrased a
> bit more nicely.

And the new option should be documented at the top of the init()
routine for perldoc.

Added in the attached version.


> That can be left for whoever commits this to
> wordsmith. Michael, what are your plans?

Not much, so feel free to not wait for me.  I've just read through the
patch because I like the idea/feature :)
 
Thank you, that helped a lot.

> 0002 seems like a reasonable approach, but there's a hunk in the wrong
> patch: 0004 modifies pg_combinebackup's check_control_files to use
> get_dir_controlfile() rather than git_controlfile(), but it looks to
> me like that should be part of 0002.
 
Fixed in the attached version. 


I'm slightly concerned about 0002 that silently changes the meaning of
get_controlfile().  That would impact extension code without people
knowing about it when compiling, just when they run their stuff under
17~.

Agreed, now they will have an error as _could not read file "<DataDir>": Is a
directory_. But, IIUC, that what usually happens with the dev version, and the
extension needs to be updated for compatibility with the newer version for the
same reason.

Regards,
Amul




Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Fix race condition in InvalidatePossiblyObsoleteSlot()
Next
From: Amit Kapila
Date:
Subject: Re: A new message seems missing a punctuation