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