On Mon, Feb 12, 2024 at 3:33 PM Bertrand Drouvot <bertranddrouvot...@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.