Re: Refactor replication origin state reset helpers

On Tue, Dec 30, 2025 at 1:07 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

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

Fixed a bug in v4.

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