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

On Mon, Sep 8, 2025 at 11:14 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Sep 5, 2025 at 8:50 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Sep 4, 2025 at 1:24 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > 2.
> > > > - /*
> > > > - * Invalidate logical slots if we are in hot standby and the primary
> > > > - * does not have a WAL level sufficient for logical decoding. No need
> > > > - * to search for potentially conflicting logically slots if standby is
> > > > - * running with wal_level lower than logical, because in that case, we
> > > > - * would have either disallowed creation of logical slots or
> > > > - * invalidated existing ones.
> > > > - */
> > > > - if (InRecovery && InHotStandby &&
> > > > - xlrec.wal_level < WAL_LEVEL_LOGICAL &&
> > > > - wal_level >= WAL_LEVEL_LOGICAL)
> > > > - InvalidateObsoleteReplicationSlots(RS_INVAL_WAL_LEVEL,
> > > > -    0, InvalidOid,
> > > > -    InvalidTransactionId);
> > > > -
> > > >   LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
> > > >   ControlFile->MaxConnections = xlrec.MaxConnections;
> > > >   ControlFile->max_worker_processes = xlrec.max_worker_processes;
> > > > @@ -8605,6 +8643,50 @@ xlog_redo(XLogReaderState *record)
> > > >   {
> > > >   /* nothing to do here, just for informational purposes */
> > > >   }
> > > > + else if (info == XLOG_LOGICAL_DECODING_STATUS_CHANGE)
> > > > + {
> > > > + bool logical_decoding;
> > > > +
> > > > + /* Update the status on shared memory */
> > > > + memcpy(&logical_decoding, XLogRecGetData(record), sizeof(bool));
> > > > + UpdateLogicalDecodingStatus(logical_decoding, true);
> > > > +
> > > > + if (InRecovery && InHotStandby)
> > > > + {
> > > > + if (!logical_decoding)
> > > >
> > > > Like previously, shouldn't we have a check for standby's wal_level as
> > > > well due to the reasons mentioned in the removed comments?
> > >
> > > IIUC we need to replay the STATUS_CHANGE record when wal_level is set
> > > to 'replica' or 'logical'. If we want to add a check for standby's
> > > wal_level, the check would be "wal_level >= WAL_LEVEL_REPLICA" but it
> > > would be redundant as we already checked "InRecovery && InHotStandby".
> > >
> >
> > If we want to mimic the current implementation, won't
> > effective_wal_level be 'logical' even on standby? Otherwise, there
> > shouldn't be any logical slots which can be invalidated.
>
> Yes, effective_wal_level should be logical on the standby in this
> case. But when replaying STATUS_CHANGE with logical_decoding=false
> (i.e., !logical_decoding), it's obvious that the previous
> effective_wal_level was logical, no?
>

Isn't it possible that when we are replaying STATUS_CHANGE with
logical_decoding=false, the standby already has effective_wal_level
lesser than 'logical'? If so, then in that case, we don't need to
attempt invalidating the slots.

--
With Regards,
Amit Kapila.