On Mon, Dec 1, 2025 at 11:48 AM Masahiko Sawada <[email protected]> wrote:
>
> On Thu, Nov 27, 2025 at 11:00 PM shveta malik <[email protected]> wrote:
> >
> > On Thu, Nov 27, 2025 at 4:33 PM shveta malik <[email protected]> wrote:
> > >
> > > A few more comments:
> > >
> > >
> > > 10)
> > > +# Promote standby3, increasing effective_wal_level to 'logical' as
> > > its wal_level
> > > +# is set to 'logical'.
> > > +$standby3->promote;
> > > +
> > > +# Check if effective_wal_level is increased to 'logical' on the
> > > cascaded standby.
> > > +$standby3->wait_for_replay_catchup($cascade);
> > > +test_wal_level($cascade, "replica|logical",
> > > + "effective_wal_level got increased to 'logical' on standby as the
> > > new primary has wal_level='logical'"
> > > +);
> > >
> > > The message is slightly confusing due to usage of both 'standby' and
> > > 'new primary'. Can we make it:
> > > effective_wal_level got increased to 'logical' as the new primary has
> > > wal_level='logical'
> > >
> >
> > Upon reconsideration, we can keep it as is. I understand the intent now.
>
> Okay.
>
> >
> > ===
> >
> > I tried to test possible race-scenarios (known to me), they seem to
> > work well, except one minor thing:
> >
> > Let's say there is slot1 present, backend1 is trying to drop slot1 and
> > backend2 is trying to create slot2.
> >
> > DisableLogicalDecodingIfNecessary() first kicks in and reaches the
> > stage where it has disabled and released the lock. Before it could
> > EmitSignal and log, EnsureLogicalDecodingEnabled() kicks in and
> > completes its execution.
> > In such a case we end up with reverse LOG messages in log file:
> >
> > 09:47:16.489 IST LOG: logical decoding is enabled upon creating a new
> > logical replication slot
> > 09:47:17.484 IST LOG: logical decoding is disabled because there are
> > no valid logical replication slots
> >
> > while logical decoding is actually enabled in the system.
> >
> > Shall we check 'if (!LogicalDecodingCtl->xlog_logical_info)' before
> > logging in DisableLogicalDecodingIfNecessary()?
> >
>
> In DisableLogicalDecodingIfNecessary(), we have (without comments):
>
> if (!LogicalDecodingCtl->xlog_logical_info || CheckLogicalSlotExists())
> {
> LogicalDecodingCtl->pending_disable = false;
> LWLockRelease(LogicalDecodingControlLock);
> return;
> }
>
> START_CRIT_SECTION();
>
> LogicalDecodingCtl->logical_decoding_enabled = false;
> write_logical_decoding_status_update_record(false);
> LogicalDecodingCtl->xlog_logical_info = false;
> LogicalDecodingCtl->pending_disable = false;
>
> LWLockRelease(LogicalDecodingControlLock);
>
> END_CRIT_SECTION();
>
> EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO);
>
> ereport(LOG,
> errmsg("logical decoding is disabled because there are no
> valid logical replication slots"));
>
> Does it make sense to reorder them to the following?
>
yes, it will avoid the issue.
> START_CRIT_SECTION();
>
> LogicalDecodingCtl->logical_decoding_enabled = false;
> write_logical_decoding_status_update_record(false);
> LogicalDecodingCtl->xlog_logical_info = false;
> LogicalDecodingCtl->pending_disable = false;
>
> END_CRIT_SECTION();
>
> ereport(LOG,
> errmsg("logical decoding is disabled because there are no
> valid logical replication slots"));
>
> LWLockRelease(LogicalDecodingControlLock);
>
> Regards,
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
