Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0b619df

Browse files
committed
Restore robustness of TAP tests that wait for postmaster restart.
Several TAP tests use poll_query_until() to wait for the postmaster to restart. They were checking to see if a trivial query (e.g. "SELECT 1") succeeds. However, that's problematic in the wake of commit 11e9caf, because now that we feed said query to psql via stdin, we risk IPC::Run whining about a SIGPIPE failure if psql quits before reading the query. Hence, we can't use a nonempty query in cases where we need to wait for connection failures to stop happening. Per the precedent of commits c757a3d and 6d41dd0, we can pass "undef" as the query in such cases to ensure that IPC::Run has nothing to write. However, then we have to say that the expected output is empty, and this exposes a deficiency in poll_query_until: if psql fails altogether and returns empty stdout, poll_query_until will treat that as a success! That's because, contrary to its documentation, it makes no actual check for psql failure, looking neither at the exit status nor at stderr. To fix that, adjust poll_query_until to insist on empty stderr as well as a stdout match. (I experimented with checking exit status instead, but it seems that psql often does exit(1) in cases that we need to consider successes. That might be something to fix someday, but it would be a non-back-patchable behavior change.) Back-patch to v10. The test cases needing this exist only as far back as v11, but it seems wise to keep poll_query_until's behavior the same in v10, in case we back-patch another such test case in future. (9.6 does not currently need this change, because in that branch poll_query_until can't be told to accept empty stdout as a success case.) Per assorted buildfarm failures, mostly on hoverfly. Discussion: https://postgr.es/m/CAA4eK1+zM6L4QSA1XMvXY_qqWwdUmqkOS1+hWvL8QcYEBGA1Uw@mail.gmail.com
1 parent 7681b78 commit 0b619df

File tree

2 files changed

+6
-10
lines changed

2 files changed

+6
-10
lines changed

src/test/perl/PostgresNode.pm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,8 +1560,10 @@ sub poll_query_until
15601560

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

1564-
if ($stdout eq $expected)
1566+
if ($stdout eq $expected && $stderr eq '')
15651567
{
15661568
return 1;
15671569
}
@@ -1574,8 +1576,6 @@ sub poll_query_until
15741576

15751577
# The query result didn't change in 180 seconds. Give up. Print the
15761578
# output from the last attempt, hopefully that's useful for debugging.
1577-
$stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
1578-
chomp($stderr);
15791579
diag qq(poll_query_until timed out executing this query:
15801580
$query
15811581
expecting this output:

src/test/recovery/t/013_crash_restart.pl

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,8 @@
134134
$monitor->finish;
135135

136136
# Wait till server restarts
137-
is( $node->poll_query_until(
138-
'postgres',
139-
'SELECT $$restarted after sigquit$$;',
140-
'restarted after sigquit'),
141-
"1",
142-
"reconnected after SIGQUIT");
137+
is($node->poll_query_until('postgres', undef, ''),
138+
"1", "reconnected after SIGQUIT");
143139

144140

145141
# restart psql processes, now that the crash cycle finished
@@ -214,7 +210,7 @@
214210
$monitor->finish;
215211

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

220216
# Make sure the committed rows survived, in-progress ones not

0 commit comments

Comments
 (0)