Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

On Tue, Oct 14, 2025 at 12:52 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Sawada-San,
>
> I started to look again at this thread. Here are some comments for v18
> (the documentation only).

Thank you for reviewing the patch!

>
> ======
> doc/src/sgml/config.sgml
>
> 1.
> + <para>
> + It is important to note that when
> <varname>wal_level</varname> is set to
> + <literal>replica</literal>, the effective WAL level can
> automatically change
> + based on the presence of <link
> linkend="logicaldecoding-replication-slots">
> + logical replication slots</link>. The system automatically
> increases the
> + effective WAL level to <literal>logical</literal> when
> creating the first
> + logical replication slot, and decreases it back to
> <literal>replica</literal>
> + when dropping the last logical replication slot. The current
> effective WAL
> + level can be monitored through <xref
> linkend="guc-effective-wal-level"/>
> + parameter.
> + </para>
>
>
> As you say "It is important to note...". So, should this all be using
> <note> SGML markup?

I'm not sure when we have to use <note> but I find that it's not
necessary here. I think we can change later if others think so too.

>
> ~~~
>
> 2.
> + <indexterm>
> + <primary><varname>effective_wal_level</varname> configuration
> parameter</primary>
> + </indexterm>
>
> Should this even be called a "configuration parameter", given that you
> can't configure it?

I think we call even pre-set GUC parameters "configuration
parameters", so it should follow.

>
> ======
> doc/src/sgml/logicaldecoding.sgml
>
> 3.
> + <para>
> + When <varname>wal_level</varname> is set to <literal>replica</literal>,
> + logical decoding is automatically activated upon creation of the first
> + logical replication slot. This activation process involves several steps
> + and requires waiting for any concurrent transactions to finish, ensuring
> + system-wide consistency. Conversely, when the last logical
> replication slot
> + is dropped from a system with <varname>wal_level</varname> set to
> + <literal>replica</literal>, logical decoding is automatically disabled.
> + Note that the deactivation of logical decoding might take some time as
> + it is performed asynchronously by the checkpointer process.
> + </para>
>
> Here it seems to be describing again the concept of the
> "effective_wal_level" as in the config.sgml, so should this paragraph
> also make mention of that terminology, and link to that concept like
> was done before?

Could you elaborate on why we need to mention effective_wal_level
here? I thought that in this section, it's sufficient to describe what
conditions are required to enable or disable logical decoding.

> ~~~
>
> 4.
> Existing logical slots on standby also get invalidated if
> - <varname>wal_level</varname> on the primary is reduced to less than
> - <literal>logical</literal>.
> + logical decoding is disabled on the primary.
>
> Would it be easier just to leave the text as it was, but say
> "effective_wal_level" instead of "wal_level"?

I find that saying "if logical decoding is disabled" sounds more
understandable for users than saying "if effective_wal_level is
reduced to less than logical", but what do you think?

> ======
> doc/src/sgml/system-views.sgml
>
> 5.
> <para>
> <literal>wal_level_insufficient</literal> means that the
> - primary doesn't have a <xref linkend="guc-wal-level"/> sufficient to
> - perform logical decoding. It is set only for logical slots.
> + logical decoding is disabled on primary (See
> + <xref linkend="logicaldecoding-explanation-log-dec"/>. It is set
> + only for logical slots.
> </para>
>
> 5a.
> Should this also make use of the "effective_wal_level" terminology?
>
> e.g
> "...means that the logical decoding is disabled on primary (i.e the
> effective_wal_level is not logical)"

What is the point of mentioning both "logical decoding is disabled"
and "effective_wal_level is not logical"? I think it isn't necessary
to mention both things in all the places where we mention logical
decoding availability, but I'll change it if others also think the
same.

>
> ~
>
> 5b.
> Would it be better to list all these reasons in alphabetical order?
>
> ~
>
> 5c.
> I felt the "It is set only for logical slots" to be a bit vague. IMO
> it is clearer to say "This reason value is only used for logical
> slots".
>
> Alternatively, maybe the whole list should be restructured like below
>
> SUGGESTION
> The reason for the slot's invalidation. NULL if the slot is not invalidated.
>
> Possible reason values for logical or physical slots are:
> - wal_removed means ...
> - idle_timeout means ...
>
> Possible reason values, only for logical slots are:
> - rows_removed means ...
> - wal_level_insufficient means ...

I find that those changes would be a general improvement for the
description of invalidation_reason. It should be discussed in a
separate thread.

>
> ~
>
> 5d.
> A missing closing parenthesis. "(See xxx"

Fixed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com