PostgresNode::poll_query_until hacking
The attached proposed patch changes the TAP test infrastructure's
poll_query_until function in two ways:
1. An optional argument is added to allow specifying the query result
value we're waiting for, overriding the normal "t". This allows
folding a handwritten delay loop in 007_sync_rep.pl into the
poll_query_until ecosystem. As far as I've found, there are no other
handwritten delay loops in the TAP tests.
2. poll_query_until is modified to probe 10X per second not once
per second, in keeping with the changes I've been making elsewhere
to remove not-carefully-analyzed 1s delays in the regression tests.
On my workstation, the reduced probe delay shaves a useful amount
of time off the recovery and subscription regression tests. I also
tried it on dromedary, which is about the slowest hardware I'd care
to run the TAP tests on regularly, and it seems to be about a wash
there --- some tests get faster, but some get slower, presumably due
to more overhead from the probe queries.
I notice that buildfarm member skink (which runs with valgrind)
recently failed a test run due to poll_query_until timing out:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2017-06-30%2000%3A50%3A01
I'm inclined to respond to that either by increasing the fixed
180-second timeout, or by making it configurable from an environment
variable (which Andres would then have to add to skink's configuration).
Thoughts?
regards, tom lane
Attachments:
improve-poll_query_until.patchtext/x-diff; charset=us-ascii; name=improve-poll_query_until.patchDownload+48-48
On Sun, Jul 2, 2017 at 4:53 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
The attached proposed patch changes the TAP test infrastructure's
poll_query_until function in two ways:1. An optional argument is added to allow specifying the query result
value we're waiting for, overriding the normal "t". This allows
folding a handwritten delay loop in 007_sync_rep.pl into the
poll_query_until ecosystem.
True that in this test the expected output can be quite complicated,
so turning that into a query that returned 't' would make the code
unreadable.
As far as I've found, there are no other
handwritten delay loops in the TAP tests.
Good catch. Yes here using a poll_query_until call makes sense.
2. poll_query_until is modified to probe 10X per second not once
per second, in keeping with the changes I've been making elsewhere
to remove not-carefully-analyzed 1s delays in the regression tests.On my workstation, the reduced probe delay shaves a useful amount
of time off the recovery and subscription regression tests. I also
tried it on dromedary, which is about the slowest hardware I'd care
to run the TAP tests on regularly, and it seems to be about a wash
there --- some tests get faster, but some get slower, presumably due
to more overhead from the probe queries.
Check.
Thoughts?
- is($result, $expected, $msg);
+ ok( $self->poll_query_until('postgres', $check_sql, $expected), $msg);
A matter of taste here: using a space after "ok(".
If you would like to get some feedback from me, waiting until Monday
morning my time (Sunday evening yours) before pushing patches would be
better.
--
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:
If you would like to get some feedback from me, waiting until Monday
morning my time (Sunday evening yours) before pushing patches would be
better.
If you have ideas for improvement, it's always possible to amend the
code later. I've been pushing this stuff to see what happens in the
buildfarm, not to foreclose discussion.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers