On Mon, Feb 12, 2024 at 3:33 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> A few random comments:
>
>
> 003 ===
>
> + If, after executing the function,
> + <link linkend="guc-hot-standby-feedback">
> + <varname>hot_standby_feedback</varname></link> is disabled on
> + the standby or the physical slot configured in
> + <link linkend="guc-primary-slot-name">
> + <varname>primary_slot_name</varname></link> is
> + removed,
>
> I think another option that could lead to slot invalidation is if primary_slot_name
> is NULL or miss-configured.
>
If the primary_slot_name is NULL then the function will error out. So,
not sure, if we need to say anything explicitly here.
> Indeed hot_standby_feedback would be working
> (for the catalog_xmin) but only as long as the standby is up and running.
>
...
>
> 005 ===
>
> + To resume logical replication after failover from the synced logical
> + slots, the subscription's 'conninfo' must be altered
>
> Only in a pub/sub context but not for other ways of using the logical replication
> slot(s).
>
Right, but what additional information do you want here? I thought we
were speaking about the in-build logical replication here so this is
okay.
>
> 008 ===
>
> + ereport(LOG,
> + errmsg("dropped replication slot \"%s\" of dbid %d",
> + NameStr(local_slot->data.name),
> + local_slot->data.database));
>
> We emit a message when an "invalidated" slot is dropped but not when we create
> a slot. Shouldn't we emit a message when we create a synced slot on the standby?
>
> I think that could be confusing to see "a drop" message not followed by "a create"
> one when it's expected (slot valid on the primary for example).
>
Isn't the below message for sync-ready slot sufficient? Otherwise, in
most cases, we will LOG multiple similar messages.
+ ereport(LOG,
+ errmsg("newly created slot \"%s\" is sync-ready now",
+ remote_slot->name));
--
With Regards,
Amit Kapila.