Re: Synchronizing slots from primary to standby

On Fri, Dec 1, 2023 at 11:17 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Friday, December 1, 2023 12:51 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> Hi,
>
> >
> > On Fri, Dec 1, 2023 at 9:40 AM Zhijie Hou (Fujitsu)
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Wednesday, November 29, 2023 5:12 PM Zhijie Hou (Fujitsu)
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > I was reviewing slotsync worker design and here
> > > are few comments on 0002 patch:
> >
> > Thanks for reviewing the patch.
> >
> > >
> > >
> > > 3. In synchronize_one_slot, do we need to skip the slot sync and drop if the
> > > local slot is a physical one ?
> > >
> >
> > IMO, if a local slot exists which is a physical one, it will be a user
> > created slot and in that case worker will error out on finding
> > existing slot with same name. And the case where local slot is
> > physical one but not user-created is not possible on standby (assuming
> > we have correct check on primary disallowing setting 'failover'
> > property for physical slot). Do you have some other scenario in mind,
> > which I am missing here?
>
> I was thinking about the race condition when it has confirmed that the slot is
> not a user created one and enter "sync_state == SYNCSLOT_STATE_READY" branch,
> but at this moment, if someone uses "DROP_REPLICATION_SLOT" to drop this slot and
> recreate another one(e.g. a physical one), then the slotsync worker will
> overwrite the fields of this physical slot. Although this affects user created
> logical slots in similar cases as well.
>

User can not drop the synced slots on standby. It should result in
ERROR. Currently we emit this error in pg_drop_replication_slot(),
same is needed in "DROP_REPLICATION_SLOT" replication cmd. I will
change it. Thanks for raising this point. I think, after this ERROR,
there is no need to worry about physical slots handling in
synchronize_one_slot().

> And the same is true for slotsync_drop_initiated_slots() and
> drop_obsolete_slots(), as we don't lock the slots in the list, if user tri to
> drop and re-create old slot concurrently, then we could drop user created slot
> here.
>