Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

On Thu, Nov 27, 2025 at 5:03 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
>
> A few more comments:
>
> 8)
> InvalidateObsoleteReplicationSlots():
>
> + if (InvalidatePossiblyObsoleteSlot(possible_causes, s, oldestLSN,
> + dboid, snapshotConflictHorizon,
> + &released_lock))
> {
> ......
> }
> +
> + SpinLockAcquire(&s->mutex);
> + found_valid_logicalslot |= (s->data.invalidated == RS_INVAL_NONE);
> + SpinLockRelease(&s->mutex);
>
> Shouldn't we move SpinLockAcquire and setting
> 'found_valid_logicalslot' to else-block? Only when
> InvalidatePossiblyObsoleteSlot() has not invalidated slot, we would
> like to check for RS_INVAL_NONE and set 'found_valid_logicalslot'.
>
> 9)
> test:
> + "effective_wal_level increased to logical upon a logical slot creation");
> + "effective_wal_level remains logical even after a server restart");
> + "effective_wal_level stays 'logical' as one slot remains");
> + "effective_wal_level got decreased to 'replica' after invalidating
> the last logical slot"
>
> In the messages above, at some places we single quote the
> effective_wal_level value while at other places we do not. We can make
> all same.
>
> 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'

Fixed the above points.

> 11)
> my ($result, $stdout, $stderr) = $standby4->psql('postgres',
> qq[select pg_logical_slot_get_changes('standby4_slot', null, null)]);
> like(
> $stderr,
> qr/ERROR: logical decoding on standby requires "effective_wal_level"
> >= "logical" on the primary/,
> "cannot use logical decoding on standby as it is disabled on primary");
>
> Since the slot is invalidated, shouldn't the better error message be
> what we usually get:
> ERROR: can no longer access replication slot "slot"
> DETAIL: This replication slot has been invalidated due to "..".
>
> IMO, the 'invalidated' message is better because even if we enable
> logical-decoding on primary, it is of no use for this slot.
>
> But I also see the difficulty in achieving this, as the
> primary-related error message is by CheckLogicalDecodingRequirements()
> which happens earlier than
> ReplicationSlotAcquire(). So if there is no better way, we can leave it as is.

We can achieve it by creating another valid logical slot, but I
believe that we already have tests to check if we cannot use an
invalidated slot. What we want to test here is if logical decoding is
disabled by invalidating the last logical slot. So I think the test
works as expected.

Regards,

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