Hi,

An update: I have two topics from the review perspective.

On the test I proposed to be added to 030_pg_recvlogical.pl, I found a way to write it without using sleeps (which risk flakyness in CI). I've attached it as a patch for your consideration. I checked the test in the following way: on master it fails, but with your patch it passes.

Secondly I noticed in function sendFeedback at line 166, theĀ startpos is set to output_written_lsn. This seems to counter conceptually the change you made in the patch, however it seems to not affect correctness. Shall we remove this line as to avoid confusion?
From 908983e7b9d5d8924a2ee3f2a45bd8302f494880 Mon Sep 17 00:00:00 2001
From: Mircea Cadariu <[email protected]>
Date: Mon, 24 Nov 2025 08:49:10 +0000
Subject: [PATCH v1] Add test for pg_recvlogical reconnection

---
 src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 65 ++++++++++++++
 ...test-for-pg_recvlogical-reconnection.patch | 86 +++++++++++++++++++
 2 files changed, 151 insertions(+)
 create mode 100644 v1-0001-Add-test-for-pg_recvlogical-reconnection.patch

diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl 
b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index 1b7a6f6f43..726941943a 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -151,4 +151,69 @@ my $result = $node->safe_psql('postgres',
 );
 is($result, 't', "failover is enabled for the new slot");
 
+use IPC::Run qw(start);
+
+my $outfile = $node->basedir . '/reconnect.out';
+  
+$node->command_ok(
+    [
+        'pg_recvlogical',
+        '--slot' => 'reconnect_test',
+        '--dbname' => $node->connstr('postgres'),
+        '--create-slot',
+    ],
+    'slot created for reconnection test');
+
+$node->safe_psql('postgres', 'CREATE TABLE t(x int);');
+$node->safe_psql('postgres', 'INSERT INTO t VALUES (1);');
+
+my $recv = start [
+    'pg_recvlogical',
+    '--slot', 'reconnect_test',
+    '--dbname', $node->connstr('postgres'),
+    '--start',
+    '--file', $outfile,
+    '--fsync-interval', '1',
+    '--status-interval', '1',
+    '--verbose'
+], '>', \my $out, '2>', \my $err;
+
+# Wait only for initial connection
+$node->poll_query_until('postgres',
+    "SELECT active_pid IS NOT NULL FROM pg_replication_slots WHERE slot_name = 
'reconnect_test'");
+
+# Terminate the backend
+my $backend_pid = $node->safe_psql('postgres',
+    "SELECT active_pid FROM pg_replication_slots WHERE slot_name = 
'reconnect_test'");
+$node->safe_psql('postgres', "SELECT pg_terminate_backend($backend_pid)");
+
+# Wait for reconnection
+$node->poll_query_until('postgres',
+    "SELECT active_pid IS NOT NULL AND active_pid != $backend_pid FROM 
pg_replication_slots WHERE slot_name = 'reconnect_test'");
+
+# Insert after reconnection
+$node->safe_psql('postgres', 'INSERT INTO t VALUES (2);');
+
+# Wait for file to contain both inserts
+$node->poll_query_until('postgres',
+    "SELECT (SELECT pg_read_file('$outfile') ~ 'INSERT.*INSERT') AS has_both");
+
+$recv->signal('TERM');
+$recv->finish();
+
+open(my $file, '<', $outfile);
+my $count = () = do { local $/; <$file> } =~ /INSERT/g;
+close($file);
+
+cmp_ok($count, '==', 2, 'two INSERTs');
+
+$node->command_ok(
+    [
+        'pg_recvlogical',
+        '--slot' => 'reconnect_test',
+        '--dbname' => $node->connstr('postgres'),
+        '--drop-slot'
+    ],
+    'reconnect_test slot dropped');
+
 done_testing();
diff --git a/v1-0001-Add-test-for-pg_recvlogical-reconnection.patch 
b/v1-0001-Add-test-for-pg_recvlogical-reconnection.patch
new file mode 100644
index 0000000000..f9f9b355b5
--- /dev/null
+++ b/v1-0001-Add-test-for-pg_recvlogical-reconnection.patch
@@ -0,0 +1,86 @@
+From 2e2a0a8c91cd4452b0b9457f04285a7c1c2e9b36 Mon Sep 17 00:00:00 2001
+From: Mircea Cadariu <[email protected]>
+Date: Mon, 24 Nov 2025 08:49:10 +0000
+Subject: [PATCH v1] Add test for pg_recvlogical reconnection
+
+---
+ src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 65 +++++++++++++++++++
+ 1 file changed, 65 insertions(+)
+
+diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl 
b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+index 1b7a6f6f43..6dd96ec095 100644
+--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
++++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+@@ -151,4 +151,69 @@ my $result = $node->safe_psql('postgres',
+ );
+ is($result, 't', "failover is enabled for the new slot");
+ 
++use IPC::Run qw(start);
++
++my $outfile = $node->basedir . '/reconnect.out';
++  
++$node->command_ok(
++    [
++        'pg_recvlogical',
++        '--slot' => 'reconnect_test',
++        '--dbname' => $node->connstr('postgres'),
++        '--create-slot',
++    ],
++    'slot created for reconnection test');
++
++$node->safe_psql('postgres', 'CREATE TABLE t(x int);');
++$node->safe_psql('postgres', 'INSERT INTO t VALUES (1);');
++
++my $recv = start [
++    'pg_recvlogical',
++    '--slot', 'reconnect_test',
++    '--dbname', $node->connstr('postgres'),
++    '--start',
++    '--file', $outfile,
++    '--fsync-interval', '1',
++    '--status-interval', '1',
++    '--verbose'
++], '>', \my $out, '2>', \my $err;
++
++# Wait only for initial connection
++$node->poll_query_until('postgres',
++    "SELECT active_pid IS NOT NULL FROM pg_replication_slots WHERE slot_name 
= 'reconnect_test'");
++
++# Terminate the backend
++my $backend_pid = $node->safe_psql('postgres',
++    "SELECT active_pid FROM pg_replication_slots WHERE slot_name = 
'reconnect_test'");
++$node->safe_psql('postgres', "SELECT pg_terminate_backend($backend_pid)");
++
++# Wait for reconnection
++$node->poll_query_until('postgres',
++    "SELECT active_pid IS NOT NULL AND active_pid != $backend_pid FROM 
pg_replication_slots WHERE slot_name = 'reconnect_test'");
++
++# Insert after reconnection
++$node->safe_psql('postgres', 'INSERT INTO t VALUES (2);');
++
++# Wait for file to contain both inserts
++$node->poll_query_until('postgres',
++    "SELECT (SELECT pg_read_file('$outfile') ~ 'INSERT.*INSERT') AS 
has_both");
++
++$recv->signal('TERM');
++$recv->finish();
++
++open(my $file, '<', $outfile);
++my $count = () = do { local $/; <$file> } =~ /INSERT/g;
++close($file);
++
++cmp_ok($count, '==', 2, 'at least two INSERTs');
++
++$node->command_ok(
++    [
++        'pg_recvlogical',
++        '--slot' => 'reconnect_test',
++        '--dbname' => $node->connstr('postgres'),
++        '--drop-slot'
++    ],
++    'reconnect_test slot dropped');
++
+ done_testing();
+-- 
+2.39.5 (Apple Git-154)
+
-- 
2.39.5 (Apple Git-154)

Reply via email to