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.
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.
Best Regards,
Hou zj
