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: Peter Smith <smithpb2250@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2021-03-06T12:41:07Z
Lists: pgsql-hackers 
On Sat, Mar 6, 2021 at 7:19 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Please find attached the latest patch set v50*
>

Few comments on the latest patch series:
=================================
1. I think we can extract the changes to make streaming optional with
2PC and infact you can start a separate thread for it.

2. I think we can get rid of table-sync early exit patch
(v50-0007-Tablesync-early-exit) as we have kept two_phase off from
tablesync worker. I agree that has its own independent value but it is
not required for this patch series.

3. Now, that we are not supporting streaming with two_pc option, do we
really need the first patch
(v50-0001-Refactor-spool-file-logic-in-worker.c)? I suggest to get rid
of the same unless it is really required. If we decide to remove this
patch, then remove the reference to apply_spooled_messages from 0008
patch.

v50-0005-Support-2PC-txn-subscriber-tests
4.
+###############################
+# Test cases involving DDL.
+###############################
+
+# TODO This can be added after we add functionality to replicate DDL
changes to subscriber.

We can remove this from the patch.

v50-0006-Support-2PC-txn-Subscription-option
5.
- /* Binary mode and streaming are only supported in v14 and higher */
+ /* Binary mode and streaming and Two phase commit are only supported
in v14 and higher */

It looks odd that only one of the option starts with capital letter
/Two/two. I suggest to two_phase.

v50-0008-Fix-apply-worker-empty-prepare
6. In 0008, the commit message lines are too long, it is difficult to
read those. Try to keep them 75 char long, this is generally what I
use but you can try something else if you want but not as long as you
have kept in this patch.

7.
+ /*
+ * A Problem:
+ *
..
Let's call this the "empty prepare" problem.
+ *
+ * The following code has a 2-part fix for that scenario.

No need to describe it in terms of problem and fix. You can say
something like: "This can lead to "empty prepare". We avoid this by
...."

-- 
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