Horton

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

vignesh C <vignesh21@gmail.com>

View thread View on postgr.es
From: vignesh C <vignesh21@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2021-03-29T07:38:25Z
Lists: pgsql-hackers

Attachments

On Sun, Mar 21, 2021 at 1:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Mar 20, 2021 at 10:09 AM Ajin Cherian <itsajin@gmail.com> wrote:
> >
> > On Sat, Mar 20, 2021 at 1:35 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>
> >> On Fri, Mar 19, 2021 at 5:03 AM Ajin Cherian <itsajin@gmail.com> wrote:
> >> >
> >> > Missed the patch - 0001, resending.
> >> >
> >>
> >> I have made miscellaneous changes in the patch which includes
> >> improving comments, error messages, and miscellaneous coding
> >> improvements. The most notable one is that we don't need an additional
> >> parameter in walrcv_startstreaming, if the two_phase option is set
> >> properly. My changes are in v63-0002-Misc-changes-by-Amit, if you are
> >> fine with those, then please merge them in the next version. I have
> >> omitted the dev-logs patch but feel free to submit it. I have one
> >> question:
> >>
> >
> > I am fine with these changes. I see that Peter has already merged in these changes.
> >
>
> I have further updated the patch to implement unique GID on the
> subscriber-side as discussed in the nearby thread [1]. That requires
> some changes in the test. Additionally, I have updated some comments
> and docs. Let me know what do you think about the changes?
>

+static void
+TwoPhaseTransactionGid(RepOriginId originid, TransactionId xid,
+                                          char *gid, int szgid)
+{
+       /* Origin and Transaction ids must be valid */
+       Assert(originid != InvalidRepOriginId);
+       Assert(TransactionIdIsValid(xid));
+
+       snprintf(gid, szgid, "pg_%u_%u", originid, xid);
+}

I found one issue in the current mechanism that we use to generate the
GID's. In one of the scenarios it will generate the same GID's, steps
for the same is given below:
---- setup 2 publisher and one subscriber with synchronous_standby_names
 prepare txn 't1' on publisher1 (This prepared txn is prepared as
pg_1_542 on subscriber)
drop subscription of publisher1
create subscription subscriber for publisher2 (We have changed the
subscription to subscribe to publisher2 which was earlier subscribing
to publisher1)
prepare txn 't2' on publisher2 (This prepared txn also uses pg_1_542
on subscriber even though user has given a different gid)

This prepared txn keeps waiting for it to complete in the subscriber,
but never completes. Here user uses different gid for prepared
transaction but it ends up using the same gid at the subscriber. The
subscriber keeps failing with:
2021-03-22 10:14:57.859 IST [73959] ERROR:  transaction identifier
"pg_1_542" is already in use
2021-03-22 10:14:57.860 IST [73868] LOG:  background worker "logical
replication worker" (PID 73959) exited with exit code 1

Attached file has the steps for it.
This might be a rare scenario, may or may not be a user scenario,
Should we handle this scenario?

Regards,
Vignesh

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