Re: Found issues related with logical replication and 2PC

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: shveta malik <shveta.malik@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2024-08-08T12:23:22Z
Lists: pgsql-hackers

Attachments

On Thu, Aug 8, 2024 at 2:37 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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.

-- 
With Regards,
Amit Kapila.

Commits

  1. Change the misleading local end_lsn for prepared transactions.