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

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.


--
Best Wishes,
Ashutosh Bapat