timeouts in PostgresNode::psql
Hi all
While writing some tests I noticed that in newer IPC::Run or Perl
versions (haven't dug extensively to find out which), perl appends the
location to the extension, so 'ne' doesn't match the passed exception
string.
Pattern-match the exception string to handle this.
Bugfix, should be applied to 9.6 and master.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Fix-timeouts-in-PostgresNode-psql.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-timeouts-in-PostgresNode-psql.patchDownload
From ba803ba3c94624ce5e6044e14272b2db2793252c Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Fri, 24 Feb 2017 11:43:30 +0800
Subject: [PATCH] Fix timeouts in PostgresNode::psql
Newer Perl or IPC::Run versions default to appending the filename to string
exceptions, e.g. the exception
psql timed out
is thrown as
psql timed out at /usr/share/perl5/vendor_perl/IPC/Run.pm line 2961.
To handle this, match exceptions with !~ rather than ne.
---
src/test/perl/PostgresNode.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 9b712eb..d0a996d 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1116,7 +1116,7 @@ sub psql
# IPC::Run::run threw an exception. re-throw unless it's a
# timeout, which we'll handle by testing is_expired
die $exc_save
- if (blessed($exc_save) || $exc_save ne $timeout_exception);
+ if (blessed($exc_save) || $exc_save !~ /^$timeout_exception.*/);
$ret = undef;
--
2.5.5
Amended patch attached after a few Perl-related comments I got on
private mail. Instead of
$exc_save !~ /^$timeout_exception.*/
I've updated to:
$exc_save !~ /^\Q$timeout_exception\E/
i.e. don't do an unnecessary wildcard match at the end, and disable
metachar interpretation in the substituted range.
Still needs applying to pg9.6 and pg10.
Attachments:
0001-Fix-timeouts-in-PostgresNode-psql.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-timeouts-in-PostgresNode-psql.patchDownload
From f96b78c19c09fda83b26b75cca500dd35c849002 Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Fri, 24 Feb 2017 11:43:30 +0800
Subject: [PATCH] Fix timeouts in PostgresNode::psql
Newer Perl or IPC::Run versions default to appending the filename to string
exceptions, e.g. the exception
psql timed out
is thrown as
psql timed out at /usr/share/perl5/vendor_perl/IPC/Run.pm line 2961.
To handle this, match exceptions with !~ rather than ne.
---
src/test/perl/PostgresNode.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 9b712eb..bd627b2 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1116,7 +1116,7 @@ sub psql
# IPC::Run::run threw an exception. re-throw unless it's a
# timeout, which we'll handle by testing is_expired
die $exc_save
- if (blessed($exc_save) || $exc_save ne $timeout_exception);
+ if (blessed($exc_save) || $exc_save !~ /^\Q$timeout_exception\E/);
$ret = undef;
--
2.5.5
On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Amended patch attached after a few Perl-related comments I got on
private mail.
Out of curiosity, what are they?
Instead of
$exc_save !~ /^$timeout_exception.*/
I've updated to:
$exc_save !~ /^\Q$timeout_exception\E/
i.e. don't do an unnecessary wildcard match at the end, and disable
metachar interpretation in the substituted range.Still needs applying to pg9.6 and pg10.
I did not understand at first what you meant, but after looking at the
commit message of the patch things are clear:
Newer Perl or IPC::Run versions default to appending the filename to string
exceptions, e.g. the exception
psql timed out
is thrown as
psql timed out at /usr/share/perl5/vendor_perl/IPC/Run.pm line 2961.
And that's good to know, I can see as well that the patch is on the CF app:
https://commitfest.postgresql.org/13/
And that it has been marked as ready for committer.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier <michael.paquier@gmail.com> writes:
On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Still needs applying to pg9.6 and pg10.
I did not understand at first what you meant, but after looking at the
commit message of the patch things are clear:
Newer Perl or IPC::Run versions default to appending the filename to string
exceptions, e.g. the exception
psql timed out
is thrown as
psql timed out at /usr/share/perl5/vendor_perl/IPC/Run.pm line 2961.And that's good to know, I can see as well that the patch is on the CF app:
https://commitfest.postgresql.org/13/
And that it has been marked as ready for committer.
That was me reviewing it, but the email to the list bounced, because the
commitfest app (or the python email.message module, depending on who you
feel like blaming) doesn't handle non-ASCII characters in structured
headers correctly. In my case, it generated the header
From: =?utf-8?q?Dagfinn_Ilmari_Manns=C3=A5ker_=3Cilmari=40ilmari=2Eorg=3E?=\n\n
when it should have been
From: =?utf-8?q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
Anyway, my rewview was as follows:
Tested by copying the timeout example from the PostgresNode docs into a
test file and adding an assertion that the variable passed to the
'timed_out' parameter gets set. Without the patch the test script dies
when the timeout occurs, with the patch the test passes.
PostgresNode itself could do with some tests (nothing in the actual
tests seems to use the timeout functionality), but that's for another
patch.
--
"I use RMS as a guide in the same way that a boat captain would use
a lighthouse. It's good to know where it is, but you generally
don't want to find yourself in the same spot." - Tollef Fog Heen
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier wrote:
On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Instead of
$exc_save !~ /^$timeout_exception.*/
I've updated to:
$exc_save !~ /^\Q$timeout_exception\E/
i.e. don't do an unnecessary wildcard match at the end, and disable
metachar interpretation in the substituted range.Still needs applying to pg9.6 and pg10.
I did not understand at first what you meant, but after looking at the
commit message of the patch things are clear:
Newer Perl or IPC::Run versions default to appending the filename to string
exceptions, e.g. the exception
psql timed out
is thrown as
psql timed out at /usr/share/perl5/vendor_perl/IPC/Run.pm line 2961.
Hmm, I think this is really a bugfix that we should backpatch all the
way back to where we introduced PostgresNode.
Lately I've been wondering about backpatching the whole TAP test
infrastructure, all the way back. As we notice bugs, it's really useful
to use newly added tests in all branches; but currently PostgresNode
doesn't work with old branches, particularly since the '-w' switch was
removed from pg_ctl invokations in PostgresNode->start and ->restart
methods -- (the test just fail without any indication of what is going
on).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/28/2017 07:39 AM, Alvaro Herrera wrote:
Lately I've been wondering about backpatching the whole TAP test
infrastructure, all the way back. As we notice bugs, it's really useful
to use newly added tests in all branches; but currently PostgresNode
doesn't work with old branches, particularly since the '-w' switch was
removed from pg_ctl invokations in PostgresNode->start and ->restart
methods -- (the test just fail without any indication of what is going
on).
+1
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 28 February 2017 at 20:39, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Lately I've been wondering about backpatching the whole TAP test
infrastructure, all the way back. As we notice bugs, it's really useful
to use newly added tests in all branches; but currently PostgresNode
doesn't work with old branches, particularly since the '-w' switch was
removed from pg_ctl invokations in PostgresNode->start and ->restart
methods -- (the test just fail without any indication of what is going
on).
Yeah. I actually did prepare a backpatch of PostgresNode to 9.5 (with
room to go back further), and it's pretty simple if you don't care
about breaking all existing TAP tests ;)
The irritating bit is that the various TAP stuff introduced earlier
for src/bin and so on uses its own node management plus some helpers
in TestLib. Those helpers went away when we introduced PostgresNode
and they aren't easily emulated because the tests make assumptions
about the directory structure that don't fit well with PostgresNode's
way of doing things.
It's fixable, it just wasn't worth the hassle for what I needed given
that I saw the backport as having minimal chance of getting into core.
I've pushed what I did to
https://github.com/2ndQuadrant/postgres/tree/dev/backport-96-tap-to-95
in case anyone finds it useful.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/26/17 21:28, Craig Ringer wrote:
Amended patch attached after a few Perl-related comments I got on
private mail. Instead of$exc_save !~ /^$timeout_exception.*/
I've updated to:
$exc_save !~ /^\Q$timeout_exception\E/
i.e. don't do an unnecessary wildcard match at the end, and disable
metachar interpretation in the substituted range.Still needs applying to pg9.6 and pg10.
committed to master and 9.6
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2 March 2017 at 03:19, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2/26/17 21:28, Craig Ringer wrote:
Amended patch attached after a few Perl-related comments I got on
private mail. Instead of$exc_save !~ /^$timeout_exception.*/
I've updated to:
$exc_save !~ /^\Q$timeout_exception\E/
i.e. don't do an unnecessary wildcard match at the end, and disable
metachar interpretation in the substituted range.Still needs applying to pg9.6 and pg10.
committed to master and 9.6
Thanks.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers