From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Greg Sabino Mullane <htamfids@gmail.com>
Cc: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us
Subject: Re: optimize file transfer in pg_upgrade
Date: Tue, 18 Mar 2025 14:08:42 -0500
Message-ID: <Z9nEup6k56N_9l0X@nathan> (raw)
In-Reply-To: <7rhxl5nl5cuenm4kzprjhaojo74m2teuwzjffcrdi6bxx6b2wn@orb5h3mteh2y>
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>
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
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.buildindex 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.plnew file mode 100644index 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.pmindex 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.pmindex 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)
Attachments:
[text/plain] v2-0001-Add-test-for-pg_upgrade-file-transfer-modes.patch (5.0K, ../Z9nEup6k56N_9l0X@nathan/2-v2-0001-Add-test-for-pg_upgrade-file-transfer-modes.patch)
download | inline diff:
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.buildindex 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.plnew file mode 100644index 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.pmindex 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.pmindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, htamfids@gmail.com, bruce@momjian.us
Subject: Re: optimize file transfer in pg_upgrade
In-Reply-To: <Z9nEup6k56N_9l0X@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox