Re: POC: enable logical decoding when wal_level = 'replica' without a server restart - Mailing list pgsql-hackers

On Sun, Sep 21, 2025 at 8:40 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Fri, Sep 19, 2025 at 10:49 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Sep 19, 2025 at 7:45 AM Ashutosh Bapat
> > <ashutosh.bapat.oss@gmail.com> wrote:
> > >
> > > On Fri, Sep 12, 2025 at 2:26 PM Ashutosh Bapat
> > > <ashutosh.bapat.oss@gmail.com> wrote:
> > > >
> > > > On Fri, Sep 12, 2025 at 9:38 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > >
> > > > > One thing related to this which needs a discussion is after this
> > > > > change, it is possible that part of the transaction contains
> > > > > additional logical_wal_info. I couldn't think of a problem due to this
> > > > > but users using pg_waldump or other WAL reading utilities could
> > > > > question this. One possibility is that we always start including
> > > > > logical_wal_info for the next new transaction but not sure if that is
> > > > > required. It would be good if other people involved in the discussion
> > > > > or otherwise could share their opinion on this point.
> > > > >
> > > >
> > > > AFAIR, logical info is a separate section in a WAL record, and there
> > > > is not marker which says "WAL will contain logical info henceforth".
> > > > So the utilities should be checking for the existence of such info
> > > > before reading it. So I guess it should be ok. Some extra sensitive
> > > > utilities may expect that once a WAL record has logical info, all the
> > > > succeeding WAL records will have it. They may find it troublesome that
> > > > WAL records with and without logical info are interleaved. Generally,
> > > > I would prefer that presence/absence of logical info changes at
> > > > transaction boundaries, but we will still have interleaving WAL
> > > > records. So I doubt how much that matters.
> > > >
> > > > Sorry for jumping late in the discussion. I have a few comments,
> > > > mostly superficial ones. I am yet to take a deeper look at the
> > > > synchronization logic.
> > >
> > > I started looking at the synchronization logic but stumbled at
> > >
> > > @@ -5100,6 +5139,7 @@ BootStrapXLOG(uint32 data_checksum_version)
> > > checkPoint.ThisTimeLineID = BootstrapTimeLineID;
> > > checkPoint.PrevTimeLineID = BootstrapTimeLineID;
> > > checkPoint.fullPageWrites = fullPageWrites;
> > > + checkPoint.logicalDecodingEnabled = IsLogicalDecodingEnabled();
> > >
> > > At the time of bootstrapping, logical decoding is solely dependent on
> > > the boot_val of wal_level as there will not be any logical slots.
> > > Above code however does not make this clear. If we were to change the
> > > boot value of wal_level to logical this leads to a misleading
> > > CHECKPOINT_SHUTDOWN record being added at the time of bootstrap like
> > > below.
> > > rmgr: XLOG len (rec/tot): 122/ 122, tx: 0, lsn: 0/01000028, prev
> > > 0/00000000, desc: CHECKPOINT_SHUTDOWN redo 0/01000028; tli 1; prev tli
> > > 1; fpw true; wal_level logical; logical decoding false; xid 0:3; oid
> > > 10000; multi 1; offset 0; oldest xid 3 in DB 1; oldest multi 1 in DB
> > > 1; oldest/newest commit timestamp xid: 0/0; oldest running xid 0;
> > > shutdown
> > >
> > > This soon gets corrected by the following WAL record
> > > rmgr: XLOG len (rec/tot): 27/ 27, tx: 0, lsn: 0/010000A8, prev
> > > 0/01000028, desc: LOGICAL_DECODING_STATUS_CHANGE true
> > >
> > > So beyond misleading a code reader or someone who is reading the WAL,
> > > this does not have any functional impact. But maybe we should consider
> > > making this a bit more clear by setting
> > > checkPoint.logicalDecodingEnabled based on wal_level in
> > > BootStrapXLOG(). Whether we change the code or not, I think we should
> > > add a comment to explain this code.
> >
> > I agree that calling IsLogicalDecodingEnabled() in BootStrapXLOG()
> > could be quite confusing. I think we can directly set false there and
> > add some comments for those who try to change the default wal_level
> > value.
>
> Or just set the value based on the wal_level.

Agreed.

I've attached the updated patch. It incorporates all comments I got so
far and implements to lazily disable logical decoding. It's used only
when the process tries to disable logical decoding during process
exit.

Regards,

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