Re: [HACKERS] timeouts in PostgresNode::psql
On 2 March 2017 at 03:19, Peter Eisentraut 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
Re: [HACKERS] timeouts in PostgresNode::psql
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
Re: [HACKERS] timeouts in PostgresNode::psql
On 28 February 2017 at 20:39, 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). 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
Re: [HACKERS] timeouts in PostgresNode::psql
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 Dunstanhttps://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
Re: [HACKERS] timeouts in PostgresNode::psql
Michael Paquier wrote: > On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer 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 Herrerahttps://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
Re: [HACKERS] timeouts in PostgresNode::psql
Michael Paquier writes: > On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer 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?= 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
Re: [HACKERS] timeouts in PostgresNode::psql
On Mon, Feb 27, 2017 at 11:28 AM, Craig Ringer 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
Re: [HACKERS] timeouts in PostgresNode::psql
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. From f96b78c19c09fda83b26b75cca500dd35c849002 Mon Sep 17 00:00:00 2001 From: Craig Ringer 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
