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

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>

To: Peter Smith <smithpb2250@gmail.com>

Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Date: 2021-03-09T08:20:53Z

Lists: pgsql-hackers

On Tue, Mar 9, 2021 at 10:46 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Please find attached the latest patch set v54*
>
> Differences from v53* are:
>
> * Rebased to HEAD @ today
>
> * Addresses some recent feedback issues for patch 0001
>
> Feedback from Amit @ 7/March [ak]
> - (36) Fixed. Comment about the psf replay.
> - (37) Fixed. prepare_spoolfile_create, check file already exists (on
> disk) instead of just checking HTAB.
> - (38) Fixed. Added comment about potential overwrite of existing file.
>
> Feedback from Vignesh @ 8/March [vc]
> - (45) Fixed. Changed some comment to be single-line comments (e.g. if
> they only apply to a single following stmt)
> - (46) Fixed. prepare_spoolfile_create, refactored slightly to make
> more use of common code in if/else
> - (47) Skipped. This was feedback suggesting using ints instead of
> character values for message type enum.
>

Thanks for the updated patch.
Few comments:

+# Setup logical replication
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_publisher->safe_psql('postgres',
+       "CREATE PUBLICATION tap_pub");
+$node_publisher->safe_psql('postgres',
+       "ALTER PUBLICATION tap_pub ADD TABLE tab_full");

This can be changed to :
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub FOR TABLE tab_full");

We can make similar changes in:
+# node_A (pub) -> node_B (sub)
+my $node_A_connstr = $node_A->connstr . ' dbname=postgres';
+$node_A->safe_psql('postgres',
+       "CREATE PUBLICATION tap_pub_A");
+$node_A->safe_psql('postgres',
+       "ALTER PUBLICATION tap_pub_A ADD TABLE tab_full");
+my $appname_B = 'tap_sub_B';
+$node_B->safe_psql('postgres', "
+       CREATE SUBSCRIPTION tap_sub_B
+       CONNECTION '$node_A_connstr application_name=$appname_B'
+       PUBLICATION tap_pub_A");
+
+# node_B (pub) -> node_C (sub)
+my $node_B_connstr = $node_B->connstr . ' dbname=postgres';
+$node_B->safe_psql('postgres',
+       "CREATE PUBLICATION tap_pub_B");
+$node_B->safe_psql('postgres',
+       "ALTER PUBLICATION tap_pub_B ADD TABLE tab_full");

+# rollback post the restart
+$node_publisher->safe_psql('postgres',
+       "ROLLBACK PREPARED 'test_prepared_tab';");
+$node_publisher->poll_query_until('postgres', $caughtup_query)
+       or die "Timed out while waiting for subscriber to catch up";
+
+# check inserts are rolled back
+$result = $node_subscriber->safe_psql('postgres',
+       "SELECT count(*) FROM tab_full where a IN (12,13);");
+is($result, qq(0), 'Rows inserted via 2PC are visible on the subscriber');

"Rows inserted via 2PC are visible on the subscriber"
should be something like:
"Rows rolled back are not on the subscriber"

git diff --check
src/backend/replication/logical/worker.c:3704: trailing whitespace.

Regards,
Vignesh

Commits

Add prepare API support for streaming transactions in logical replication.
- 63cf61cdeb7b 15.0 landed
Doc: minor improvements for logical replication protocol documentation.
- a5cb4f9829fb 15.0 cited
Fix test failure in 021_twophase.pl.
- eaf5321c3524 15.0 landed
Refactor to make common functions in proto.c and worker.c.
- 91f9861242cd 15.0 landed
Unify PostgresNode's new() and get_new_node() methods
- 201a76183e20 15.0 cited
Fix potential buffer overruns in proto.c.
- 01c3adcdd85f 15.0 landed
Add support for prepared transactions to built-in logical replication.
- a8fd13cab0ba 15.0 cited
Refactor function parse_subscription_options.
- 8aafb0261675 15.0 cited
Allow enabling two-phase option via replication protocol.
- cda03cfed6b8 15.0 cited
Don't use Asserts to check for violations of replication protocol.
- fe6a20ce54cb 14.0 cited
Improve psql tab completion for options of subcriptions and publications
- d08237b5b494 14.0 cited
Rearrange logrep worker's snapshot handling some more.
- 3a09d75b4f6c 14.0 cited
doc: Update information of new messages for logical replication.
- 15c1a9d9cb76 14.0 cited
ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION
- 82ed7748b710 14.0 cited
Allow pgoutput to send logical decoding messages.
- ac4645c0157f 14.0 cited
Refactor function parse_output_parameters.
- 531737ddad21 14.0 cited
Avoid repeated decoding of prepared transactions after a restart.
- 8bdb1332eb51 14.0 cited
Fix an oversight in ReorderBufferFinishPrepared.
- ade89ba5f408 14.0 landed
Allow multiple xacts during table sync in logical replication.
- ce0fdbfe9722 14.0 cited
Fix replication of in-progress transactions in tablesync worker.
- 0926e96c4934 14.0 cited
Fix 'skip-empty-xacts' option in test_decoding for streaming mode.
- 9653f24ad830 14.0 cited
Use Enum for top level logical replication message types.
- 644f0d7cc9c2 14.0 cited
Add support for streaming to built-in logical replication.
- 464824323e57 14.0 cited
Fix the logical streaming test.
- 82a0ba7707e0 14.0 cited
Implement streaming mode in ReorderBuffer.
- 7259736a6e5b 14.0 cited
Extend the logical decoding output plugin API with stream methods.
- 45fdc9738b36 14.0 cited
Store 2PC GID in commit/abort WAL recs for logical decoding
- 1eb6d6527aae 11.0 cited