Re: Found issues related with logical replication and 2PC

On Thu, Aug 8, 2024 at 5:53 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Aug 8, 2024 at 2:37 PM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Thanks for discussing!
> >
> > I reported the issue because 1) I feared the risk of data loss and 2) simply
> > because the coding looked incorrect. However, per discussion, I understood that
> > it wouldn't lead to loss, and adding a global variable was unacceptable in this
> > case. I modified the patch completely.
> >
> > The attached patch avoids using the LastCommitLSN as the local_lsn while applying
> > PREPARE. get_flush_position() was not changed. Also, it contains changes that
> > have not been discussed yet:
> >
> > - Set last_commit_end to InvaldXLogPtr in the PREPARE case.
> > This causes the same result as when the stream option is not "parallel."
> > - XactLastCommitEnd was replaced even ROLLBACK PREPARED case.
> > Since the COMMIT PREPARED record is flushed in RecordTransactionAbortPrepared(),
> > there is no need to ensure the WAL must be sent.
> >
>
> The code changes look mostly good to me. I have changed/added a few
> comments in the attached modified version.
>

Code changes with Amit's correction patch look good to me.

thanks
Shveta