Re: Refactor replication origin state reset helpers

On Wed, Jan 28, 2026 at 2:34 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Jan 29, 2026, at 04:57, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Fri, Jan 16, 2026 at 4:46 AM Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >>
> >> On Fri, Jan 16, 2026 at 7:37 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>>
> >>>
> >>>
> >>> On Jan 15, 2026, at 17:51, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >>>
> >>>
> >>> Hi Ashutosh,
> >>>
> >>> Thanks for your review again.
> >>>
> >>>
> >>> On Thu, Jan 15, 2026 at 5:30 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>>
> >>>
> >>> On Thu, Jan 15, 2026 at 2:56 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >>>
> >>>
> >>> On Sun, Jan 11, 2026 at 5:41 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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 extern them.
> >>>
> >>>
> >>> 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 that for me.

I applied the change myself as it was a simple replacement. Thanks for
working on this!

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com