Reduce time taken by 002_pg_upgrade test to run

This test is one of the longest running tests and
172259a 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>

Author: ashutosh-bapat

src/bin/pg_upgrade/t/002_pg_upgrade.pl

@@ -359,58 +359,53 @@ sub get_dump_for_comparison
 $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
 	# differ because of locale changes. Additionally this provides test
 	# 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 @@ sub get_dump_for_comparison
 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();