| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Refactor replication origin state reset helpers |
| Date: | 2026-01-15 00:00:31 |
| Message-ID: | CAEoWx2mk4AS6H39PRs8JNS4ZaCMfZbbKqddConBmTcOGnLGtdA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
>
> Thank you for updating the patch.
>
> I'm not even sure that we need to have setter functions like
> replorigin_xact_set_{origin,lsn_timestamp} given that
> replorigin_xact_state is exposed. While the reset helper function
> helps us as it removes duplicated codes and some potential accidents
> like wrongly setting -1 as an invalid timestamp etc. these setter
> functions don't so much. So I think we can have the patch just
> consolidating the separated variables. What do you think?
>
>
No problem. I reverted the two helpers in v10.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0002-Consolidate-replication-origin-session-globals-i.patch | application/octet-stream | 21.5 KB |
| v10-0001-Refactor-replication-origin-state-reset-helpers.patch | application/octet-stream | 5.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-01-15 00:01:38 | Re: Adding basic NUMA awareness |
| Previous Message | Amit Langote | 2026-01-14 23:57:36 | Re: Segmentation fault on proc exit after dshash_find_or_insert |