Re: Refactor replication origin state reset helpers

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Refactor replication origin state reset helpers
Date: 2025-12-30 05:07:44
Message-ID: CAEoWx2mT+gu8H-KhaeNodk4T7csX-_wwqRew=-t3zo6b6jXfaQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 30, 2025 at 12:48 PM Ashutosh Bapat <
ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:

> On Mon, Dec 29, 2025 at 8:14 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de>
> wrote:
> >
> > On 2025-Dec-24, Ashutosh Bapat wrote:
> >
> > > If we go this route, we at least need to declare the new functions as
> > > static inline and move them to a header file instead of .c file.
> >
> > Hmm, why would we make them static inline instead of standard (extern)
> > functions? We use static inline functions when we want to avoid the
> > overhead of a function call in a hot code path, but I doubt that's the
> > case here. Am I mistaken on this?
> >
>
> I wasn't aware that we are using static inline only in hot code paths.
> Looking around I see most of the static inline functions are from
> modules which are used in hot code paths. So, yeah that seems to be
> the convention. I also see some exceptions like those in
> basebackup_sink.h - I don't think all of those are used in hot code
> paths.
>
> In this case, we are moving three assignments into their own
> functions. CPU instructions to call extern functions will be
> significant compared to CPU instructions for those assignments. static
> inline functions, OTOH, would have similar performance as the existing
> code while providing modularization. If you feel that's not a good
> enough reason, I am ok keeping them extern.
>
> > > Further, does it make sense to put together all the state variables
> > > into a single structure?
> >
> > Yeah -- keeping the threaded-backend project in mind, moving them to a
> > single struct seems to make sense. I think it's a separate patch though
> > because it'd be more invasive than Chao's initial patch, as those
> > variables are used in many places.
> >
>

Attached v3 patch set. Comparing to v2, the changes are:

0001:
* Combine the two cleanup functions into one and control them by a bool
flag.
* Change the helper function to be extern.
* Move out cleanup from reset function.

0002: Consolidate replication origin session globals into a single state
struct.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachment Content-Type Size
v3-0001-Refactor-replication-origin-state-reset-helpers.patch application/octet-stream 5.5 KB
v3-0002-Consolidate-replication-origin-session-globals-in.patch application/octet-stream 18.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bryan Green 2025-12-30 05:07:58 Re: [PATCH] Allow complex data for GUC extra.
Previous Message Ashutosh Bapat 2025-12-30 04:47:34 Re: Refactor replication origin state reset helpers