On 12 April 2017 at 13:27, Craig Ringer <craig.rin...@2ndquadrant.com> wrote:
> On 12 April 2017 at 08:22, Michael Paquier <michael.paqu...@gmail.com> wrote:
>> On Wed, Apr 12, 2017 at 9:12 AM, Craig Ringer
>> <craig.rin...@2ndquadrant.com> wrote:
>>> Well, you can get a lot of information on timings in crake's latest
>>> builds for example, or nightjar's.
>>>
>>> Here's an interesting fact: almost all the time taken up in the scripts
>>> test set seems to be consumed in running initdb over and over.
>>>
>>> Is it worth adding an init(cache => 1) option to PostgresNode, where we
>>> stash an initdb'd db in tmp_check/ and just do a simple fs copy of it ?
>>
>> This looks like a good idea to me, but I don't like much the idea of
>> an option in the init() routine as that's hard to maintain.
>
> How so?
>
>> It would
>> make sense to me to be able to override the initialization with an
>> environment variable instead
>
> Yeah, that's reasonable.
>
> Patch attached. It supports setting CACHE_INITDB=0 to disable the cache.
>
> With cache: 3m55.063s (user 7.7s, sys 0m11.833s)
>
> Without cache: 4m11.229s (user 0m16.963s, sys 0m12.384s)
>
> so in a serial run it doesn't make a ton of difference.
>
> Adding some timing on initdb runs shows that initdb takes about 1s, a
> cached copy about 0.1s with our Perl based recursive copy code. So
> it's hardly an overwhelming benefit, but might be more beneficial with
> prove -j .
Of course, it'll need locking for prove -j. Added. Patch attached,
applies on top of prior patch.
With
make PROVE_FLAGS="--timer -j 9" check
I don't see much difference with/without caching initdb results -
saves about 4 seconds, from 74 to 70 seconds, but hard to tell with
the error margins.
So if we're going to do anything, defaulting to parallel prove seems a
good start, and I'm not sure caching initdb results is worth it.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From db1aadf450a15cbd4041c5f9bf5a0b16f66c2ad5 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Wed, 12 Apr 2017 14:03:25 +0800
Subject: [PATCH] Add locking for initdb caching to allow parallel prove
---
src/test/perl/PostgresNode.pm | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index eef9f66..0eacd41 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -94,6 +94,7 @@ use Socket;
use Test::More;
use TestLib ();
use Scalar::Util qw(blessed);
+use Fcntl qw(:flock);
our @EXPORT = qw(
get_new_node
@@ -477,15 +478,27 @@ sub _initdb_cached
my $pgdata = $self->data_dir;
my $cachedir = TestLib::tmp_check_dir() . "/initdb_cache";
+ my $lockfile = TestLib::tmp_check_dir() . "/.initdb_cache.lock";
if (! -d $cachedir)
{
print(STDERR "initializing initdb cache\n");
- # initdb into a tempdir, then rename the result so we don't risk
- # leaving failed initdb results around
- my $temp_dbdir = TestLib::tempdir("initdb_cache_");
- $self->_initdb($temp_dbdir, %params);
- rename($temp_dbdir, $cachedir);
+ open my $lockfh, ">", $lockfile
+ or die "cannot open $lockfile for writing: $1\n";
+ flock($lockfh, LOCK_EX)
+ or die "cannot flock($lockfile) in exclusive mode: $!\n";
+ # If running in parallel with other tests, someone else might've got
+ # the lock first and have swapped an initdb'd directory in already.
+ if (! -d $cachedir)
+ {
+ # initdb into a tempdir, then rename the result so we don't risk
+ # leaving failed initdb results around
+ my $temp_dbdir = TestLib::tempdir("initdb_cache_");
+ $self->_initdb($temp_dbdir, %params);
+ rename($temp_dbdir, $cachedir);
+ }
+ flock($lockfh, LOCK_UN)
+ or die "cannot unlock $lockfile: $!";
}
RecursiveCopy::copypath($cachedir, $pgdata);
--
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
