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

On Wed, Dec 16, 2020 at 2:54 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Dec 16, 2020 at 1:04 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
>
> > Also, I guess we can improve the description of
> > ’two_phase’ option of CREATE SUBSCRIPTION in the doc by adding the
> > fact that when this option is not enabled the transaction prepared on
> > the publisher is decoded as a normal transaction:
> >
>
> Sounds reasonable.
>

Fixed in the attached.

> > ------
> > + if (LookupGXact(begin_data.gid))
> > + {
> > + /*
> > + * If this gid has already been prepared then we dont want to apply
> > + * this txn again. This can happen after restart where upstream can
> > + * send the prepared transaction again. See
> > + * ReorderBufferFinishPrepared. Don't update remote_final_lsn.
> > + */
> > + skip_prepared_txn = true;
> > + return;
> > + }
> >
> > When PREPARE arrives at the subscriber node but there is the prepared
> > transaction with the same transaction identifier, the apply worker
> > skips the whole transaction. So if the users prepared a transaction
> > with the same identifier on the subscriber, the prepared transaction
> > that came from the publisher would be ignored without any messages. On
> > the other hand, if applying other operations such as HEAP_INSERT
> > conflicts (such as when violating the unique constraint) the apply
> > worker raises an ERROR and stops logical replication until the
> > conflict is resolved. IIUC since we can know that the prepared
> > transaction came from the same publisher again by checking origin_lsn
> > in TwoPhaseFileHeader I guess we can skip the PREPARE message only
> > when the existing prepared transaction has the same LSN and the same
> > identifier. To be exact, it’s still possible that the subscriber gets
> > two PREPARE messages having the same LSN and name from two different
> > publishers but it’s unlikely happen in practice.
> >
>
> The idea sounds reasonable. I'll try and see if this works.
>

I went ahead and used both origin_lsn and origin_timestamp to avoid
the possibility of a match of prepared xact from two different nodes.
We can handle this at begin_prepare and prepare time but we don't have
prepare_lsn and prepare_timestamp at rollback_prepared time, so what
do about that? As of now, I am using just GID at rollback_prepare time
and that would have been sufficient if we always receive prepare
before rollback because at prepare time we would have checked
origin_lsn and origin_timestamp. But it is possible that we get
rollback prepared without prepare in case if prepare happened before
consistent_snapshot is reached and rollback happens after that. For
commit-case, we do send prepare and all the data at commit time in
such a case but doing so for rollback case doesn't sound to be a good
idea. Another possibility is that we send prepare_lsn and prepare_time
in rollback_prepared API to deal with this. I am not sure if it is a
good idea to just rely on GID in rollback_prepare. What do you think?

I have done some additional changes in the patch-series.
1. Removed some declarations from
0001-Extend-the-output-plugin-API-to-allow-decoding-o which were not
required.
2. In 0002-Allow-decoding-at-prepare-time-in-ReorderBuffer,
+ txn->txn_flags |= RBTXN_PREPARE;
+ txn->gid = palloc(strlen(gid) + 1); /* trailing '\0' */
+ strcpy(txn->gid, gid);

Changed the above code to use pstrdup.

3. Merged the test-code from 0003 to 0002. I have yet to merge the
latest test case posted by Ajin[1].
4. Removed the test for Rollback Prepared from two_phase_streaming.sql
because I think a similar test exists for non-streaming case in
two_phase.sql and it doesn't make sense to repeat it.
5. Comments update and minor cosmetic changes for test cases merged
from 0003 to 0002.

[1] - https://www.postgresql.org/message-id/CAFPTHDYWj99%2BysDjCH_z8BfN8hG2FoxtJg%2BEU8_MpJe5owXg4A%40mail.gmail.com

--
With Regards,
Amit Kapila.