Re: recovery test failures on hoverfly - Mailing list pgsql-hackers

I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> This is the same problem as c757a3da and 6d41dd0, where we write a
>> query to a pipe but the kill, causing a failure, makes the test fail
>> with a SIGPIPE in IPC::Run as a query is sent down to a pipe.

> The precedent of the previous fixes would seem to suggest seeing if
> we can replace 'SELECT 1' with "undef".  Not sure if that'll work
> without annoying changes to poll_query_until, though.

I noticed that elver failed this same way today, so that got me
annoyed enough to pursue a fix.  Using "undef" as poll_query_until's
input almost works, except it turns out that it fails to notice psql
connection failures in that case!  It is *only* looking at psql's
stdout, not at either stderr or the exit status, which seems seriously
bogus in its own right; not least because poll_query_until's own
documentation claims it will continue waiting after an error, which
is exactly what it's not doing.  So I propose the attached.

(I first tried to make it check $result == 0, but it seems there are a
lot of cases where psql returns status 1 in these tests.  That seems
pretty bogus too, but probably beta is no time to change that
behavior.)

            regards, tom lane

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 45d1636128..2027cbf43d 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2140,8 +2140,10 @@ sub poll_query_until

         $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
         chomp($stdout);
+        $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+        chomp($stderr);

-        if ($stdout eq $expected)
+        if ($stdout eq $expected && $stderr eq '')
         {
             return 1;
         }
@@ -2154,8 +2156,6 @@ sub poll_query_until

     # The query result didn't change in 180 seconds. Give up. Print the
     # output from the last attempt, hopefully that's useful for debugging.
-    $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
-    chomp($stderr);
     diag qq(poll_query_until timed out executing this query:
 $query
 expecting this output:
diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl
index e1c36abe97..868a50b33d 100644
--- a/src/test/recovery/t/013_crash_restart.pl
+++ b/src/test/recovery/t/013_crash_restart.pl
@@ -136,12 +136,8 @@ ok( pump_until(
 $monitor->finish;

 # Wait till server restarts
-is( $node->poll_query_until(
-        'postgres',
-        'SELECT $$restarted after sigquit$$;',
-        'restarted after sigquit'),
-    "1",
-    "reconnected after SIGQUIT");
+is($node->poll_query_until('postgres', undef, ''),
+    "1", "reconnected after SIGQUIT");


 # restart psql processes, now that the crash cycle finished
@@ -216,7 +212,7 @@ ok( pump_until(
 $monitor->finish;

 # Wait till server restarts
-is($node->poll_query_until('postgres', 'SELECT 1', '1'),
+is($node->poll_query_until('postgres', undef, ''),
     "1", "reconnected after SIGKILL");

 # Make sure the committed rows survived, in-progress ones not
diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl
index b912f4b232..157ddba8cf 100644
--- a/src/test/recovery/t/022_crash_temp_files.pl
+++ b/src/test/recovery/t/022_crash_temp_files.pl
@@ -139,7 +139,7 @@ $killme->finish;
 $killme2->finish;

 # Wait till server restarts
-$node->poll_query_until('postgres', 'SELECT 1', '1');
+$node->poll_query_until('postgres', undef, '');

 # Check for temporary files
 is( $node->safe_psql(
@@ -228,7 +228,7 @@ $killme->finish;
 $killme2->finish;

 # Wait till server restarts
-$node->poll_query_until('postgres', 'SELECT 1', '1');
+$node->poll_query_until('postgres', undef, '');

 # Check for temporary files -- should be there
 is( $node->safe_psql(