On Thu, Feb 29, 2024 at 7:04 AM Zhijie Hou (Fujitsu)
<houzj.fnst@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