Re: [HACKERS] logical decoding of two-phase transactions

On Tue, Sep 15, 2020 at 10:43 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

>
> Few other comments:
> ===================
> 1.
> ReorderBufferProcessTXN()
> {
> ..
> if (streaming)
> {
> ReorderBufferTruncateTXN(rb, txn);
>
> /* Reset the CheckXidAlive */
> CheckXidAlive = InvalidTransactionId;
> }
> else
> ReorderBufferCleanupTXN(rb, txn);
> ..
> }
>
> I don't think we can perform ReorderBufferCleanupTXN for the prepared
> transactions because if we have removed the ReorderBufferTxn before
> commit, the later code might not consider such a transaction in the
> system and compute the wrong value of restart_lsn for a slot.
> Basically, in SnapBuildProcessRunningXacts() when we call
> ReorderBufferGetOldestTXN(), it should show the ReorderBufferTxn of
> the prepared transaction which is not yet committed but because we
> have removed it after prepare, it won't get that TXN and then that
> leads to wrong computation of restart_lsn. Once we start from a wrong
> point in WAL, the snapshot built was incorrect which will lead to the
> wrong result. This is the same reason why the patch is not doing
> ReorderBufferForget in DecodePrepare when we decide to skip the
> transaction. Also, here, we need to set CheckXidAlive =
> InvalidTransactionId; for prepared xact as well.
>
>
>
Just to confirm what you are expecting here. so after we send out the
prepare transaction to the plugin, you are suggesting to NOT do a
ReorderBufferCleanupTXN, but what to do instead?. Are you suggesting to do
what you suggested
as part of concurrent abort handling? Something equivalent
to ReorderBufferTruncateTXN()? remove all changes of the transaction but
keep the invalidations and tuplecids etc? Do you think we should have a new
flag in txn to indicate that this transaction has already been decoded?
(prepare_decoded?) Any other special handling you think is required?

regards,
Ajin Cherian
Fujitsu Australia