Re: Synchronizing slots from primary to standby

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