Re: Move backup-related code to xlogbackup.c/.h

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Move backup-related code to xlogbackup.c/.h
Date: 2022-10-06 12:24:15
Message-ID: CALj2ACXqwhYAL8AxMNBxrOLK_AU3-cv4JGHyq4xKX2jKcvV3xg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 6, 2022 at 4:50 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> I'm doubtful it's a good idea to expose these to outside of xlog.c - they are
> very low level, and it's very easy to break stuff by using them wrongly.

Hm. Here's the v3 patch set without exposing WAL insert lock related
functions. Please have a look.

On Thu, Oct 6, 2022 at 4:22 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> Can we also replace the relevant code with calls to these functions in
> 0001? That way, we can more easily review the changes you are making to
> this code separately from the large patch that just moves the code.

Done. Please have a look at 0001.

On Wed, Oct 5, 2022 at 11:28 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2022-Oct-05, Michael Paquier wrote:
>
> > And FWIW, the SQL interfaces for pg_backup_start() and
> > pg_backup_stop() could stay in xlogfuncs.c. This has the advantage to
> > centralize in the same file all the SQL-function-specific checks.
>
> As I recall, that has the disadvantage that the API exposure is a bit
> higher -- I mean, with the patch as originally posted, there was less
> cross-inclusion of header files, but that is gone in the version Bharat
> posted as reply to this. I'm not sure if that's caused by *this*
> comment, or even that it's terribly significant, but it seems worth
> considering at least.

FWIW, I'm attaching 0003 patch for moving backup functions from
xlogfuncs.c to xlogbackup.c. It's natural to have them there when
we're moving backup related things, this also reduces backup code
footprint. We can leave xlogfuncs.c for WAL related SQL-callable
functions.

Please review the attached v3 patch set.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v3-0001-Add-functions-for-xlogbackup.c-to-call-back-into-.patch application/x-patch 10.7 KB
v3-0002-Move-backup-related-code-from-xlog.c-to-xlogbacku.patch application/x-patch 53.9 KB
v3-0003-Move-backup-related-code-from-xlogfuncs.c-to-xlog.patch application/x-patch 10.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2022-10-06 12:28:27 Re: Record SET session in VariableSetStmt
Previous Message Drouvot, Bertrand 2022-10-06 12:19:32 Re: Record SET session in VariableSetStmt