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). Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
