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

Hi Sawada-San,

I started to look again at this thread. Here are some comments for v18
(the documentation only).

======
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?

~~~

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?

======
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?

~~~

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"?

======
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)"

~

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 ...

~

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

======
Kind Regards,
Peter Smith.
Fujitsu Australia.