RE: Fix slot synchronization with two_phase decoding enabled

On Sat, Apr 19, 2025 at 2:19 AM Masahiko Sawada wrote:
>
> On Tue, Apr 8, 2025 at 10:14 PM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> >
> > ----------
> > Approach 2
> > ----------
> >
> > Instead of disallowing the use of two-phase and failover together, a more
> > flexible strategy could be only restrict failover for slots with two-phase
> > enabled when there's a possibility of existing prepared transactions before
> the
> > two_phase_at that are not yet replicated. During slot creation with
> two-phase
> > and failover, we could check for any decoded prepared transactions when
> > determining the decoding start point (DecodingContextFindStartpoint). For
> > subsequent attempts to alter failover to true, we ensure that two_phase_at is
> > less than restart_lsn, indicating that all prepared transactions have been
> > committed and replicated, thus the bug would not happen.
> >
> > pros:
> >
> > This method minimizes restrictions for users. Especially during slot creation
> > with (two_phase=on, failover=on), as it’s uncommon for transactions to
> prepare
> > during consistent snapshot creation, the restriction becomes almost
> > unnoticeable.
>
> I think this approach can work for the transactions that are prepared
> while the slot is created. But if I understand the problem correctly,
> while the initial table sync is performing, the slot's two_phase is
> still false, so we need to deal with the transactions that are
> prepared during the initial table sync too. What do you think?
>

Yes, I agree that we need to restrict this case too. Given that we haven't
started decoding when setting two_phase=true during CreateDecodingContext()
after tablesync, we could check prepared transactions afterwards during
decoding. This could involve reporting an ERROR when skipping a prepared
transaction during decoding if its prepare LSN is less than two_phase_at.
Alternatively, a simpler method would be to prevent this situation entirely
during the CREATE SUBSCRIPTION command. For example, we could restrict slots
created with failover set to true and twophase is later modified to true after
tablesync. Although the simpler check is more user-visible, it may offer less
flexibility.

What do you think ?

Best Regards,
Hou zj