Re: Refactor replication origin state reset helpers - Mailing list pgsql-hackers

> On Jan 29, 2026, at 04:57, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Jan 16, 2026 at 4:46 AM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
>>
>> On Fri, Jan 16, 2026 at 7:37 AM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>>
>>> On Jan 15, 2026, at 17:51, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>>>
>>>
>>> Hi Ashutosh,
>>>
>>> Thanks for your review again.
>>>
>>>
>>> On Thu, Jan 15, 2026 at 5:30 AM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>> On Thu, Jan 15, 2026 at 2:56 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>>
>>>
>>> On Sun, Jan 11, 2026 at 5:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>>
>>> ---
>>> In origin.h:
>>>
>>> +/*
>>> + * Clear the per-transaction replication origin state.
>>> + *
>>> + * replorigin_session_origin is also cleared if clear_origin is set.
>>> + */
>>> +static inline void
>>> +replorigin_xact_clear(bool clear_origin)
>>> +{
>>> +   replorigin_xact_state.origin_lsn = InvalidXLogRecPtr;
>>> +   replorigin_xact_state.origin_timestamp = 0;
>>> +   if (clear_origin)
>>> +       replorigin_xact_state.origin = InvalidRepOriginId;
>>> +}
>>>
>>> Why does this function need to move to origin.h from origin.c?
>>>
>>>
>>> That’s because, per Ashutosh’s suggestion, I added two static inline helpers replorigin_xact_set_origin() and
replorigin_xact_set_lsn_timestamp(),and I thought replorigin_xact_clear() should stay close with them. 
>>>
>>> But looks like they don’t have to be inline as they are not on hot paths. So I moved them all to origin.c and only
externthem. 
>>>
>>>
>>> I am fine with it being non-static-inline.
>>>
>>> +/*
>>> + * Clear the per-transaction replication origin state.
>>> + *
>>> + * replorigin_session_origin is also cleared if clear_origin is set.
>>> + */
>>> +void
>>> +replorigin_xact_clear(bool clear_origin)
>>>
>>> Nitpick. This file exposes a few functions. The function with name
>>> replogrigin_* deal with replication origin itself. The functions with
>>> name replorigin_session_* deal with the session state of replication
>>> origin. It feels like the new function is dealing with per-transaction
>>> state within a given session. Does it make sense to name it
>>> replorigin_session_xact_clear() instead of replorigin_xact_clear()?
>>> Just a thought.
>>>
>>>
>>> We’ve already gone back and forth on this function for several rounds, so I’d prefer not to make further changes
here.
>>>
>>
>> Ok. Let's leave this to committer's judgement.
>>
>>>
>>> Hopefully, v11 is ready to go.
>>
>> I don't have any further comments.
>>
>
> Pushed these patches after adjusting them based on the recent commit
> 1fdbca159e0 (changing RepOrigin -> ReplOrigin).
>

Thank you so much for taking care of this patch. I thought to rebase once you pushed the rename patch, you have handled
thatfor me. 

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