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

On Tue, Mar 9, 2021 at 10:46 AM Peter Smith <smithpb2250(at)gmail(dot)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