Hi,
On Fri, Aug 01, 2025 at 03:04:20PM -0400, Melanie Plageman wrote:
> Hi,
>
> 035_standby_logical_decoding.pl has this code
>
> # wait for postgres to terminate
> foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
> {
> last if !-f $node_standby->data_dir . '/postmaster.pid';
> usleep(100_000);
> }
>
> but it does not import usleep as other tests do (like this)
> use Time::HiRes qw(usleep);
>
> I think it simply hasn't been called because postgres exits and
> terminates the loop before it has had a chance to be called. If you
> reorder the sleep and the loop termination condition, it fails
> immediately because it can't find usleep.
Thanks for the report! I think it's an oversight in commit 48796a98d5ae. Fixed
in the attached.
> On an unrelated note, I also noticed that hot_standby_feedback is not
> turned on during the test until we are explicitly checking for
> recovery conflicts.
Yeah, it's expected.
> When it is enabled, the comment says "Turn
> hot_standby_feedback back on" -- but it was never on to begin with. I
> suspect this doesn't produce test instability because autovacuum is
> turned off for the whole test. But, I was wondering if the test
> authors meant for hot_standby_feedback to be enabled during the
> initial phases of the test (until it is explicitly disabled for
> testing purposes).
It's typo/bad wording fixed in the attached. hot_standby_feedback does not need
to be enabled during the initial phases of the test because there is no catalog
changes that could produce a conflict.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From 82070e75b40bc29d5eb6596ea2f6ba70c211843e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Mon, 4 Aug 2025 06:26:30 +0000
Subject: [PATCH v1] Fix some 035_standby_logical_decoding.pl tests and
wordsmithing
usleep is used but not imported (oversight in commit 48796a98d5ae).
"replication" is missing in two checks leading to tests that could never fail.
Also using "back" in a comment gives the wrong impression that it has been already
set that way (which is not the case on purpose).
Reported-by: Melanie Plageman <melanieplage...@gmail.com>
---
src/test/recovery/t/035_standby_logical_decoding.pl | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
100.0% src/test/recovery/t/
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 921813483e3..20a15638c69 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -9,6 +9,7 @@ use warnings FATAL => 'all';
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
+use Time::HiRes qw(usleep);
if ($ENV{enable_injection_points} ne 'yes')
{
@@ -623,7 +624,7 @@ ok( $stderr =~
/ERROR: cannot copy invalidated replication slot "vacuum_full_inactiveslot"/,
"invalidated slot cannot be copied");
-# Turn hot_standby_feedback back on
+# Set hot_standby_feedback to on
change_hot_standby_feedback_and_wait_for_xmins(1, 1);
##################################################
@@ -754,12 +755,12 @@ wait_until_vacuum_can_remove(
# message should not be issued
ok( !$node_standby->log_contains(
- "invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
+ "invalidating obsolete replication slot \"no_conflict_inactiveslot\"", $logstart),
'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
);
ok( !$node_standby->log_contains(
- "invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
+ "invalidating obsolete replication slot \"no_conflict_activeslot\"", $logstart),
'activeslot slot invalidation is not logged with vacuum on conflict_test'
);
--
2.34.1
