agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH 2/4] fix costing in cost_incremental_sort 14+ messages / 4 participants [nested] [flat]
* [PATCH 2/4] fix costing in cost_incremental_sort @ 2019-07-08 22:13 Tomas Vondra <tomas@2ndquadrant.com> 0 siblings, 0 replies; 14+ messages in thread From: Tomas Vondra @ 2019-07-08 22:13 UTC (permalink / raw) --- src/backend/optimizer/path/costsize.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 7f820e7351..c6aa17ba67 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -1875,16 +1875,8 @@ cost_incremental_sort(Path *path, limit_tuples); /* If we have a LIMIT, adjust the number of groups we'll have to return. */ - if (limit_tuples > 0 && limit_tuples < input_tuples) - { - output_tuples = limit_tuples; - output_groups = floor(output_tuples / group_tuples) + 1; - } - else - { - output_tuples = input_tuples; - output_groups = input_groups; - } + output_tuples = input_tuples; + output_groups = input_groups; /* * Startup cost of incremental sort is the startup cost of its first group -- 2.21.0 --z44ikx7gwyo2xwzv Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0003-fix-explain-in-parallel-mode-20190709.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 2/4] fix costing in cost_incremental_sort @ 2019-07-08 22:13 Tomas Vondra <tomas@2ndquadrant.com> 0 siblings, 0 replies; 14+ messages in thread From: Tomas Vondra @ 2019-07-08 22:13 UTC (permalink / raw) --- src/backend/optimizer/path/costsize.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 7f820e7351..c6aa17ba67 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -1875,16 +1875,8 @@ cost_incremental_sort(Path *path, limit_tuples); /* If we have a LIMIT, adjust the number of groups we'll have to return. */ - if (limit_tuples > 0 && limit_tuples < input_tuples) - { - output_tuples = limit_tuples; - output_groups = floor(output_tuples / group_tuples) + 1; - } - else - { - output_tuples = input_tuples; - output_groups = input_groups; - } + output_tuples = input_tuples; + output_groups = input_groups; /* * Startup cost of incremental sort is the startup cost of its first group -- 2.20.1 --5bdgxkv4n6n2squ7 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0003-fix-explain-in-parallel-mode.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v1 2/9] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 0a11efc97a1..0e8a72b12b9 100644 --- a/meson.build +++ b/meson.build @@ -3048,8 +3048,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3064,6 +3066,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index e9cfc542cfe..f08da65ed76 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --uu4yojthqnm7ulmd Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0003-ci-macos-Remove-use-of-DRANDOMIZE_ALLOCATED_MEMOR.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 1/2] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 04ea3488522..47429a18c3f 100644 --- a/meson.build +++ b/meson.build @@ -3056,8 +3056,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3072,6 +3074,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index d260f15c4e2..8fce17dff08 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --m6x7pdqtqaxzqass-- ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 1/2] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 04ea3488522..47429a18c3f 100644 --- a/meson.build +++ b/meson.build @@ -3056,8 +3056,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3072,6 +3074,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index d260f15c4e2..8fce17dff08 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --m6x7pdqtqaxzqass-- ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 07/10] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ .cirrus.tasks.yml | 3 +- src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index f5ec442f9a9..8b2b521a013 100644 --- a/meson.build +++ b/meson.build @@ -3070,8 +3070,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3086,6 +3088,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index 0cf7ba77996..e137769850d 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -109,8 +109,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --uh2yukyzfvojbe2k Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0008-ci-switch-tasks-to-debugoptimized-build.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 1/2] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 04ea3488522..47429a18c3f 100644 --- a/meson.build +++ b/meson.build @@ -3056,8 +3056,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3072,6 +3074,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index d260f15c4e2..8fce17dff08 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --m6x7pdqtqaxzqass-- ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 07/10] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ .cirrus.tasks.yml | 3 +- src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index f5ec442f9a9..8b2b521a013 100644 --- a/meson.build +++ b/meson.build @@ -3070,8 +3070,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3086,6 +3088,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index 0cf7ba77996..e137769850d 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -109,8 +109,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --uh2yukyzfvojbe2k Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0008-ci-switch-tasks-to-debugoptimized-build.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 1/2] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 04ea3488522..47429a18c3f 100644 --- a/meson.build +++ b/meson.build @@ -3056,8 +3056,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3072,6 +3074,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index d260f15c4e2..8fce17dff08 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --m6x7pdqtqaxzqass-- ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v1 2/9] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 0a11efc97a1..0e8a72b12b9 100644 --- a/meson.build +++ b/meson.build @@ -3048,8 +3048,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3064,6 +3066,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index e9cfc542cfe..f08da65ed76 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --uu4yojthqnm7ulmd Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0003-ci-macos-Remove-use-of-DRANDOMIZE_ALLOCATED_MEMOR.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v1 2/9] Use "template" initdb in tests @ 2023-02-03 05:51 Andres Freund <andres@anarazel.de> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw) Discussion: https://postgr.es/m/20220120021859.3zpsfqn4z7ob7afz@alap3.anarazel.de --- meson.build | 30 ++++++++++ .cirrus.yml | 3 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++- src/test/regress/pg_regress.c | 74 ++++++++++++++++++------ src/Makefile.global.in | 52 +++++++++-------- 5 files changed, 161 insertions(+), 44 deletions(-) diff --git a/meson.build b/meson.build index 0a11efc97a1..0e8a72b12b9 100644 --- a/meson.build +++ b/meson.build @@ -3048,8 +3048,10 @@ testport = 40000 test_env = environment() temp_install_bindir = test_install_location / get_option('bindir') +test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template' test_env.set('PG_REGRESS', pg_regress.full_path()) test_env.set('REGRESS_SHLIB', regress_module.full_path()) +test_env.set('INITDB_TEMPLATE', test_initdb_template) # Test suites that are not safe by default but can be run if selected # by the user via the whitespace-separated list in variable PG_TEST_EXTRA. @@ -3064,6 +3066,34 @@ if library_path_var != '' endif +# Create (and remove old) initdb template directory. Tests use that, where +# possible, to make it cheaper to run tests. +# +# Use python to remove the old cached initdb, as we cannot rely on a working +# 'rm' binary on windows. +test('initdb_cache', + python, + args: [ + '-c', ''' +import shutil +import sys +import subprocess + +shutil.rmtree(sys.argv[1], ignore_errors=True) +sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +sys.exit(sp.returncode) +''', + test_initdb_template, + temp_install_bindir / 'initdb', + '-A', 'trust', '-N', '--no-instructions' + ], + priority: setup_tests_priority - 1, + timeout: 300, + is_parallel: false, + env: test_env, + suite: ['setup']) + + ############################################################### # Test Generation diff --git a/.cirrus.yml b/.cirrus.yml index e9cfc542cfe..f08da65ed76 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -115,8 +115,9 @@ task: test_minimal_script: | su postgres <<-EOF ulimit -c unlimited + meson test $MTEST_ARGS --suite setup meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \ - tmp_install cube/regress pg_ctl/001_start_stop + cube/regress pg_ctl/001_start_stop EOF on_failure: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 5e161dbee60..4d449c35de9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -522,8 +522,50 @@ sub init mkdir $self->backup_dir; mkdir $self->archive_dir; - PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', - 'trust', '-N', @{ $params{extra} }); + # If available and if there aren't any parameters, use a previously + # initdb'd cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. Do so only if there aren't parameters, it doesn't + # seem worth figuring out whether they affect compatibility. + # + # There's very similar code in pg_regress.c, but we can't easily + # deduplicate it until we require perl at build time. + if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE}) + { + note("initializing database system by running initdb"); + PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A', + 'trust', '-N', @{ $params{extra} }); + } + else + { + my @copycmd; + my $expected_exitcode; + + note("initializing database system by copying initdb template"); + + if ($PostgreSQL::Test::Utils::windows_os) + { + @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP); + $expected_exitcode = 1; # 1 denotes files were copied + } + else + { + @copycmd = qw(cp -a); + $expected_exitcode = 0; + } + + @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata); + + my $ret = PostgreSQL::Test::Utils::system_log(@copycmd); + + # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR + if ($ret & 127 or $ret >> 8 != $expected_exitcode) + { + BAIL_OUT( + sprintf("failed to execute command \"%s\": $ret", + join(" ", @copycmd))); + } + } + PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata, @{ $params{auth_extra} }); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index b68632320a7..407e3915cec 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[], FILE *pg_conf; const char *env_wait; int wait_seconds; + const char *initdb_template_dir; /* * Prepare the temp instance @@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[], if (!directory_exists(buf)) make_directory(buf); - /* initdb */ initStringInfo(&cmd); - appendStringInfo(&cmd, - "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", - bindir ? bindir : "", - bindir ? "/" : "", - temp_instance); - if (debug) - appendStringInfo(&cmd, " --debug"); - if (nolocale) - appendStringInfo(&cmd, " --no-locale"); - appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); - fflush(NULL); - if (system(cmd.data)) + + /* + * Create data directory. + * + * If available, use a previously initdb'd cluster as a template by + * copying it. For a lot of tests, that's substantially cheaper. + * + * There's very similar code in Cluster.pm, but we can't easily de + * duplicate it until we require perl at build time. + */ + initdb_template_dir = getenv("INITDB_TEMPLATE"); + if (initdb_template_dir == NULL || nolocale || debug) { - bail("initdb failed\n" - "# Examine \"%s/log/initdb.log\" for the reason.\n" - "# Command was: %s", - outputdir, cmd.data); + note("initializing database system by running initdb"); + + appendStringInfo(&cmd, + "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); + if (debug) + appendStringInfo(&cmd, " --debug"); + if (nolocale) + appendStringInfo(&cmd, " --no-locale"); + appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir); + fflush(NULL); + if (system(cmd.data)) + { + bail("initdb failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } + } + else + { +#ifndef WIN32 + const char *copycmd = "cp -a \"%s\" \"%s/data\""; + int expected_exitcode = 0; +#else + const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\""; + int expected_exitcode = 1; /* 1 denotes files were copied */ +#endif + + note("initializing database system by copying initdb template"); + + appendStringInfo(&cmd, + copycmd, + initdb_template_dir, + temp_instance); + if (system(cmd.data) != expected_exitcode) + { + bail("copying of initdb template failed\n" + "# Examine \"%s/log/initdb.log\" for the reason.\n" + "# Command was: %s", + outputdir, cmd.data); + } } pfree(cmd.data); diff --git a/src/Makefile.global.in b/src/Makefile.global.in index df9f721a41a..0b4ca0eb6ae 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -397,30 +397,6 @@ check: temp-install .PHONY: temp-install -temp-install: | submake-generated-headers -ifndef NO_TEMP_INSTALL -ifneq ($(abs_top_builddir),) -ifeq ($(MAKELEVEL),0) - rm -rf '$(abs_top_builddir)'/tmp_install - $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log - $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 - $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 -endif -endif -endif - -# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL -# entries appear more than once in the tree, and parallel installs of the same -# file can fail with EEXIST. -checkprep: - $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) - -PROVE = @PROVE@ -# There are common routines in src/test/perl, and some test suites have -# extra perl modules in their own directory. -PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) -# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. -PROVE_FLAGS = # prepend to path if already set, else just set it define add_to_path @@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \ $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \ $(with_temp_install_extra) +temp-install: | submake-generated-headers +ifndef NO_TEMP_INSTALL +ifneq ($(abs_top_builddir),) +ifeq ($(MAKELEVEL),0) + rm -rf '$(abs_top_builddir)'/tmp_install + $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log + $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 + + $(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 +endif +endif +endif + +# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL +# entries appear more than once in the tree, and parallel installs of the same +# file can fail with EEXIST. +checkprep: + $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done) + +PROVE = @PROVE@ +# There are common routines in src/test/perl, and some test suites have +# extra perl modules in their own directory. +PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) +# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS. +PROVE_FLAGS = + ifeq ($(enable_tap_tests),yes) ifndef PGXS -- 2.38.0 --uu4yojthqnm7ulmd Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0003-ci-macos-Remove-use-of-DRANDOMIZE_ALLOCATED_MEMOR.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 1/2] Don't cast away const where possible @ 2025-12-18 09:51 Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 0 siblings, 0 replies; 14+ messages in thread From: Bertrand Drouvot @ 2025-12-18 09:51 UTC (permalink / raw) Add const to read only local variables, preserving the const qualifiers from the function signatures. This does not change all such instances, but only those hand-picked by the author. The ones that are not changed: - are just thin wrappers (except when there is other changes in the same file) - would require public API changes - rely on external functions (such as LZ4F_compressUpdate()) - would require complex subsystem changes Discussion: https://postgr.es/m/aUQHy/MmWq7c97wK%40ip-10-97-1-34.eu-west-3.compute.internal --- src/backend/access/brin/brin_minmax_multi.c | 12 ++++++------ src/backend/access/heap/pruneheap.c | 4 ++-- src/backend/access/spgist/spgkdtreeproc.c | 8 ++++---- src/backend/statistics/mcv.c | 8 ++++---- src/backend/tsearch/spell.c | 4 ++-- src/test/modules/injection_points/injection_points.c | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) 22.0% src/backend/access/brin/ 9.9% src/backend/access/heap/ 14.9% src/backend/access/spgist/ 13.3% src/backend/statistics/ 9.2% src/backend/tsearch/ 30.5% src/test/modules/injection_points/ diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index 6b86b1fd889..75dd8d5083b 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -857,8 +857,8 @@ brin_range_deserialize(int maxvalues, SerializedRanges *serialized) static int compare_expanded_ranges(const void *a, const void *b, void *arg) { - ExpandedRange *ra = (ExpandedRange *) a; - ExpandedRange *rb = (ExpandedRange *) b; + const ExpandedRange *ra = a; + const ExpandedRange *rb = b; Datum r; compare_context *cxt = (compare_context *) arg; @@ -895,8 +895,8 @@ compare_expanded_ranges(const void *a, const void *b, void *arg) static int compare_values(const void *a, const void *b, void *arg) { - Datum *da = (Datum *) a; - Datum *db = (Datum *) b; + const Datum *da = a; + const Datum *db = b; Datum r; compare_context *cxt = (compare_context *) arg; @@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid, static int compare_distances(const void *a, const void *b) { - DistanceValue *da = (DistanceValue *) a; - DistanceValue *db = (DistanceValue *) b; + const DistanceValue *da = a; + const DistanceValue *db = b; if (da->value < db->value) return 1; diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index af788b29714..632c2427952 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -2021,8 +2021,8 @@ heap_log_freeze_eq(xlhp_freeze_plan *plan, HeapTupleFreeze *frz) static int heap_log_freeze_cmp(const void *arg1, const void *arg2) { - HeapTupleFreeze *frz1 = (HeapTupleFreeze *) arg1; - HeapTupleFreeze *frz2 = (HeapTupleFreeze *) arg2; + const HeapTupleFreeze *frz1 = arg1; + const HeapTupleFreeze *frz2 = arg2; if (frz1->xmax < frz2->xmax) return -1; diff --git a/src/backend/access/spgist/spgkdtreeproc.c b/src/backend/access/spgist/spgkdtreeproc.c index 6acda3fdcff..1ec0a4f59f3 100644 --- a/src/backend/access/spgist/spgkdtreeproc.c +++ b/src/backend/access/spgist/spgkdtreeproc.c @@ -84,8 +84,8 @@ typedef struct SortedPoint static int x_cmp(const void *a, const void *b) { - SortedPoint *pa = (SortedPoint *) a; - SortedPoint *pb = (SortedPoint *) b; + const SortedPoint *pa = a; + const SortedPoint *pb = b; if (pa->p->x == pb->p->x) return 0; @@ -95,8 +95,8 @@ x_cmp(const void *a, const void *b) static int y_cmp(const void *a, const void *b) { - SortedPoint *pa = (SortedPoint *) a; - SortedPoint *pb = (SortedPoint *) b; + const SortedPoint *pa = a; + const SortedPoint *pb = b; if (pa->p->y == pb->p->y) return 0; diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 390ad83497a..9749871b18e 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -402,8 +402,8 @@ count_distinct_groups(int numrows, SortItem *items, MultiSortSupport mss) static int compare_sort_item_count(const void *a, const void *b, void *arg) { - SortItem *ia = (SortItem *) a; - SortItem *ib = (SortItem *) b; + const SortItem *ia = a; + const SortItem *ib = b; if (ia->count == ib->count) return 0; @@ -465,8 +465,8 @@ static int sort_item_compare(const void *a, const void *b, void *arg) { SortSupport ssup = (SortSupport) arg; - SortItem *ia = (SortItem *) a; - SortItem *ib = (SortItem *) b; + const SortItem *ia = a; + const SortItem *ib = b; return ApplySortComparator(ia->values[0], ia->isnull[0], ib->values[0], ib->isnull[0], diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c index 2e96c86b0b3..e3436dbddd2 100644 --- a/src/backend/tsearch/spell.c +++ b/src/backend/tsearch/spell.c @@ -210,8 +210,8 @@ cmpspellaffix(const void *s1, const void *s2) static int cmpcmdflag(const void *f1, const void *f2) { - CompoundAffixFlag *fv1 = (CompoundAffixFlag *) f1, - *fv2 = (CompoundAffixFlag *) f2; + const CompoundAffixFlag *fv1 = f1; + const CompoundAffixFlag *fv2 = f2; Assert(fv1->flagMode == fv2->flagMode); diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 9c3806f5958..17d95caf164 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -189,7 +189,7 @@ injection_init_shmem(void) * otherwise. */ static bool -injection_point_allowed(InjectionPointCondition *condition) +injection_point_allowed(const InjectionPointCondition *condition) { bool result = true; @@ -232,7 +232,7 @@ injection_points_cleanup(int code, Datum arg) void injection_error(const char *name, const void *private_data, void *arg) { - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; char *argstr = (char *) arg; if (!injection_point_allowed(condition)) @@ -248,7 +248,7 @@ injection_error(const char *name, const void *private_data, void *arg) void injection_notice(const char *name, const void *private_data, void *arg) { - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; char *argstr = (char *) arg; if (!injection_point_allowed(condition)) @@ -268,7 +268,7 @@ injection_wait(const char *name, const void *private_data, void *arg) uint32 old_wait_counts = 0; int index = -1; uint32 injection_wait_event = 0; - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; if (inj_state == NULL) injection_init_shmem(); -- 2.34.1 --z8zicCKlggXCVSIM Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0002-Add-const-to-read-only-TableInfo-pointers-in-pg_d.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v2 1/3] Don't cast away const where possible @ 2025-12-18 09:51 Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 0 siblings, 0 replies; 14+ messages in thread From: Bertrand Drouvot @ 2025-12-18 09:51 UTC (permalink / raw) Add const to read only local variables, preserving the const qualifiers from the function signatures. This does not change all such instances, but only those hand-picked by the author. The ones that are not changed: - are just thin wrappers - would require public API changes - rely on external functions (such as LZ4F_compressUpdate()) - would require complex subsystem changes Discussion: https://postgr.es/m/aUQHy/MmWq7c97wK%40ip-10-97-1-34.eu-west-3.compute.internal --- src/backend/access/brin/brin_minmax_multi.c | 4 ++-- src/backend/access/heap/pruneheap.c | 4 ++-- src/backend/access/spgist/spgkdtreeproc.c | 8 ++++---- src/backend/statistics/mcv.c | 4 ++-- src/backend/tsearch/spell.c | 4 ++-- src/test/modules/injection_points/injection_points.c | 8 ++++---- 6 files changed, 16 insertions(+), 16 deletions(-) 10.2% src/backend/access/brin/ 12.5% src/backend/access/heap/ 18.7% src/backend/access/spgist/ 8.3% src/backend/statistics/ 11.6% src/backend/tsearch/ 38.4% src/test/modules/injection_points/ diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index 0298a9da8ba..0e87ce759f1 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid, static int compare_distances(const void *a, const void *b) { - DistanceValue *da = (DistanceValue *) a; - DistanceValue *db = (DistanceValue *) b; + const DistanceValue *da = a; + const DistanceValue *db = b; if (da->value < db->value) return 1; diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 07aa08cfe14..92aab1d6723 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -2021,8 +2021,8 @@ heap_log_freeze_eq(xlhp_freeze_plan *plan, HeapTupleFreeze *frz) static int heap_log_freeze_cmp(const void *arg1, const void *arg2) { - HeapTupleFreeze *frz1 = (HeapTupleFreeze *) arg1; - HeapTupleFreeze *frz2 = (HeapTupleFreeze *) arg2; + const HeapTupleFreeze *frz1 = arg1; + const HeapTupleFreeze *frz2 = arg2; if (frz1->xmax < frz2->xmax) return -1; diff --git a/src/backend/access/spgist/spgkdtreeproc.c b/src/backend/access/spgist/spgkdtreeproc.c index f0167d6ffa6..c02de835847 100644 --- a/src/backend/access/spgist/spgkdtreeproc.c +++ b/src/backend/access/spgist/spgkdtreeproc.c @@ -84,8 +84,8 @@ typedef struct SortedPoint static int x_cmp(const void *a, const void *b) { - SortedPoint *pa = (SortedPoint *) a; - SortedPoint *pb = (SortedPoint *) b; + const SortedPoint *pa = a; + const SortedPoint *pb = b; if (pa->p->x == pb->p->x) return 0; @@ -95,8 +95,8 @@ x_cmp(const void *a, const void *b) static int y_cmp(const void *a, const void *b) { - SortedPoint *pa = (SortedPoint *) a; - SortedPoint *pb = (SortedPoint *) b; + const SortedPoint *pa = a; + const SortedPoint *pb = b; if (pa->p->y == pb->p->y) return 0; diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index ec650ba029f..dc577b9a1e4 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -402,8 +402,8 @@ count_distinct_groups(int numrows, SortItem *items, MultiSortSupport mss) static int compare_sort_item_count(const void *a, const void *b, void *arg) { - SortItem *ia = (SortItem *) a; - SortItem *ib = (SortItem *) b; + const SortItem *ia = a; + const SortItem *ib = b; if (ia->count == ib->count) return 0; diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c index e5badb6b43f..3b0abfd9c35 100644 --- a/src/backend/tsearch/spell.c +++ b/src/backend/tsearch/spell.c @@ -210,8 +210,8 @@ cmpspellaffix(const void *s1, const void *s2) static int cmpcmdflag(const void *f1, const void *f2) { - CompoundAffixFlag *fv1 = (CompoundAffixFlag *) f1, - *fv2 = (CompoundAffixFlag *) f2; + const CompoundAffixFlag *fv1 = f1; + const CompoundAffixFlag *fv2 = f2; Assert(fv1->flagMode == fv2->flagMode); diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 25340e8d81b..12ea81a030c 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -189,7 +189,7 @@ injection_init_shmem(void) * otherwise. */ static bool -injection_point_allowed(InjectionPointCondition *condition) +injection_point_allowed(const InjectionPointCondition *condition) { bool result = true; @@ -232,7 +232,7 @@ injection_points_cleanup(int code, Datum arg) void injection_error(const char *name, const void *private_data, void *arg) { - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; char *argstr = (char *) arg; if (!injection_point_allowed(condition)) @@ -248,7 +248,7 @@ injection_error(const char *name, const void *private_data, void *arg) void injection_notice(const char *name, const void *private_data, void *arg) { - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; char *argstr = (char *) arg; if (!injection_point_allowed(condition)) @@ -268,7 +268,7 @@ injection_wait(const char *name, const void *private_data, void *arg) uint32 old_wait_counts = 0; int index = -1; uint32 injection_wait_event = 0; - InjectionPointCondition *condition = (InjectionPointCondition *) private_data; + const InjectionPointCondition *condition = private_data; if (inj_state == NULL) injection_init_shmem(); -- 2.34.1 --IBFQ0J+7bhRCocF6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0002-Add-const-to-read-only-TableInfo-pointers-in-pg_d.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v52 08/10] Introduce an option to make logical replication database specific. @ 2026-04-03 10:34 Antonin Houska <ah@cybertec.at> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-04-03 10:34 UTC (permalink / raw) By default, the logical decoding assumes access to shared catalogs, so the snapshot builder needs to consider cluster-wide XIDs during startup. That in turn means that, if any transaction is already running (and has XID assigned), the snapshot builder needs to wait for its completion, as it does not know if that transaction performed catalog changes earlier. Possible problem with this concept is that if REPACK (CONCURRENTLY) is running in some database, backends running the same command in other databases get stuck until the first one has committed. Thus only as single backend in the cluster can run REPACK (CONCURRENTLY) at any time. Likewise, REPACK (CONCURRENTLY) can block walsenders starting on behalf of subscriptions throughout the cluster. This patch introduces the possibility for a backend to declare that its output plugin does not use shared catalogs (i.e. catalogs that can be changed by transactions running in other databases in the cluster). In that case, no snapshot the backend will use during the decoding needs to contain information about transactions running in other databases. Thus the snapshot builder only needs to wait for completion of transactions in the current database. Currently we only use this option in the REPACK background worker. It could possibly be used in walsender involved in logical replication too, however that would need thorough analysis of its output plugin. The patch bumps WAL version number, due to a new field in xl_running_xacts. --- contrib/pg_visibility/pg_visibility.c | 4 ++-- src/backend/access/index/genam.c | 18 ++++++++++++++++ src/backend/access/rmgrdesc/standbydesc.c | 2 ++ src/backend/access/transam/xlog.c | 2 +- src/backend/access/transam/xlogfuncs.c | 2 +- src/backend/commands/repack_worker.c | 8 +++++++ src/backend/postmaster/bgwriter.c | 2 +- src/backend/replication/logical/snapbuild.c | 10 ++++++++- src/backend/replication/slot.c | 12 +++++++++-- src/backend/storage/ipc/procarray.c | 23 ++++++++++++++++++++- src/backend/storage/ipc/standby.c | 14 +++++++++++-- src/include/access/genam.h | 8 +++++++ src/include/access/xlog_internal.h | 2 +- src/include/storage/procarray.h | 2 +- src/include/storage/standby.h | 3 ++- src/include/storage/standbydefs.h | 1 + 16 files changed, 99 insertions(+), 14 deletions(-) diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index dfab0b64cf5..d564bd2a00c 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -621,7 +621,7 @@ GetStrictOldestNonRemovableTransactionId(Relation rel) else if (rel == NULL || rel->rd_rel->relisshared) { /* Shared relation: take into account all running xids */ - runningTransactions = GetRunningTransactionData(); + runningTransactions = GetRunningTransactionData(InvalidOid); LWLockRelease(ProcArrayLock); LWLockRelease(XidGenLock); return runningTransactions->oldestRunningXid; @@ -632,7 +632,7 @@ GetStrictOldestNonRemovableTransactionId(Relation rel) * Normal relation: take into account xids running within the current * database */ - runningTransactions = GetRunningTransactionData(); + runningTransactions = GetRunningTransactionData(InvalidOid); LWLockRelease(ProcArrayLock); LWLockRelease(XidGenLock); return runningTransactions->oldestDatabaseRunningXid; diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 1408989c568..df092dc999a 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -37,6 +37,14 @@ #include "utils/ruleutils.h" #include "utils/snapmgr.h" +/* + * If a backend is going to do logical decoding and if the output plugin does + * not need to access shared catalogs, setting this variable to false can make + * the decoding startup faster. In particular, the backend will not need to + * wait for completion of already running transactions in other databases. + */ +bool accessSharedCatalogsInDecoding = true; + /* ---------------------------------------------------------------- * general access method routines @@ -394,6 +402,16 @@ systable_beginscan(Relation heapRelation, SysScanDesc sysscan; Relation irel; + /* + * If this backend promised that it won't access shared catalogs during + * logical decoding, this seems to be the right place to check. + * + * XXX Should this be ereport(ERROR) ? + */ + Assert(!HistoricSnapshotActive() || + accessSharedCatalogsInDecoding || + !heapRelation->rd_rel->relisshared); + if (indexOK && !IgnoreSystemIndexes && !ReindexIsProcessingIndex(indexId)) diff --git a/src/backend/access/rmgrdesc/standbydesc.c b/src/backend/access/rmgrdesc/standbydesc.c index 0a291354ae2..685d1bdb024 100644 --- a/src/backend/access/rmgrdesc/standbydesc.c +++ b/src/backend/access/rmgrdesc/standbydesc.c @@ -41,6 +41,8 @@ standby_desc_running_xacts(StringInfo buf, xl_running_xacts *xlrec) for (i = 0; i < xlrec->subxcnt; i++) appendStringInfo(buf, " %u", xlrec->xids[xlrec->xcnt + i]); } + + appendStringInfo(buf, "; dbid: %u", xlrec->dbid); } void diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 2c1c6f88b74..8da8eff93e4 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7336,7 +7336,7 @@ CreateCheckPoint(int flags) * recovery we don't need to write running xact data. */ if (!shutdown && XLogStandbyInfoActive()) - LogStandbySnapshot(); + LogStandbySnapshot(InvalidOid); START_CRIT_SECTION(); diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 65bbaeda59c..0f5979691e6 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -245,7 +245,7 @@ pg_log_standby_snapshot(PG_FUNCTION_ARGS) (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("pg_log_standby_snapshot() can only be used if \"wal_level\" >= \"replica\""))); - recptr = LogStandbySnapshot(); + recptr = LogStandbySnapshot(InvalidOid); /* * As a convenience, return the WAL location of the last inserted record diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index c85166ba849..ff34e246469 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/genam.h" #include "access/table.h" #include "access/xlog_internal.h" #include "access/xlogutils.h" @@ -237,6 +238,13 @@ repack_setup_logical_decoding(Oid relid) EnsureLogicalDecodingEnabled(); + /* + * By declaring that our output plugin does not need shared catalogs, we + * avoid waiting for completion of transactions running in other databases + * than the one we're connected to. + */ + accessSharedCatalogsInDecoding = false; + /* * Neither prepare_write nor do_write callback nor update_progress is * useful for us. diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 1d8947774a9..a30de4262eb 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -289,7 +289,7 @@ BackgroundWriterMain(const void *startup_data, size_t startup_data_len) if (now >= timeout && last_snapshot_lsn <= GetLastImportantRecPtr()) { - last_snapshot_lsn = LogStandbySnapshot(); + last_snapshot_lsn = LogStandbySnapshot(InvalidOid); last_snapshot_ts = now; } } diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index b4269a3b102..2e3926e1d13 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -125,6 +125,7 @@ #include <sys/stat.h> #include <unistd.h> +#include "access/genam.h" #include "access/heapam_xlog.h" #include "access/transam.h" #include "access/xact.h" @@ -1465,7 +1466,14 @@ SnapBuildWaitSnapshot(xl_running_xacts *running, TransactionId cutoff) */ if (!RecoveryInProgress()) { - LogStandbySnapshot(); + Oid dbid; + + /* + * Only consider transactions of the current database if our plugin is + * not supposed to access shared catalogs. + */ + dbid = accessSharedCatalogsInDecoding ? InvalidOid : MyDatabaseId; + LogStandbySnapshot(dbid); } } diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index a9092fc2382..d553bd5dbff 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -39,6 +39,7 @@ #include <unistd.h> #include <sys/stat.h> +#include "access/genam.h" #include "access/transam.h" #include "access/xlog_internal.h" #include "access/xlogrecovery.h" @@ -1760,9 +1761,16 @@ ReplicationSlotReserveWal(void) if (!RecoveryInProgress() && SlotIsLogical(slot)) { XLogRecPtr flushptr; + Oid dbid; - /* make sure we have enough information to start */ - flushptr = LogStandbySnapshot(); + /* + * Make sure we have enough information to start. + * + * Only consider transactions of the current database if our plugin is + * not supposed to access shared catalogs. + */ + dbid = accessSharedCatalogsInDecoding ? InvalidOid : MyDatabaseId; + flushptr = LogStandbySnapshot(dbid); /* and make sure it's fsynced to disk */ XLogFlush(flushptr); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index cc207cb56e3..92ed34128e5 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2631,9 +2631,11 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) * * Note that if any transaction has overflowed its cached subtransactions * then there is no real need include any subtransactions. + * + * If 'dbid' is valid, only gather transactions running in that database. */ RunningTransactions -GetRunningTransactionData(void) +GetRunningTransactionData(Oid dbid) { /* result workspace */ static RunningTransactionsData CurrentRunningXactsData; @@ -2708,6 +2710,18 @@ GetRunningTransactionData(void) if (!TransactionIdIsValid(xid)) continue; + /* + * Filter by database OID if requested. + */ + if (OidIsValid(dbid)) + { + int pgprocno = arrayP->pgprocnos[index]; + PGPROC *proc = &allProcs[pgprocno]; + + if (proc->databaseId != dbid) + continue; + } + /* * Be careful not to exclude any xids before calculating the values of * oldestRunningXid and suboverflowed, since these are used to clean @@ -2758,6 +2772,12 @@ GetRunningTransactionData(void) PGPROC *proc = &allProcs[pgprocno]; int nsubxids; + /* + * Filter by database OID if requested. + */ + if (OidIsValid(dbid) && proc->databaseId != dbid) + continue; + /* * Save subtransaction XIDs. Other backends can't add or remove * entries while we're holding XidGenLock. @@ -2791,6 +2811,7 @@ GetRunningTransactionData(void) * increases if slots do. */ + CurrentRunningXacts->dbid = dbid; CurrentRunningXacts->xcnt = count - subcount; CurrentRunningXacts->subxcnt = subcount; CurrentRunningXacts->subxid_status = suboverflowed ? SUBXIDS_IN_SUBTRANS : SUBXIDS_IN_ARRAY; diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index de9092fdf5b..c653ea742bc 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -1188,6 +1188,14 @@ standby_redo(XLogReaderState *record) xl_running_xacts *xlrec = (xl_running_xacts *) XLogRecGetData(record); RunningTransactionsData running; + /* + * Records issued for specific database are not suitable for physical + * replication because that affects the whole cluster. In particular, + * the list of XID is probably incomplete here. + */ + if (OidIsValid(xlrec->dbid)) + return; + running.xcnt = xlrec->xcnt; running.subxcnt = xlrec->subxcnt; running.subxid_status = xlrec->subxid_overflow ? SUBXIDS_MISSING : SUBXIDS_IN_ARRAY; @@ -1277,11 +1285,12 @@ standby_redo(XLogReaderState *record) * as there's no independent knob to just enable logical decoding. For * details of how this is used, check snapbuild.c's introductory comment. * + * If 'dbid' is valid, only gather transactions running in that database. * * Returns the RecPtr of the last inserted record. */ XLogRecPtr -LogStandbySnapshot(void) +LogStandbySnapshot(Oid dbid) { XLogRecPtr recptr; RunningTransactions running; @@ -1314,7 +1323,7 @@ LogStandbySnapshot(void) * Log details of all in-progress transactions. This should be the last * record we write, because standby will open up when it sees this. */ - running = GetRunningTransactionData(); + running = GetRunningTransactionData(dbid); /* * GetRunningTransactionData() acquired ProcArrayLock, we must release it. @@ -1358,6 +1367,7 @@ LogCurrentRunningXacts(RunningTransactions CurrRunningXacts) xl_running_xacts xlrec; XLogRecPtr recptr; + xlrec.dbid = CurrRunningXacts->dbid; xlrec.xcnt = CurrRunningXacts->xcnt; xlrec.subxcnt = CurrRunningXacts->subxcnt; xlrec.subxid_overflow = (CurrRunningXacts->subxid_status != SUBXIDS_IN_ARRAY); diff --git a/src/include/access/genam.h b/src/include/access/genam.h index b69320a7fc8..56c573399ab 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -136,6 +136,14 @@ typedef struct IndexOrderByDistance bool isnull; } IndexOrderByDistance; +/* + * Is the backend interested in shared catalogs when performing logical + * decoding? + * + * XXX Is there a better place for this declaration? + */ +extern bool accessSharedCatalogsInDecoding; + /* * generalized index_ interface routines (in indexam.c) */ diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index 755835d63bf..ae19982d88d 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -31,7 +31,7 @@ /* * Each page of XLOG file has a header like this: */ -#define XLOG_PAGE_MAGIC 0xD11E /* can be used as WAL version indicator */ +#define XLOG_PAGE_MAGIC 0xD11F /* can be used as WAL version indicator */ typedef struct XLogPageHeaderData { diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index abdf021e66e..377b3060b9f 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -49,7 +49,7 @@ extern bool ProcArrayInstallImportedXmin(TransactionId xmin, VirtualTransactionId *sourcevxid); extern bool ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc); -extern RunningTransactions GetRunningTransactionData(void); +extern RunningTransactions GetRunningTransactionData(Oid dbid); extern bool TransactionIdIsInProgress(TransactionId xid); extern TransactionId GetOldestNonRemovableTransactionId(Relation rel); diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h index 6a314c693cd..8715c08e94f 100644 --- a/src/include/storage/standby.h +++ b/src/include/storage/standby.h @@ -126,6 +126,7 @@ typedef enum typedef struct RunningTransactionsData { + Oid dbid; /* only track xacts in this database */ int xcnt; /* # of xact ids in xids[] */ int subxcnt; /* # of subxact ids in xids[] */ subxids_array_status subxid_status; @@ -143,7 +144,7 @@ typedef RunningTransactionsData *RunningTransactions; extern void LogAccessExclusiveLock(Oid dbOid, Oid relOid); extern void LogAccessExclusiveLockPrepare(void); -extern XLogRecPtr LogStandbySnapshot(void); +extern XLogRecPtr LogStandbySnapshot(Oid dbid); extern void LogStandbyInvalidations(int nmsgs, SharedInvalidationMessage *msgs, bool relcacheInitFileInval); diff --git a/src/include/storage/standbydefs.h b/src/include/storage/standbydefs.h index 231d251fd51..e75b7078766 100644 --- a/src/include/storage/standbydefs.h +++ b/src/include/storage/standbydefs.h @@ -46,6 +46,7 @@ typedef struct xl_standby_locks */ typedef struct xl_running_xacts { + Oid dbid; /* only track xacts in this database */ int xcnt; /* # of xact ids in xids[] */ int subxcnt; /* # of subxact ids in xids[] */ bool subxid_overflow; /* snapshot overflowed, subxids missing */ -- 2.47.3 --gp2pyozrd5pweboh Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v52-0009-Reserve-replication-slots-specifically-for-REPAC.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-04-03 10:34 UTC | newest] Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-07-08 22:13 [PATCH 2/4] fix costing in cost_incremental_sort Tomas Vondra <tomas@2ndquadrant.com> 2019-07-08 22:13 [PATCH 2/4] fix costing in cost_incremental_sort Tomas Vondra <tomas@2ndquadrant.com> 2023-02-03 05:51 [PATCH v1 2/9] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 1/2] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 1/2] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 07/10] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 1/2] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 07/10] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v3 1/2] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v1 2/9] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2023-02-03 05:51 [PATCH v1 2/9] Use "template" initdb in tests Andres Freund <andres@anarazel.de> 2025-12-18 09:51 [PATCH v3 1/2] Don't cast away const where possible Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 2025-12-18 09:51 [PATCH v2 1/3] Don't cast away const where possible Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 2026-04-03 10:34 [PATCH v52 08/10] Introduce an option to make logical replication database specific. Antonin Houska <ah@cybertec.at>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox