Re: Minimal logical decoding on standbys

On Thu, Apr 6, 2023 at 11:29 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> >
>
> This doesn't seem to be addressed in the latest version. And today, I
> think I see one more point about this doc change:
> + <para>
> + A logical replication slot can also be created on a hot standby.
> To prevent
> + <command>VACUUM</command> from removing required rows from the system
> + catalogs, <varname>hot_standby_feedback</varname> should be set on the
> + standby. In spite of that, if any required rows get removed, the slot gets
> + invalidated. It's highly recommended to use a physical slot
> between the primary
> + and the standby. Otherwise, hot_standby_feedback will work, but
> only while the
> + connection is alive (for example a node restart would break it). Existing
> + logical slots on standby also get invalidated if wal_level on
> primary is reduced to
> + less than 'logical'.
>
> If hot_standby_feedback is not set then can logical decoding on
> standby misbehave? If so, that is not very clear from this doc change
> if that is acceptable. One scenario where I think it can misbehave is
> if applying WAL records generated after changing wal_level from
> 'logical' to 'replica' physically removes catalog tuples that could be
> referenced by the logical decoding on the standby. Now, as mentioned
> in patch 0003's comment in decode.c that it is possible that some
> slots may creep even after we invalidate the slots on parameter
> change, so while decoding using that slot if some required catalog
> tuple has been removed by physical replication then the decoding can
> misbehave even before reaching XLOG_PARAMETER_CHANGE record.
>

Thinking some more on this, I think such a slot won't decode any other
records. During CreateInitDecodingContext->ReplicationSlotReserveWal,
for standby's, we use lastReplayedEndRecPtr as restart_lsn. This
should be a record before parameter_change record in the above
scenario. So, ideally, the first record to decode by such a walsender
should be parameter_change which will anyway error out. So, this
shouldn't be a problem.

--
With Regards,
Amit Kapila.