Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-17T12:49:09Z
Lists: pgsql-hackers

Attachments

On Wed, Dec 16, 2020 at 2:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Dec 16, 2020 at 1:04 PM Masahiko Sawada <sawada.mshk@gmail.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.

Commits

  1. Add prepare API support for streaming transactions in logical replication.

  2. Doc: minor improvements for logical replication protocol documentation.

  3. Fix test failure in 021_twophase.pl.

  4. Refactor to make common functions in proto.c and worker.c.

  5. Unify PostgresNode's new() and get_new_node() methods

  6. Fix potential buffer overruns in proto.c.

  7. Add support for prepared transactions to built-in logical replication.

  8. Refactor function parse_subscription_options.

  9. Allow enabling two-phase option via replication protocol.

  10. Don't use Asserts to check for violations of replication protocol.

  11. Improve psql tab completion for options of subcriptions and publications

  12. Rearrange logrep worker's snapshot handling some more.

  13. doc: Update information of new messages for logical replication.

  14. ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION

  15. Allow pgoutput to send logical decoding messages.

  16. Refactor function parse_output_parameters.

  17. Avoid repeated decoding of prepared transactions after a restart.

  18. Fix an oversight in ReorderBufferFinishPrepared.

  19. Allow multiple xacts during table sync in logical replication.

  20. Fix replication of in-progress transactions in tablesync worker.

  21. Fix 'skip-empty-xacts' option in test_decoding for streaming mode.

  22. Use Enum for top level logical replication message types.

  23. Add support for streaming to built-in logical replication.

  24. Fix the logical streaming test.

  25. Implement streaming mode in ReorderBuffer.

  26. Extend the logical decoding output plugin API with stream methods.

  27. Store 2PC GID in commit/abort WAL recs for logical decoding