Re: Synchronizing slots from primary to standby - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: Synchronizing slots from primary to standby
Date
Msg-id CAD21AoBLJsjLByNJVAeEW7ODDAP0vYfPjUXDy8WP9DV=t1tDEQ@mail.gmail.com
Whole thread Raw
In response to RE: Synchronizing slots from primary to standby  ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>)
Responses Re: Synchronizing slots from primary to standby
List pgsql-hackers
On Fri, Feb 9, 2024 at 4:08 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Friday, February 9, 2024 2:44 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Thu, Feb 8, 2024 at 8:01 PM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > On Thu, Feb 8, 2024 at 12:08 PM Peter Smith <smithpb2250@gmail.com>
> > wrote:
> > > >
> > > > Here are some review comments for patch v80_2-0001.
> > >
> > > Thanks for the feedback Peter. Addressed the comments in v81.
> > > Attached patch001 for early feedback. Rest of the patches need
> > > rebasing and thus will post those later.
> > >
> > > It also addresses comments by Amit in [1].
> >
> > Thank you for updating the patch! Here are random comments:
>
> Thanks for the comments!
>
> >
> > ---
> > +
> > +        /*
> > +         * Register the callback function to clean up the shared memory of
> > slot
> > +         * synchronization.
> > +         */
> > +        SlotSyncInitialize();
> >
> > I think it would have a wider impact than expected. IIUC this callback is needed
> > only for processes who calls synchronize_slots(). Why do we want all processes
> > to register this callback?
>
> I think the current style is similar to the ReplicationSlotInitialize() above it. For backend,
> both of them can only be used when user calls slot SQL functions. So, I think it could be fine to
> register it at the general place which can also avoid registering the same again for the later
> slotsync worker patch.

Yes, but it seems to be a legitimate case since replication slot code
involves many functions that need the callback to clear the flag. On
the other hand, in the slotsync code, only one function,
SyncReplicationSlots(), needs the callback at least in 0001 patch.

> Another alternative is to register the callback when calling slotsync functions
> and unregister it after the function call. And register the callback in
> slotsyncworkmain() for the slotsync worker patch, although this may adds a few
> more codes.

Another idea is that SyncReplicationSlots() calls synchronize_slots()
in PG_ENSURE_ERROR_CLEANUP() block instead of PG_TRY(), to make sure
to clear the flag in case of ERROR or FATAL. And the slotsync worker
uses the before_shmem_callback to clear the flag.

Regards,

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



pgsql-hackers by date:

Previous
From: "Zhijie Hou (Fujitsu)"
Date:
Subject: RE: Synchronizing slots from primary to standby
Next
From: Tomas Vondra
Date:
Subject: Re: Thoughts about NUM_BUFFER_PARTITIONS