0001-Reduce-time-taken-by-002_pg_upgrade-test-to-20250404.patch

text/x-patch

Filename: 0001-Reduce-time-taken-by-002_pg_upgrade-test-to-20250404.patch
Type: text/x-patch
Part: 0
Message: Re: Test to dump and restore objects left behind by regression

Patch

Format: format-patch
Series: patch 0001
Subject: Reduce time taken by 002_pg_upgrade test to run
File+
src/bin/pg_upgrade/t/002_pg_upgrade.pl 46 26
From ef7880e1f1774c6875f4b265287f70d844e20ca4 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Fri, 4 Apr 2025 14:26:48 +0530
Subject: [PATCH 1/2] Reduce time taken by 002_pg_upgrade test to run

This test is one of the longest running tests and
172259afb563d35001410dc6daad78b250924038 makes it even longer by adding a test
to dump/restore roundtrip of regression database.

The test already creates two clusters for pg_upgrade test. When these clusters
have same version and do not use custom installation, we run dump/restore test.
The new upgraded cluster could be used as target of restore instead of creating
a new cluster.

But this separates the dump and restore phases of the test spatially and
temporaly since the dump needs to be taken while the old cluster is running and
it can be restored only after new upgraded cluster is running; in-between we run
upgrade test. This separation affects readability of the test and hence wasn't
attempted before. But since runtime of test seems to be more important, we take
a hit on readability. We have added additional comments so as to link the two
phases and improve readability.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reported-by: Andres Freund <andres@anarazel.de>
---
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 72 ++++++++++++++++----------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 311391d7acd..8e7ccd2dad0 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -359,30 +359,34 @@ if (defined($ENV{oldinstall}))
 $oldnode->append_conf('postgresql.conf', 'autovacuum = off');
 $oldnode->restart;
 
+# Dump/restore Test.
+#
 # Test that dump/restore of the regression database roundtrips cleanly.  This
 # doesn't work well when the nodes are different versions, so skip it in that
 # case.  Note that this isn't a pg_upgrade test, but it's convenient to do it
 # here because we've gone to the trouble of creating the regression database.
 #
-# Do this while the old cluster is running before it is shut down by the
-# upgrade test but after turning its autovacuum off for stable statistics.
+# We execute this in two parts as follows:
+#
+# Part 1: Take dump from the old cluster while it is running before being shut
+# down by the upgrade test but after turning its autovacuum off for stable
+# statistics. If this part succeeds and is not skipped, it will leave behind
+# dump to be restored and a dump file for comparison.
+#
+# Part 2: The dump is restored on the upgraded cluster once it is running.
+#
+# Though this separates the two parts spatially and temporally, it avoids
+# creating a new cluster, thus saving time (and resources) in this already long
+# running test.
+my $regress_dump_file;
+my $src_dump;
 SKIP:
 {
-	my $dstnode = PostgreSQL::Test::Cluster->new('dst_node');
-
 	skip "different Postgres versions"
-	  if ($oldnode->pg_version != $dstnode->pg_version);
+	  if ($oldnode->pg_version != $newnode->pg_version);
 	skip "source node not using default install"
 	  if (defined $oldnode->install_path);
 
-	# Setup destination database cluster with the same configuration as the
-	# source cluster to avoid any differences between dumps taken from both the
-	# clusters caused by differences in their configurations.
-	$dstnode->init(%old_node_params);
-	# Stabilize stats for comparison.
-	$dstnode->append_conf('postgresql.conf', 'autovacuum = off');
-	$dstnode->start;
-
 	# 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 dumps taken from both databases taken do not
@@ -390,27 +394,18 @@ SKIP:
 	# coverage for --create option.
 	#
 	# Use directory format so that we can use parallel dump/restore.
-	my $dump_file = "$tempdir/regression.dump";
+	$regress_dump_file = "$tempdir/regression.dump";
 	$oldnode->command_ok(
 		[
 			'pg_dump', '-Fd', '-j2', '--no-sync',
 			'-d' => $oldnode->connstr('regression'),
-			'--create', '-f' => $dump_file
+			'--create', '-f' => $regress_dump_file
 		],
 		'pg_dump on source instance');
 
-	$dstnode->command_ok(
-		[ 'pg_restore', '--create', '-j2', '-d' => 'postgres', $dump_file ],
-		'pg_restore to destination instance');
-
-	# Dump original and restored database for comparison.
-	my $src_dump =
+	# Dump original database for comparison.
+	$src_dump =
 	  get_dump_for_comparison($oldnode, 'regression', 'src_dump', 1);
-	my $dst_dump =
-	  get_dump_for_comparison($dstnode, 'regression', 'dest_dump', 0);
-
-	compare_files($src_dump, $dst_dump,
-		'dump outputs from original and restored regression databases match');
 }
 
 # Take a dump before performing the upgrade as a base comparison. Note
@@ -629,4 +624,29 @@ my $dump2_filtered = filter_dump(0, $oldnode->pg_version, $dump2_file);
 compare_files($dump1_filtered, $dump2_filtered,
 	'old and new dumps match after pg_upgrade');
 
+# Execute Part 2 of Dump/restore Test.
+SKIP:
+{
+	# Skip Part 2 if the dump to be restored and the dump file for comparison do
+	# not exist.  Part 1 was not executed or did not succeed.
+	skip "no dump/restore test"
+	  if not defined $regress_dump_file or not defined $src_dump;
+
+	# Use --create option as explained in Part 1. Rename upgraded regression
+	# database so that pg_restore can succeed and the so that it's available for
+	# diagnosing problems if any.
+	$newnode->safe_psql('postgres', 'ALTER DATABASE regression RENAME TO
+	regression_upgraded');
+	$newnode->command_ok(
+		[ 'pg_restore', '--create', '-j2', '-d' => 'postgres', $regress_dump_file ],
+		'pg_restore to destination instance');
+
+	# Dump restored database for comparison.
+	my $dst_dump =
+	  get_dump_for_comparison($newnode, 'regression', 'dest_dump', 0);
+
+	compare_files($src_dump, $dst_dump,
+		'dump outputs from original and restored regression databases match');
+}
+
 done_testing();

base-commit: 7afca7edef751b8d7c0f5b6402ffcefc11c67fdd
-- 
2.34.1