Date: Tue, 18 Mar 2025 14:08:42 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>,
	Greg Sabino Mullane <htamfids@gmail.com>,
	pgsql-hackers@postgresql.org, bruce@momjian.us
Subject: Re: optimize file transfer in pg_upgrade
Message-ID: <Z9nEup6k56N_9l0X@nathan>
References: <Z9h5Spp76EBygyEL@nathan>
 <CA+TgmoZmffymTYV52+a6QFYMAnLe-v4U_P-yqwxqJZ38Hc_Qww@mail.gmail.com>
 <Z9iGZ14YlMGZwBnw@nathan>
 <CA+TgmoahvU4XX+BWDCYnDD29Qpkd3TJydUJtNc7krLmhSXcgpA@mail.gmail.com>
 <17820.1742306681@sss.pgh.pa.us>
 <egjrbbaielccjecm6js5isonhm64zhcqtsoaynbopncreidtes@y3ysztu2d5v4>
 <Z9mtXsBs-31E5ksW@nathan>
 <tlkgkf3woe4ye2l4m4dnwui4wmqtajotozw5r3fhefyuv6hwrl@34xroqbvddpj>
 <Z9mxlf5uVuk2DziP@nathan>
 <7rhxl5nl5cuenm4kzprjhaojo74m2teuwzjffcrdi6bxx6b2wn@orb5h3mteh2y>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="RVUTIYpSRw61lFha"
Content-Disposition: inline
In-Reply-To: <7rhxl5nl5cuenm4kzprjhaojo74m2teuwzjffcrdi6bxx6b2wn@orb5h3mteh2y>
Archived-At: <https://www.postgresql.org/message-id/Z9nEup6k56N_9l0X%40nathan>
Precedence: bulk


--RVUTIYpSRw61lFha
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Mar 18, 2025 at 01:50:10PM -0400, Andres Freund wrote:
> On 2025-03-18 12:47:01 -0500, Nathan Bossart wrote:
>> On Tue, Mar 18, 2025 at 01:37:02PM -0400, Andres Freund wrote:
>> > - Do we need a new old cluster for each of the modes? That seems like wasted
>> >   time?  I guess it's required for --link...
>> 
>> It'll also be needed for --swap.  We could optionally save the old cluster
>> for a couple of modes if we really wanted to.  *shrug*
> 
> Don't worry about it, I think the template initdb stuff should make it cheap enough...

Cool.  I realize now why there's poor coverage for pg_upgrade with
tablespaces: you can't upgrade between the same version with tablespaces
(presumably due to the version-specific subdirectory conflict).  I don't
know if the regression tests leave around any tablespaces for the
cross-version pg_upgrade tests, but that's probably the best we can do at
the moment.

For now, here's a new version of the test with a rewritten table.  I also
tried to fix the expected error regex to handle some of the other error
messages for unsupported modes (as revealed by cfbot).

-- 
nathan

--RVUTIYpSRw61lFha
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-Add-test-for-pg_upgrade-file-transfer-modes.patch"

From c4b7816955cfb5d331851e14f9a93cbb182f4d1e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 18 Mar 2025 12:21:03 -0500
Subject: [PATCH v2 1/1] Add test for pg_upgrade file transfer modes.

---
 src/bin/pg_upgrade/meson.build           |  1 +
 src/bin/pg_upgrade/t/006_modes.pl        | 67 ++++++++++++++++++++++++
 src/test/perl/PostgreSQL/Test/Cluster.pm | 19 +++++++
 src/test/perl/PostgreSQL/Test/Utils.pm   | 25 +++++++++
 4 files changed, 112 insertions(+)
 create mode 100644 src/bin/pg_upgrade/t/006_modes.pl

diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build
index da84344966a..16cd9247e76 100644
--- a/src/bin/pg_upgrade/meson.build
+++ b/src/bin/pg_upgrade/meson.build
@@ -46,6 +46,7 @@ tests += {
       't/003_logical_slots.pl',
       't/004_subscription.pl',
       't/005_char_signedness.pl',
+      't/006_modes.pl',
     ],
     'test_kwargs': {'priority': 40}, # pg_upgrade tests are slow
   },
diff --git a/src/bin/pg_upgrade/t/006_modes.pl b/src/bin/pg_upgrade/t/006_modes.pl
new file mode 100644
index 00000000000..468591fc486
--- /dev/null
+++ b/src/bin/pg_upgrade/t/006_modes.pl
@@ -0,0 +1,67 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+# Tests for file transfer modes
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_mode
+{
+	my ($mode) = @_;
+
+	my $old = PostgreSQL::Test::Cluster->new('old');
+	my $new = PostgreSQL::Test::Cluster->new('new');
+
+	$old->init();
+	$new->init();
+
+	$old->start;
+	$old->safe_psql('postgres', "CREATE TABLE test1 AS SELECT generate_series(1, 100)");
+	$old->safe_psql('postgres', "CREATE DATABASE testdb");
+	$old->safe_psql('testdb', "CREATE TABLE test2 AS SELECT generate_series(200, 300)");
+	$old->safe_psql('testdb', "VACUUM FULL test2");
+	$old->safe_psql('testdb', "CREATE SEQUENCE testseq START 5432");
+	$old->stop;
+
+	my $result = command_ok_or_fails_like(
+		[
+			'pg_upgrade', '--no-sync',
+			'--old-datadir' => $old->data_dir,
+			'--new-datadir' => $new->data_dir,
+			'--old-bindir' => $old->config_data('--bindir'),
+			'--new-bindir' => $new->config_data('--bindir'),
+			'--socketdir' => $new->host,
+			'--old-port' => $old->port,
+			'--new-port' => $new->port,
+			$mode
+		],
+		qr/.* not supported on this platform|could not .* between old and new data directories: .*/,
+		qr/^$/,
+		"pg_upgrade with transfer mode $mode");
+
+	if ($result)
+	{
+		$new->start;
+		$result = $new->safe_psql('postgres', "SELECT COUNT(*) FROM test1");
+		is($result, '100', "test1 data after pg_upgrade $mode");
+		$result = $new->safe_psql('testdb', "SELECT COUNT(*) FROM test2");
+		is($result, '101', "test2 data after pg_upgrade $mode");
+		$result = $new->safe_psql('testdb', "SELECT nextval('testseq')");
+		is($result, '5432', "sequence data after pg_upgrade $mode");
+		$new->stop;
+	}
+
+	$old->clean_node();
+	$new->clean_node();
+}
+
+test_mode('--clone');
+test_mode('--copy');
+test_mode('--copy-file-range');
+test_mode('--link');
+
+done_testing();
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index bab3f3d2dbe..bda19bbcee2 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2801,6 +2801,25 @@ sub command_fails_like
 
 =pod
 
+=item $node->command_ok_or_fails_like(...)
+
+PostgreSQL::Test::Utils::command_ok_or_fails_like with our connection parameters. See command_ok(...)
+
+=cut
+
+sub command_ok_or_fails_like
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+	my $self = shift;
+
+	local %ENV = $self->_get_env();
+
+	return PostgreSQL::Test::Utils::command_ok_or_fails_like(@_);
+}
+
+=pod
+
 =item $node->command_checks_all(...)
 
 PostgreSQL::Test::Utils::command_checks_all with our connection parameters. See
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index d1ad131eadf..7d7ca83495f 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -89,6 +89,7 @@ our @EXPORT = qw(
   command_like
   command_like_safe
   command_fails_like
+  command_ok_or_fails_like
   command_checks_all
 
   $windows_os
@@ -1067,6 +1068,30 @@ sub command_fails_like
 
 =pod
 
+=item command_ok_or_fails_like(cmd, expected_stdout, expected_stderr, test_name)
+
+Check that the command either succeeds or fails with an error that matches the
+given regular expressions.
+
+=cut
+
+sub command_ok_or_fails_like
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	my ($cmd, $expected_stdout, $expected_stderr, $test_name) = @_;
+	my ($stdout, $stderr);
+	print("# Running: " . join(" ", @{$cmd}) . "\n");
+	my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr;
+	if (!$result)
+	{
+		like($stdout, $expected_stdout, "$test_name: stdout matches");
+		like($stderr, $expected_stderr, "$test_name: stderr matches");
+	}
+	return $result;
+}
+
+=pod
+
 =item command_checks_all(cmd, ret, out, err, test_name)
 
 Run a command and check its status and outputs.
-- 
2.39.5 (Apple Git-154)


--RVUTIYpSRw61lFha--