From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
Date: | 2025-10-01 14:43:59 |
Message-ID: | CAD21AoAPwHNpf8pOKai0SP4mRmTOG2Housy1HROvz3Hpn6LVEg@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 1, 2025 at 7:01 AM 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].
Thank you for testing the patch! Good catch.
> 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.
I think that the problem stems from the fact that the patch sets
allow_status_change to true before completing all end-of-recovery
actions for logical decoding status update. I think it should be done
at the end of UpdateLogicalDecodingStatusEndOfRecovery(), i.e., after
WaitForProcSignalBarrier(). That way, the logical decoding can always
start from after the point where the startup updated the logical
decoding status.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2025-10-01 14:48:22 | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
Previous Message | Bertrand Drouvot | 2025-10-01 14:33:11 | Re: relfilenode statistics |