Re: Found issues related with logical replication and 2PC

On Wed, Aug 7, 2024 at 5:43 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Aug 7, 2024 at 3:32 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > I also think so. Additionally, I feel a test case (or some description
> > of the bug that can arise) should be provided for issue-1.
> >
>
> IIUC, the problem could be that we would end up updating the wrong
> local_end LSN in lsn_mappings via store_flush_position(). Then
> get_flush_position() could end up computing the wrong flush position
> and send the confirmation of flush to the publisher even when it is
> not flushed. This ideally could lead to a situation where the prepared
> transaction is not flushed to disk on the subscriber and because
> publisher would have gotten the confirmation earlier than required, it
> won't send the prepared transaction again.

Yes, that is what my understanding was.

> I think this theory is not
> true for prepare transactions because we always flush WAL of prepare
> even for asynchronous commit mode. See EndPrepare().

Okay, I was not aware of this. Thanks for explaining.

> So, if my
> analysis is correct, this shouldn't be a bug and ideally, we should
> update local_end LSN as InvalidXLogRecPtr and add appropriate
> comments.

Okay, we can do that. Then get_flush_position() can also be changed to
*explicitly* deal with the case where local_end is InvalidXLogRecPtr.
Having said that, even though it is not a bug, shouldn't we still have
the correct mapping updated in lsn_mapping? When remote_end is PREPARE
Or COMMIT_PREPARED, local_end should also point to the same?

thanks
Shveta