On Thu, May 13, 2021 at 4:41 PM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Thu, May 13, 2021 at 04:14:55PM +0530, vignesh C wrote:
> > +$node_publisher->wait_for_catchup('tap_sub');
> > +
> > # Ensure a transactional logical decoding message shows up on the slot
> > $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub
> > DISABLE");
> >
> > # wait for the replication connection to drop from the publisher
> > $node_publisher->poll_query_until('postgres',
> > - 'SELECT COUNT(*) FROM pg_catalog.pg_stat_replication', 0);
> > + "SELECT COUNT(*) FROM pg_catalog.pg_replication_slots WHERE slot_name
> > = 'tap_sub' AND active='f'", 1);
>
> There are three places in this test where a slot is disabled, followed
> by a wait to make sure that the slot is gone. Perhaps it would be
> better to wrap that in a small-ish routine?
Thanks for the comments, Please find the attached patch having the changes.
Regards,
Vignesh
From 495b48981f18a5e05453cefeb80838fb60982e0f Mon Sep 17 00:00:00 2001
From: vignesh <vignes...@gmail.com>
Date: Thu, 13 May 2021 16:00:43 +0530
Subject: [PATCH v2] Messages test failure fix.
This test was failing because there was no wait for catch up after
creating subscription. After disabling subscription pg_stat_replication
was used to verify if the walsender is exited. The steps of walsender
exit includes cleaning up of walsendr and then releasing replication
slot. There will be random test failure if pg_logical_slot_peek_binary_changes
is called in the narrow time window. Fixed it by checking
active column in pg_replication_slot instead of pg_stat_replication
which is more reliable.
---
src/test/subscription/t/020_messages.pl | 60 +++++++++++++------------
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/src/test/subscription/t/020_messages.pl b/src/test/subscription/t/020_messages.pl
index 0940d0f45f..f1b2642640 100644
--- a/src/test/subscription/t/020_messages.pl
+++ b/src/test/subscription/t/020_messages.pl
@@ -8,6 +8,35 @@ use PostgresNode;
use TestLib;
use Test::More tests => 5;
+# Setup the subscription before checking pg_logical_slot_peek_binary_changes
+sub setup_subscription
+{
+ my $node_publisher = shift;
+ my $node_subscriber = shift;
+ my $create_subscription = shift;
+
+ if ($create_subscription == 1)
+ {
+ my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+ $node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub");
+ }
+ else
+ {
+ $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub ENABLE");
+ }
+
+ $node_publisher->wait_for_catchup('tap_sub');
+
+ # Ensure a transactional logical decoding message shows up on the slot
+ $node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub DISABLE");
+
+ # wait for the replication connection to drop from the publisher
+ $node_publisher->poll_query_until('postgres',
+ "SELECT COUNT(*) FROM pg_catalog.pg_replication_slots WHERE slot_name = 'tap_sub' AND active='f'", 1);
+ return;
+}
+
# Create publisher node
my $node_publisher = get_new_node('publisher');
$node_publisher->init(allows_streaming => 'logical');
@@ -27,20 +56,10 @@ $node_subscriber->safe_psql('postgres',
"CREATE TABLE tab_test (a int primary key)");
# Setup logical replication
-my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub FOR TABLE tab_test");
-$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
-);
-
-# Ensure a transactional logical decoding message shows up on the slot
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub DISABLE");
-
-# wait for the replication connection to drop from the publisher
-$node_publisher->poll_query_until('postgres',
- 'SELECT COUNT(*) FROM pg_catalog.pg_stat_replication', 0);
+setup_subscription($node_publisher, $node_subscriber, 1);
$node_publisher->safe_psql('postgres',
"SELECT pg_logical_emit_message(true, 'pgoutput', 'a transactional message')"
@@ -88,15 +107,7 @@ is( $result, qq(66
'option messages defaults to false so message (M) is not available on slot'
);
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub ENABLE");
-$node_publisher->wait_for_catchup('tap_sub');
-
-# ensure a non-transactional logical decoding message shows up on the slot
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub DISABLE");
-
-# wait for the replication connection to drop from the publisher
-$node_publisher->poll_query_until('postgres',
- 'SELECT COUNT(*) FROM pg_catalog.pg_stat_replication', 0);
+setup_subscription($node_publisher, $node_subscriber, 0);
$node_publisher->safe_psql('postgres', "INSERT INTO tab_test VALUES (1)");
@@ -118,14 +129,7 @@ $result = $node_publisher->safe_psql(
is($result, qq(77|0), 'non-transactional message on slot is M');
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub ENABLE");
-$node_publisher->wait_for_catchup('tap_sub');
-
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tap_sub DISABLE");
-
-# wait for the replication connection to drop from the publisher
-$node_publisher->poll_query_until('postgres',
- 'SELECT COUNT(*) FROM pg_catalog.pg_stat_replication', 0);
+setup_subscription($node_publisher, $node_subscriber, 0);
# Ensure a non-transactional logical decoding message shows up on the slot when
# it is emitted within an aborted transaction. The message won't emit until
--
2.25.1
