On Wed, Jan 28, 2026 at 2:34 PM Chao Li <[email protected]> wrote: > > > > > On Jan 29, 2026, at 04:57, Masahiko Sawada <[email protected]> wrote: > > > > On Fri, Jan 16, 2026 at 4:46 AM Ashutosh Bapat > > <[email protected]> wrote: > >> > >> On Fri, Jan 16, 2026 at 7:37 AM Chao Li <[email protected]> wrote: > >>> > >>> > >>> > >>> On Jan 15, 2026, at 17:51, Ashutosh Bapat <[email protected]> > >>> wrote: > >>> > >>> > >>> Hi Ashutosh, > >>> > >>> Thanks for your review again. > >>> > >>> > >>> On Thu, Jan 15, 2026 at 5:30 AM Chao Li <[email protected]> wrote: > >>> > >>> > >>> On Thu, Jan 15, 2026 at 2:56 AM Masahiko Sawada <[email protected]> > >>> wrote: > >>> > >>> > >>> On Sun, Jan 11, 2026 at 5:41 PM Chao Li <[email protected]> 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
