Re: POC: enable logical decoding when wal_level = 'replica' without a server restart - Mailing list pgsql-hackers
From | Masahiko Sawada |
---|---|
Subject | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
Date | |
Msg-id | CAD21AoAT5zy2GrPv_5qhrjhzCjoOXS1WeFAn1TG3A5KFC4UFQQ@mail.gmail.com Whole thread Raw |
In response to | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart (Peter Smith <smithpb2250@gmail.com>) |
List | pgsql-hackers |
On Wed, Oct 15, 2025 at 2:17 PM Peter Smith <smithpb2250@gmail.com> wrote: > > On Thu, Oct 16, 2025 at 7:18 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > On Tue, Oct 14, 2025 at 12:52 AM Peter Smith <smithpb2250@gmail.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 > > > > > > > > > > > ====== > > > 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. > > > > (re my above comments about effective_wal_level) > > When reading the patched documentation, it just seemed to me that > although there is the introduced concept / parameter for > "effective_wal_level", it is hardly used at all except buried away > with just a few mentions in the decoding/config sections. In > particular, I was surprised that "effective_wal_level" was not > mentioned once in the entire "Logical Replication" section of the > docs. > > I felt this new parameter needed to be referred to much more often > (with an xref to it). Just saying "if logical decoding is disabled" by > itself doesn't convey anything about the "effective" wal level; a user > who hasn't stumbled across those rare mentions of > "effective_wal_level" might easily think that wal_level=replica always > means "logical decoding is disabled", because that's what it always > used to mean until this patch. OTOH, referring to > "effective_wal_level" (with an xref to it) makes it so much clearer. > > BTW, I repeated similar comments in my subsequent code review for > things like code-comments and error messages. Basically, I think > saying "logical decoding is enabled/disabled" sounds good, but it just > doesn't convey as much meaning that "effective_wal_level is logical" > has. > Understood. I agree that we can mention effective_wal_level somewhere in the Logical Decoding section. I'll try to add it. FYI since logical decoding availability is decoupled from wal_level being 'logical', it's not necessarily true that if effective_wal_level is 'logical' then logical decoding is available. The opposite is always true though. For example, during the activation, we increase effective_wal_level to 'logical' while keeping logical decoding disabled, and then enable logical decoding. My intention is to avoid mentioning both things together as if they are the same, because they're not exactly the equivalent conditions. In user-facing error messages and the documentations I preferably used "logical decoding" in places where either term can work well as it's more understandable for users. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
pgsql-hackers by date: