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 Shveta
