Date: Mon, 31 Aug 2026 11:27:07 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Ewan Young <kdbase.hack@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org,
	Michael Paquier <michael@paquier.xyz>
Subject: Re: pg_upgrade: Test --check with a running source server
Message-ID: <apVlC6auNuqnuhKD@bdtpg>
References: <apU4/hmRv/4gv20W@bdtpg>
 <CAON2xHMRAb9M_dKF6WuwVHQLLmGYXkKp=KQpCy7URKD3Av2OhA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="Iv49dcxIGoH7n1uf"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAON2xHMRAb9M_dKF6WuwVHQLLmGYXkKp=KQpCy7URKD3Av2OhA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/apVlC6auNuqnuhKD%40bdtpg>
Precedence: bulk


--Iv49dcxIGoH7n1uf
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hi,

On Mon, Aug 31, 2026 at 06:54:52PM +0800, Ewan Young wrote:
> On Mon, Aug 31, 2026 at 4:19 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> > 0001 attached adds coverage with the source still running. This covers live server
> > detection, connection details from postmaster.pid, control data from pg_controldata,
> > reuse of the running postmaster, and the requirement for different source and target
> > ports.
> 
> Thanks for the patches. I had a look at both patches.

Thanks!

> One optional thought on the success case in 002: command_like()
> matching the live-mode banner would be a bit more precise than
> command_ok(), since exit code 0 alone can't tell a live check from an
> offline one (against a stopped old server, pg_upgrade would start it
> itself and succeed just the same):
> 
>     command_like(..., qr/Performing Consistency Checks on Old Live Server/, ...)

The preceding test already proves that live mode is selected by checking that
using the same port error, which is specific to live checks. That said, matching
the banner makes it self contained, so changed it that way in the attached v2.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--Iv49dcxIGoH7n1uf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-pg_upgrade-Test-check-with-a-running-source-serve.patch"

From 04f238fb28ed814ab7a6ebc107f25f9c64431c51 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 31 Aug 2026 03:03:33 +0000
Subject: [PATCH v2 1/2] pg_upgrade: Test --check with a running source server

Commit 4fff78f009 added TAP coverage for pg_upgrade --check, but only after
stopping the source server. Add coverage with the source still running.

This covers live server detection, connection details from postmaster.pid,
control data from pg_controldata, reuse of the running postmaster, and the
requirement for different source and target ports.

Also test that logical slots with unconsumed WAL are accepted during a live
check. The existing offline check verifies that the same slots are rejected after
shutdown.

Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/apU4/hmRv/4gv20W%40bdtpg
---
 src/bin/pg_upgrade/t/002_pg_upgrade.pl    | 46 ++++++++++++++++++++---
 src/bin/pg_upgrade/t/003_logical_slots.pl | 13 +++++++
 2 files changed, 54 insertions(+), 5 deletions(-)
 100.0% src/bin/pg_upgrade/t/

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 0a4121fdc4d..a0bc51f3292 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -475,6 +475,47 @@ if (defined($ENV{oldinstall}))
 	}
 }
 
+# In a VPATH build, we'll be started in the source directory, but we want
+# to run pg_upgrade in the build directory so that any files generated finish
+# in it, like delete_old_cluster.{sh,bat}.
+chdir ${PostgreSQL::Test::Utils::tmp_check};
+
+my @live_check_command = (
+	'pg_upgrade', '--no-sync',
+	'--old-datadir' => $oldnode->data_dir,
+	'--new-datadir' => $newnode->data_dir,
+	'--old-bindir' => $oldbindir,
+	'--new-bindir' => $newbindir,
+	'--socketdir' => $newnode->host,
+	'--old-port' => $oldnode->port);
+
+# A live check must use different ports for the running old server and
+# the temporary new server.
+command_checks_all(
+	[
+		@live_check_command,
+		'--new-port' => $oldnode->port,
+		$mode, '--check',
+	],
+	1,
+	[
+		qr/When checking a live server, the old and new port numbers must be different\./
+	],
+	[],
+	'pg_upgrade --check with the same old and new ports');
+
+rmtree($newnode->data_dir . "/pg_upgrade_output.d");
+
+# Check the old cluster while it is running.
+command_like(
+	[
+		@live_check_command,
+		'--new-port' => $newnode->port,
+		$mode, '--check',
+	],
+	qr/Performing Consistency Checks on Old Live Server/,
+	'run of pg_upgrade --check with old instance running');
+
 # Create an invalid database, will be deleted below
 $oldnode->safe_psql(
 	'postgres', qq(
@@ -482,11 +523,6 @@ $oldnode->safe_psql(
   UPDATE pg_database SET datconnlimit = -2 WHERE datname = 'regression_invalid';
 ));
 
-# In a VPATH build, we'll be started in the source directory, but we want
-# to run pg_upgrade in the build directory so that any files generated finish
-# in it, like delete_old_cluster.{sh,bat}.
-chdir ${PostgreSQL::Test::Utils::tmp_check};
-
 # Upgrade the instance.
 $oldnode->stop;
 
diff --git a/src/bin/pg_upgrade/t/003_logical_slots.pl b/src/bin/pg_upgrade/t/003_logical_slots.pl
index 01ab82402ae..b0ebf845747 100644
--- a/src/bin/pg_upgrade/t/003_logical_slots.pl
+++ b/src/bin/pg_upgrade/t/003_logical_slots.pl
@@ -158,6 +158,19 @@ $oldpub->safe_psql(
 		SELECT count(*) FROM pg_logical_emit_message('false', 'prefix', 'This is a non-transactional message', true);
 		SELECT pg_replication_slot_advance('test_slot3', pg_current_wal_lsn());
 ]);
+
+# Sleep here because Windows builds cannot check postmaster.pid exactly,
+# so they may mistake a pre-existing postmaster.pid for one created by the
+# postmaster they start. Waiting more than the 2 seconds slop time allowed
+# by wait_for_postmaster_start() prevents that mistake.
+sleep 3 if ($windows_os);
+
+# A live check cannot require slots to have consumed all WAL because the old
+# server can generate more WAL concurrently. Verify that these slots are accepted
+# now. The check below verifies rejection after shutdown.
+command_ok([ @pg_upgrade_cmd, '--check' ],
+	'pg_upgrade --check with live old cluster and unconsumed WAL');
+
 $oldpub->stop;
 
 # pg_upgrade will fail because there are slots still having unconsumed WAL
-- 
2.34.1


--Iv49dcxIGoH7n1uf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0002-Fix-outdated-function-name-in-001_start_stop.pl.patch"

From dd2d5e36ae864a3397eab59fe810528aab67ba07 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 31 Aug 2026 07:54:08 +0000
Subject: [PATCH v2 2/2] Fix outdated function name in 001_start_stop.pl

Commit a745b936507 renamed wait_for_postmaster() to wait_for_postmaster_start(),
but missed this reference in 001_start_stop.pl. Update the comment.

Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/apU4/hmRv/4gv20W%40bdtpg
---
 src/bin/pg_ctl/t/001_start_stop.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 100.0% src/bin/pg_ctl/t/

diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index a189b379f55..38634fd26ce 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -55,7 +55,7 @@ command_like($ctlcmd, qr/done.*server started/s, 'pg_ctl start');
 # sleep here is because Windows builds can't check postmaster.pid exactly,
 # so they may mistake a pre-existing postmaster.pid for one created by the
 # postmaster they start.  Waiting more than the 2 seconds slop time allowed
-# by wait_for_postmaster() prevents that mistake.
+# by wait_for_postmaster_start() prevents that mistake.
 sleep 3 if ($windows_os);
 command_fails([ 'pg_ctl', 'start', '--pgdata' => "$tempdir/data" ],
 	'second pg_ctl start fails');
-- 
2.34.1


--Iv49dcxIGoH7n1uf--