On Thursday, February 29, 2024 7:36 PM shveta malik <shveta.ma...@gmail.com> wrote: > > On Thu, Feb 29, 2024 at 7:04 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> > wrote: > > > > Here is the v101 patch set which addressed above comments. > > > > Thanks for the patch. Few comments: > > 1) Shall we mention in doc that shutdown will wait for standbys in > standby_slot_names to confirm receiving WAL: > > Suggestion for logicaldecoding.sgml: > > When <varname>standby_slot_names</varname> is utilized, the primary > server will not completely shut down until the corresponding standbys, > associated with the physical replication slots specified in > <varname>standby_slot_names</varname>, have confirmed receiving the > WAL up to the latest flushed position on the primary server. > > slot.c > 2) > /* > * If a logical slot name is provided in standby_slot_names, report > * a message and skip it. Although logical slots are disallowed in > * the GUC check_hook(validate_standby_slots), it is still > * possible for a user to drop an existing physical slot and > * recreate a logical slot with the same name. > */ > > This is not completely true, we can still specify a logical slot during > instance > start and it will accept it. > > Suggestion: > /* > * If a logical slot name is provided in standby_slot_names, report > * a message and skip it. It is possible for user to specify a > * logical slot name in standby_slot_names just before the server > * startup. The GUC check_hook(validate_standby_slots) can not > * validate such a slot during startup as the ReplicationSlotCtl > * shared memory is not initialized by that time. It is also > * possible for user to drop an existing physical slot and > * recreate a logical slot with the same name. > */ > > 3. Wait for physical standby to confirm receiving the given lsn > > standby -->standbys > > > 4. > In StandbyConfirmedFlush(), is it better to have below errdetail in all > problematic cases: > Logical replication is waiting on the standby associated with \"%s\ > > We have it only for inactive pid case but we are waiting in all cases.
Thanks for the comments, I have addressed them. Best Regards, Hou zj
