From 47ae7fd5ed7ef5104123c083a400f44fd5a13906 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 20 Jul 2026 21:54:19 +0200 Subject: [PATCH v1] Keep using the initdb template when PG_TEST_INITDB_EXTRA_OPTS is set Setting PG_TEST_INITDB_EXTRA_OPTS used to disable the initdb template mechanism entirely: both Cluster.pm` and `pg_regress.c fell back to running a full initdb for every test cluster, since they couldn't know whether the template was compatible with the requested options. Because CI sets this variable in most jobs, nearly every test paid the full initdb cost, which is a major contributor to slow CI runs (initdb is roughly 3x the CPU of copying the template, and worse under sanitizers). This fixes this by using `PG_TEST_INITDB_EXTRA_OPTS` too when creating the initdb template and storing the options that were used for the template in a '{}.extra-opts' file. That way the runners can compare the options in this file to their PG_TEST_INITDB_EXTRA_OPTS value. If it's the same they can use the template, if not they need to run initdb. --- meson.build | 23 +++++++++++- src/Makefile.global.in | 12 +++++- src/test/perl/PostgreSQL/Test/Cluster.pm | 42 +++++++++++++++------ src/test/regress/pg_regress.c | 47 +++++++++++++++++++++++- 4 files changed, 108 insertions(+), 16 deletions(-) diff --git a/meson.build b/meson.build index f4cde249242..74bf7b9f714 100644 --- a/meson.build +++ b/meson.build @@ -3901,18 +3901,37 @@ endif # Create (and remove old) initdb template directory. Tests use that, where # possible, to make it cheaper to run tests. # +# The template is created with PG_TEST_INITDB_EXTRA_OPTS applied, and the +# variable's value is recorded in an ".extra-opts" file next to the template +# directory. That allows tests to use the template when the variable is set, +# as long as its value still matches, instead of falling back to a full +# initdb. +# # 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 os +import shlex import shutil import sys import subprocess -shutil.rmtree(sys.argv[1], ignore_errors=True) -sp = subprocess.run(sys.argv[2:] + [sys.argv[1]]) +template_dir = sys.argv[1] +extra_opts = os.environ.get('PG_TEST_INITDB_EXTRA_OPTS', '') + +shutil.rmtree(template_dir, ignore_errors=True) +try: + os.remove(template_dir + '.extra-opts') +except FileNotFoundError: + pass + +sp = subprocess.run(sys.argv[2:] + shlex.split(extra_opts) + [template_dir]) +if sp.returncode == 0: + with open(template_dir + '.extra-opts', 'w') as f: + f.write(extra_opts) sys.exit(sp.returncode) ''', test_initdb_template, diff --git a/src/Makefile.global.in b/src/Makefile.global.in index cef1ad7f87d..ea0b218717a 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -442,7 +442,17 @@ ifeq ($(MAKELEVEL),0) $(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 --auth trust --no-sync --no-instructions --lc-messages=C --no-clean '$(abs_top_builddir)'/tmp_install/initdb-template >>'$(abs_top_builddir)'/tmp_install/log/initdb-template.log 2>&1 +# The initdb template is created with PG_TEST_INITDB_EXTRA_OPTS applied, and +# the variable's value is recorded in an ".extra-opts" file next to the +# template directory. That allows tests to use the template when the variable +# is set, as long as its value still matches. Run initdb through an extra +# shell so that any quoting in PG_TEST_INITDB_EXTRA_OPTS is interpreted the +# same way as in pg_regress.c and Cluster.pm. The template path is passed as +# a positional argument ($0) instead of being embedded in the command string, +# so that only PG_TEST_INITDB_EXTRA_OPTS gets that second round of shell +# interpretation. + $(with_temp_install) sh -c "initdb --auth trust --no-sync --no-instructions --lc-messages=C --no-clean $$PG_TEST_INITDB_EXTRA_OPTS \"\$$0\"" '$(abs_top_builddir)'/tmp_install/initdb-template >>'$(abs_top_builddir)'/tmp_install/log/initdb-template.log 2>&1 + printf '%s' "$$PG_TEST_INITDB_EXTRA_OPTS" >'$(abs_top_builddir)'/tmp_install/initdb-template.extra-opts endif endif endif diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 3eae4cf6281..8b1c3d73c39 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -639,38 +639,58 @@ sub init $params{has_archiving} = 0 unless defined $params{has_archiving}; my $initdb_extra_opts_env = $ENV{PG_TEST_INITDB_EXTRA_OPTS}; + + my @initdb_opts = @{ $params{extra} // [] }; if (defined $initdb_extra_opts_env) { - push @{ $params{extra} }, shellwords($initdb_extra_opts_env); + push @initdb_opts, shellwords($initdb_extra_opts_env); } # This should override user-supplied initdb options. if ($params{no_data_checksums}) { - push @{ $params{extra} }, '--no-data-checksums'; + push @initdb_opts, '--no-data-checksums'; } mkdir $self->backup_dir; mkdir $self->archive_dir; - # If available, if there aren't any parameters and if force_initdb is - # disabled, use a previously initdb'd cluster as a template by copying it. - # For a lot of tests, that's substantially cheaper. It does not seem - # worth figuring out whether extra parameters affect compatibility, so - # initdb is forced if any are defined. + # If available, if there aren't any parameters the template wasn't + # created with and if force_initdb is disabled, use a previously initdb'd + # cluster as a template by copying it. For a lot of tests, that's + # substantially cheaper. + # + # The value of PG_TEST_INITDB_EXTRA_OPTS used when creating the template + # is recorded in an ".extra-opts" file next to the template directory, so + # the template stays usable when that variable is set, as long as its + # value matches. It does not seem worth figuring out whether + # test-supplied extra parameters affect compatibility, so initdb is + # forced if any are defined. # # There's very similar code in pg_regress.c, but we can't easily # deduplicate it until we require perl at build time. - if ( $params{force_initdb} - or defined $params{extra} - or !defined $ENV{INITDB_TEMPLATE}) + my $template_usable = 0; + if ( !$params{force_initdb} + and !defined $params{extra} + and !$params{no_data_checksums} + and defined $ENV{INITDB_TEMPLATE}) + { + my $extra_opts_file = $ENV{INITDB_TEMPLATE} . '.extra-opts'; + my $template_opts = + -e $extra_opts_file + ? PostgreSQL::Test::Utils::slurp_file($extra_opts_file) + : ''; + $template_usable = $template_opts eq ($initdb_extra_opts_env // ''); + } + + if (!$template_usable) { note("initializing database system by running initdb"); PostgreSQL::Test::Utils::system_or_bail( 'initdb', '--no-sync', '--pgdata' => $pgdata, '--auth' => 'trust', - @{ $params{extra} }); + @initdb_opts); } else { diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 21d00f792d2..d7adc211cc0 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2138,6 +2138,45 @@ help(void) printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL); } +/* + * Check whether the initdb template was created with the same + * PG_TEST_INITDB_EXTRA_OPTS that are in effect now. The value used during + * template creation is recorded in an ".extra-opts" file next to the + * template directory; a missing file means the template was created without + * extra options. Without this check we'd have to fall back to a full initdb + * whenever PG_TEST_INITDB_EXTRA_OPTS is set, which is a lot more expensive. + */ +static bool +initdb_template_matches(const char *initdb_template_dir, + const char *initdb_extra_opts_env) +{ + char path[MAXPGPATH]; + char recorded[4096]; + FILE *f; + + snprintf(path, sizeof(path), "%s.extra-opts", initdb_template_dir); + + recorded[0] = '\0'; + if ((f = fopen(path, "r")) != NULL) + { + /* + * A value too long for the buffer can never compare equal, so treat + * truncation as a mismatch instead of comparing just the prefix. + */ + size_t nread = fread(recorded, 1, sizeof(recorded) - 1, f); + bool truncated = (nread == sizeof(recorded) - 1 && !feof(f)); + + recorded[nread] = '\0'; + fclose(f); + + if (truncated) + return false; + } + + return strcmp(recorded, + initdb_extra_opts_env ? initdb_extra_opts_env : "") == 0; +} + int regression_main(int argc, char *argv[], init_function ifunc, @@ -2401,13 +2440,17 @@ regression_main(int argc, char *argv[], * 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. + * copying it. For a lot of tests, that's substantially cheaper. The + * template is only usable if it was created with the same + * PG_TEST_INITDB_EXTRA_OPTS that are in effect now, which + * initdb_template_matches() checks. * * 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 || initdb_extra_opts_env) + if (initdb_template_dir == NULL || nolocale || debug || + !initdb_template_matches(initdb_template_dir, initdb_extra_opts_env)) { note("initializing database system by running initdb"); -- 2.54.0