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

On Wed, Oct 1, 2025 at 4:31 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Sawada-san,
>
> > If we implement these ideas, we can simplify the patch quite well as
> > we no longer need the lazy behavior nor wait for the recovery to
> > complete. I've attached a PoC patch that can be applied on top of the
> > v15 patch.
>
> In 0002, I found an assertion failure. Steps:
>
> 0. There is a streaming replication system and only primary has a logical slot.
> 1. Attached to a startup process and set a break at UpdateLogicalDecodingStatusEndOfRecovery.
> 2. Sent a promote signal to the standby and ensured the startup stopped.
> 3. Established new connection to the standby
> 4. Attached to the backend process and set a break at create_logical_replication_slot.
> 5. Tried to create a new slot on the standby and ensured the backend stopped
> 6. Moved the startup process till WaitForProcSignalBarrier().
> 7. Moved the backend process till WaitForProcSignalBarrier(). Both processes could go ahead.
> 8. Moved the backend till ReplicationSlotReserveWal() and restart_lsn was set.
> 9. Detached from the startup process. Recovery state became "DONE".
> 10. Detached from the backend. It would crash at xlog_decode().
>
> Some data was obtained by the gdb, see [1].
>
> Direct cause is that restart_lsn of the slot points the value before STATUS_CHANGE(false).
> Per my analysis, ReplicationSlotReserveWal() uses GetXLogReplayRecPtr(NULL) as the
> initial decode point, which is the last record the standby receives from the primary.
> However, the standby can generate additional record, STATUS_CHANGE (false) in
> this case. After the recovery, the decoder would read the STATUS_CHANGE record,
> but it breaks our assumption.
>
> Per my understanding, this cannot happen with 0001 because EnsureLogicalDecodingEnabled()
> waits till RecoveryInProgress() becomes false.
>
> How should we fix the issue? One approach is to remove the Assert() and ereport(ERROR),
> but even in the case the slot may not be able to establish the consistent snapshot.
>

The other point to consider is that during promotion after
UpdateLogicalDecodingStatusEndOfRecovery(), we have multiple things
that seems to be necessary to perform before backends are allowed to
write. For example, refer to comments: "If any of the critical GUCs
have changed, log them before we allow backends to write WAL.*/. I
think the key thing is that before we set state DB_IN_PRODUCTION in
ControlFile and mark SharedRecoverstate as RECOVERY_STATE_DONE,
backends shouldn't be allowed to write WAL. If we want to take an
exception for writing a WAL during slot_creation before the
RECOVERY_STATE_DONE is set, we should analyze and explain in comments
why it is okay to take this exception.

--
With Regards,
Amit Kapila.