On Monday, February 12, 2024 5:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sun, Feb 11, 2024 at 6:53 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > Agreed. Here is the V84 patch which addressed this.
> >
>
> Few comments:
> =============
> 1. Isn't the new function (pg_sync_replication_slots()) allowed to sync the slots
> from physical standby to another cascading standby?
> Won't it be better to simply disallow syncing slots on cascading standby to keep
> it consistent with slotsync worker behavior?
>
> 2.
> Previously, I commented to keep the declaration and definition of functions in
> the same order but I see that it still doesn't match in the below case:
>
> @@ -44,6 +46,7 @@ extern void WalSndWakeup(bool physical, bool logical);
> extern void WalSndInitStopping(void); extern void WalSndWaitStopping(void);
> extern void HandleWalSndInitStopping(void);
> +extern XLogRecPtr GetStandbyFlushRecPtr(TimeLineID *tli);
> extern void WalSndRqstFileReload(void);
>
> I think we can keep the new declaration just before WalSndSignals().
> That would be more consistent.
>
> 3.
> + <para>
> + True if this is a logical slot that was synced from a primary server.
> + </para>
> + <para>
> + On a hot standby, the slots with the synced column marked as true can
> + neither be used for logical decoding nor dropped by the user.
> + The value
>
> I don't think we need a separate para here.
>
> Apart from this, I have made several cosmetic changes in the attached.
> Please include these in the next version unless you see any problems.
Thanks for the comments, I have addressed them.
Here is the new version patch which addressed above and
most of Bertrand's comments.
TODO: trying to add one test for the case the slot is valid on
primary while the synced slots is invalidated on the standby.
Best Regards,
Houzj
