From c0bb2eab8cba8910d7c50df3c29c146679a1ae6c Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Mon, 24 Mar 2025 11:21:12 +0530 Subject: [PATCH v2j 2/3] Use only one format and make the test run default According to Alvaro (and I agree with him), the test should be run by default. Otherwise we get to know about a bug only after buildfarm animal where it's enabled reports a failure. Further testing only one format may suffice; since all the formats have shown the same bugs till now. If we use --directory format we can use -j which reduces the time taken by dump/restore test by about 12%. This patch removes PG_TEST_EXTRA option as well as runs the test only in directory format with parallelism enabled. Note for committer: If we decide to accept this change, it should be merged with the previous commit. --- doc/src/sgml/regress.sgml | 12 ---- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 76 +++++++++----------------- 2 files changed, 25 insertions(+), 63 deletions(-) diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 237b974b3ab..0e5e8e8f309 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -357,18 +357,6 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption' - - - regress_dump_test - - - When enabled, src/bin/pg_upgrade/t/002_pg_upgrade.pl - tests dump and restore of regression database left behind by the - regression run. Not enabled by default because it is time and resource - consuming. - - - Tests for features that are not supported by the current build diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index d08eea6693f..cbd9831bf9e 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -268,16 +268,9 @@ else # should be done in a separate TAP test, but doing it here saves us one full # regression run. # - # This step takes several extra seconds and some extra disk space, so - # requires an opt-in with the PG_TEST_EXTRA environment variable. - # # Do this while the old cluster is running before it is shut down by the # upgrade test. - if ( $ENV{PG_TEST_EXTRA} - && $ENV{PG_TEST_EXTRA} =~ /\bregress_dump_test\b/) - { - test_regression_dump_restore($oldnode, %node_params); - } + test_regression_dump_restore($oldnode, %node_params); } # Initialize a new node for the upgrade. @@ -590,53 +583,34 @@ sub test_regression_dump_restore $dst_node->append_conf('postgresql.conf', 'autovacuum = off'); $dst_node->start; - # Test all formats one by one. - for my $format ('plain', 'tar', 'directory', 'custom') - { - my $dump_file = "$tempdir/regression_dump.$format"; - my $restored_db = 'regression_' . $format; - - # Use --create in dump and restore commands so that the restored - # database has the same configurable variable settings as the original - # database and the plain dumps taken for comparsion do not differ - # because of locale changes. Additionally this provides test coverage - # for --create option. - $src_node->command_ok( - [ - 'pg_dump', "-F$format", '--no-sync', - '-d', $src_node->connstr('regression'), - '--create', '-f', $dump_file - ], - "pg_dump on source instance in $format format"); + my $dump_file = "$tempdir/regression.dump"; - my @restore_command; - if ($format eq 'plain') - { - # Restore dump in "plain" format using `psql`. - @restore_command = [ 'psql', '-d', 'postgres', '-f', $dump_file ]; - } - else - { - @restore_command = [ - 'pg_restore', '--create', - '-d', 'postgres', $dump_file - ]; - } - $dst_node->command_ok(@restore_command, - "restored dump taken in $format format on destination instance"); + # Use --create in dump and restore commands so that the restored database + # has the same configurable variable settings as the original database so + # that the plain dumps taken from both the database taken for comparisong do + # not differ because of locale changes. Additionally this provides test + # coverage for --create option. + # + # We use directory format which allows dumping and restoring in parallel to + # reduce the test's run time. + $src_node->command_ok( + [ + 'pg_dump', '-Fd', '-j2', '--no-sync', + '-d', $src_node->connstr('regression'), + '--create', '-f', $dump_file + ], + "pg_dump on source instance succeeded"); - my $dst_dump = - get_dump_for_comparison($dst_node, 'regression', - 'dest_dump.' . $format, 0); + $dst_node->command_ok( + [ 'pg_restore', '--create', '-j2', '-d', 'postgres', $dump_file ], + "restored dump to destination instance"); - compare_files($src_dump, $dst_dump, - "dump outputs from original and restored regression database (using $format format) match" - ); + my $dst_dump = get_dump_for_comparison($dst_node, 'regression', + 'dest_dump', 0); - # Rename the restored database so that it is available for debugging in - # case the test fails. - $dst_node->safe_psql('postgres', "ALTER DATABASE regression RENAME TO $restored_db"); - } + compare_files($src_dump, $dst_dump, + "dump outputs from original and restored regression database match" + ); } # Dump database `db` from the given `node` in plain format and adjust it for -- 2.34.1