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 .
BTW, I suggest adding --timer to our default PROVE_FLAGS, so we can
collect more data from the buildfarm on what takes a while. Sample
output:
[13:20:47] t/001_stream_rep.pl .................. ok 50445 ms (
0.01 usr 0.00 sys + 2.16 cusr 3.57 csys = 5.74 CPU)
[13:21:38] t/002_archiving.pl ................... ok 5883 ms (
0.01 usr 0.00 sys + 0.19 cusr 0.45 csys = 0.65 CPU)
[13:21:44] t/003_recovery_targets.pl ............ ok 26197 ms (
0.00 usr 0.00 sys + 0.70 cusr 1.75 csys = 2.45 CPU)
[13:22:10] t/004_timeline_switch.pl ............. ok 10049 ms (
0.01 usr 0.00 sys + 0.24 cusr 0.53 csys = 0.78 CPU)
[13:22:20] t/005_replay_delay.pl ................ ok 10224 ms (
0.00 usr 0.01 sys + 0.18 cusr 0.43 csys = 0.62 CPU)
[13:22:30] t/006_logical_decoding.pl ............ ok 7570 ms (
0.00 usr 0.00 sys + 0.25 cusr 0.32 csys = 0.57 CPU)
[13:22:38] t/007_sync_rep.pl .................... ok 20693 ms (
0.00 usr 0.00 sys + 0.51 cusr 1.24 csys = 1.75 CPU)
[13:22:58] t/008_fsm_truncation.pl .............. ok 25399 ms (
0.01 usr 0.00 sys + 0.26 cusr 0.52 csys = 0.79 CPU)
[13:23:24] t/009_twophase.pl .................... ok 58531 ms (
0.01 usr 0.00 sys + 0.49 cusr 0.92 csys = 1.42 CPU)
[13:24:22] t/010_logical_decoding_timelines.pl .. ok 8135 ms (
0.00 usr 0.01 sys + 0.34 cusr 0.76 csys = 1.11 CPU)
[13:24:30] t/011_crash_recovery.pl .............. ok 4334 ms (
0.00 usr 0.00 sys + 0.12 cusr 0.16 csys = 0.28 CPU)
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From d2b30831803eb3edb2e39b2e49b89bc74a65ee37 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Wed, 12 Apr 2017 13:25:19 +0800
Subject: [PATCH] Cache initdb runs in TAP tests
---
src/test/perl/PostgresNode.pm | 51 +++++++++++++++++++++++++++++++++++++++---
src/test/perl/RecursiveCopy.pm | 5 +++++
src/test/perl/TestLib.pm | 5 +++++
3 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index cb84f1f..eef9f66 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -391,13 +391,28 @@ sub init
$params{allows_streaming} = 0 unless defined $params{allows_streaming};
$params{has_archiving} = 0 unless defined $params{has_archiving};
+ $params{cache_initdb} = 1 unless defined $params{cache_initdb};
mkdir $self->backup_dir;
mkdir $self->archive_dir;
- TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
- @{ $params{extra} });
- TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+ # Respect CACHE_INITDB=0 in the environment if set
+ $params{cache_initdb} = 0
+ if (defined($ENV{'CACHE_INITDB'}) && !$ENV{'CACHE_INITDB'});
+
+ # We don't cache initdb results for non-default runs
+ # and they're too uncommon to care about anyway.
+ $params{cache_initdb} = 0
+ if defined($params{extra});
+
+ if ($params{cache_initdb})
+ {
+ $self->_initdb_cached(%params);
+ }
+ else
+ {
+ $self->_initdb($pgdata, %params);
+ }
open my $conf, '>>', "$pgdata/postgresql.conf";
print $conf "\n# Added by PostgresNode.pm\n";
@@ -446,6 +461,36 @@ sub init
$self->enable_archiving if $params{has_archiving};
}
+# Wrapper to actually run initdb its self
+sub _initdb
+{
+ my ($self, $pgdata, %params) = @_;
+
+ TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
+ @{ $params{extra} });
+ TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+}
+
+sub _initdb_cached
+{
+ my ($self, %params) = @_;
+ my $pgdata = $self->data_dir;
+
+ my $cachedir = TestLib::tmp_check_dir() . "/initdb_cache";
+
+ 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);
+ }
+
+ RecursiveCopy::copypath($cachedir, $pgdata);
+}
+
=pod
=item $node->append_conf(filename, str)
diff --git a/src/test/perl/RecursiveCopy.pm b/src/test/perl/RecursiveCopy.pm
index 28ecaf6..103b976 100644
--- a/src/test/perl/RecursiveCopy.pm
+++ b/src/test/perl/RecursiveCopy.pm
@@ -22,6 +22,9 @@ use warnings;
use File::Basename;
use File::Copy;
+# see https://perldoc.perl.org/perlport.html#stat
+${^WIN32_SLOPPY_STAT} = 1;
+
=pod
=head1 DESCRIPTION
@@ -111,6 +114,8 @@ sub _copypath_recurse
# Otherwise this is directory: create it on dest and recurse onto it.
mkdir($destpath) or die "mkdir($destpath) failed: $!";
+ # It's OK to run this on win32, see perlport
+ chmod((stat $srcpath)[2] & 07777, $destpath) or die "chmod() of $destpath failed: $!";
opendir(my $directory, $srcpath) or die "could not opendir($srcpath): $!";
while (my $entry = readdir($directory))
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index ae8d178..5187f75 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -141,6 +141,11 @@ sub tempdir
CLEANUP => 1);
}
+sub tmp_check_dir
+{
+ return $tmp_check;
+}
+
sub tempdir_short
{
--
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
