On Fri, Dec 1, 2023 at 11:17 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Friday, December 1, 2023 12:51 PM shveta malik <shveta.ma...@gmail.com> > wrote: > > Hi, > > > > > On Fri, Dec 1, 2023 at 9:40 AM Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> wrote: > > > > > > On Wednesday, November 29, 2023 5:12 PM Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.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. >
