Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 252dcb3

Browse files
committed
Use "template" data directory in tests
When running all (or just many) of our tests, a significant portion of both CPU time and IO is spent running initdb. Most of those initdb runs don't specify any options influencing properties of the created data directory. Avoid most of that overhead by creating a "template" data directory, alongside the temporary installation. Instead of running initdb, pg_regress and tap tests can copy that data directory. When a tap test specifies options to initdb, the template data directory is not used. That could be relaxed for some options, but it's not clear it's worth the effort. There unfortunately is some duplication between pg_regress.c and Cluster.pm, but there are no easy ways of sharing that code without introducing additional complexity. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de
1 parent 9625845 commit 252dcb3

File tree

5 files changed

+156
-37
lines changed

5 files changed

+156
-37
lines changed

.cirrus.tasks.yml

+2-1
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,9 @@ task:
109109
test_minimal_script: |
110110
su postgres <<-EOF
111111
ulimit -c unlimited
112+
meson test $MTEST_ARGS --suite setup
112113
meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \
113-
tmp_install cube/regress pg_ctl/001_start_stop
114+
cube/regress pg_ctl/001_start_stop
114115
EOF
115116
116117
on_failure:

meson.build

+30
Original file line numberDiff line numberDiff line change
@@ -3070,8 +3070,10 @@ testport = 40000
30703070
test_env = environment()
30713071

30723072
temp_install_bindir = test_install_location / get_option('bindir')
3073+
test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template'
30733074
test_env.set('PG_REGRESS', pg_regress.full_path())
30743075
test_env.set('REGRESS_SHLIB', regress_module.full_path())
3076+
test_env.set('INITDB_TEMPLATE', test_initdb_template)
30753077

30763078
# Test suites that are not safe by default but can be run if selected
30773079
# by the user via the whitespace-separated list in variable PG_TEST_EXTRA.
@@ -3086,6 +3088,34 @@ if library_path_var != ''
30863088
endif
30873089

30883090

3091+
# Create (and remove old) initdb template directory. Tests use that, where
3092+
# possible, to make it cheaper to run tests.
3093+
#
3094+
# Use python to remove the old cached initdb, as we cannot rely on a working
3095+
# 'rm' binary on windows.
3096+
test('initdb_cache',
3097+
python,
3098+
args: [
3099+
'-c', '''
3100+
import shutil
3101+
import sys
3102+
import subprocess
3103+
3104+
shutil.rmtree(sys.argv[1], ignore_errors=True)
3105+
sp = subprocess.run(sys.argv[2:] + [sys.argv[1]])
3106+
sys.exit(sp.returncode)
3107+
''',
3108+
test_initdb_template,
3109+
temp_install_bindir / 'initdb',
3110+
'-A', 'trust', '-N', '--no-instructions'
3111+
],
3112+
priority: setup_tests_priority - 1,
3113+
timeout: 300,
3114+
is_parallel: false,
3115+
env: test_env,
3116+
suite: ['setup'])
3117+
3118+
30893119

30903120
###############################################################
30913121
# Test Generation

src/Makefile.global.in

+21-17
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,25 @@ check: temp-install
397397

398398
.PHONY: temp-install
399399

400+
401+
# prepend to path if already set, else just set it
402+
define add_to_path
403+
$(1)="$(if $($(1)),$(2):$$$(1),$(2))"
404+
endef
405+
406+
# platform-specific environment variable to set shared library path
407+
# individual ports can override this later, this is the default name
408+
ld_library_path_var = LD_LIBRARY_PATH
409+
410+
# with_temp_install_extra is for individual ports to define if they
411+
# need something more here. If not defined then the expansion does
412+
# nothing.
413+
with_temp_install = \
414+
PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \
415+
$(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \
416+
INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \
417+
$(with_temp_install_extra)
418+
400419
temp-install: | submake-generated-headers
401420
ifndef NO_TEMP_INSTALL
402421
ifneq ($(abs_top_builddir),)
@@ -405,6 +424,8 @@ ifeq ($(MAKELEVEL),0)
405424
$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
406425
$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
407426
$(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
427+
428+
$(with_temp_install) initdb -A trust -N --no-instructions '$(abs_top_builddir)'/tmp_install/initdb-template >>'$(abs_top_builddir)'/tmp_install/log/initdb-template.log 2>&1
408429
endif
409430
endif
410431
endif
@@ -422,23 +443,6 @@ PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
422443
# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS.
423444
PROVE_FLAGS =
424445

425-
# prepend to path if already set, else just set it
426-
define add_to_path
427-
$(1)="$(if $($(1)),$(2):$$$(1),$(2))"
428-
endef
429-
430-
# platform-specific environment variable to set shared library path
431-
# individual ports can override this later, this is the default name
432-
ld_library_path_var = LD_LIBRARY_PATH
433-
434-
# with_temp_install_extra is for individual ports to define if they
435-
# need something more here. If not defined then the expansion does
436-
# nothing.
437-
with_temp_install = \
438-
PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \
439-
$(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \
440-
$(with_temp_install_extra)
441-
442446
ifeq ($(enable_tap_tests),yes)
443447

444448
ifndef PGXS

src/test/perl/PostgreSQL/Test/Cluster.pm

+44-2
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,50 @@ sub init
522522
mkdir $self->backup_dir;
523523
mkdir $self->archive_dir;
524524

525-
PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A',
526-
'trust', '-N', @{ $params{extra} });
525+
# If available and if there aren't any parameters, use a previously
526+
# initdb'd cluster as a template by copying it. For a lot of tests, that's
527+
# substantially cheaper. Do so only if there aren't parameters, it doesn't
528+
# seem worth figuring out whether they affect compatibility.
529+
#
530+
# There's very similar code in pg_regress.c, but we can't easily
531+
# deduplicate it until we require perl at build time.
532+
if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE})
533+
{
534+
note("initializing database system by running initdb");
535+
PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A',
536+
'trust', '-N', @{ $params{extra} });
537+
}
538+
else
539+
{
540+
my @copycmd;
541+
my $expected_exitcode;
542+
543+
note("initializing database system by copying initdb template");
544+
545+
if ($PostgreSQL::Test::Utils::windows_os)
546+
{
547+
@copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP);
548+
$expected_exitcode = 1; # 1 denotes files were copied
549+
}
550+
else
551+
{
552+
@copycmd = qw(cp -a);
553+
$expected_exitcode = 0;
554+
}
555+
556+
@copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata);
557+
558+
my $ret = PostgreSQL::Test::Utils::system_log(@copycmd);
559+
560+
# See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR
561+
if ($ret & 127 or $ret >> 8 != $expected_exitcode)
562+
{
563+
BAIL_OUT(
564+
sprintf("failed to execute command \"%s\": $ret",
565+
join(" ", @copycmd)));
566+
}
567+
}
568+
527569
PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS},
528570
'--config-auth', $pgdata, @{ $params{auth_extra} });
529571

src/test/regress/pg_regress.c

+59-17
Original file line numberDiff line numberDiff line change
@@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[],
22952295
FILE *pg_conf;
22962296
const char *env_wait;
22972297
int wait_seconds;
2298+
const char *initdb_template_dir;
22982299

22992300
/*
23002301
* Prepare the temp instance
@@ -2316,25 +2317,66 @@ regression_main(int argc, char *argv[],
23162317
if (!directory_exists(buf))
23172318
make_directory(buf);
23182319

2319-
/* initdb */
23202320
initStringInfo(&cmd);
2321-
appendStringInfo(&cmd,
2322-
"\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync",
2323-
bindir ? bindir : "",
2324-
bindir ? "/" : "",
2325-
temp_instance);
2326-
if (debug)
2327-
appendStringInfo(&cmd, " --debug");
2328-
if (nolocale)
2329-
appendStringInfo(&cmd, " --no-locale");
2330-
appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
2331-
fflush(NULL);
2332-
if (system(cmd.data))
2321+
2322+
/*
2323+
* Create data directory.
2324+
*
2325+
* If available, use a previously initdb'd cluster as a template by
2326+
* copying it. For a lot of tests, that's substantially cheaper.
2327+
*
2328+
* There's very similar code in Cluster.pm, but we can't easily de
2329+
* duplicate it until we require perl at build time.
2330+
*/
2331+
initdb_template_dir = getenv("INITDB_TEMPLATE");
2332+
if (initdb_template_dir == NULL || nolocale || debug)
2333+
{
2334+
note("initializing database system by running initdb");
2335+
2336+
appendStringInfo(&cmd,
2337+
"\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync",
2338+
bindir ? bindir : "",
2339+
bindir ? "/" : "",
2340+
temp_instance);
2341+
if (debug)
2342+
appendStringInfo(&cmd, " --debug");
2343+
if (nolocale)
2344+
appendStringInfo(&cmd, " --no-locale");
2345+
appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
2346+
fflush(NULL);
2347+
if (system(cmd.data))
2348+
{
2349+
bail("initdb failed\n"
2350+
"# Examine \"%s/log/initdb.log\" for the reason.\n"
2351+
"# Command was: %s",
2352+
outputdir, cmd.data);
2353+
}
2354+
}
2355+
else
23332356
{
2334-
bail("initdb failed\n"
2335-
"# Examine \"%s/log/initdb.log\" for the reason.\n"
2336-
"# Command was: %s",
2337-
outputdir, cmd.data);
2357+
#ifndef WIN32
2358+
const char *copycmd = "cp -a \"%s\" \"%s/data\"";
2359+
int expected_exitcode = 0;
2360+
#else
2361+
const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\"";
2362+
int expected_exitcode = 1; /* 1 denotes files were copied */
2363+
#endif
2364+
2365+
note("initializing database system by copying initdb template");
2366+
2367+
appendStringInfo(&cmd,
2368+
copycmd,
2369+
initdb_template_dir,
2370+
temp_instance);
2371+
appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
2372+
fflush(NULL);
2373+
if (system(cmd.data) != expected_exitcode)
2374+
{
2375+
bail("copying of initdb template failed\n"
2376+
"# Examine \"%s/log/initdb.log\" for the reason.\n"
2377+
"# Command was: %s",
2378+
outputdir, cmd.data);
2379+
}
23382380
}
23392381

23402382
pfree(cmd.data);

0 commit comments

Comments
 (0)