Hi hackers, While doing some tests for [0], I noticed that we don't have coverage for pg_upgrade --check with a running source server.
Indeed, commit 4fff78f009 added TAP coverage for pg_upgrade --check, but only after stopping the source server. 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. It also tests 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. Remarks: 1/ my CI was failing on Windows (at the new 003_logical_slots.pl test) for the same reason why a sleep is part of 001_start_stop.pl. So adding the same comment and sleep in 003_logical_slots.pl too. 2/ while at 1/ I observed the comment in 001_start_stop.pl was using a stale function name. 0002 fixes it. 3/ the proposed tests would have had catch === 2 mentioned in [0]. [0]: https://postgr.es/m/apUL3N4IE934qJ08%40bdtpg Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From e21baac6d18d9d4803cc9a79170070e9c25ba65e Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 31 Aug 2026 03:03:33 +0000 Subject: [PATCH v1 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 <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/... --- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 45 ++++++++++++++++++++--- src/bin/pg_upgrade/t/003_logical_slots.pl | 13 +++++++ 2 files changed, 53 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..9b904a8d1ea 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -475,6 +475,46 @@ 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_ok( + [ + @live_check_command, + '--new-port' => $newnode->port, + $mode, '--check', + ], + 'run of pg_upgrade --check with old instance running'); + # Create an invalid database, will be deleted below $oldnode->safe_psql( 'postgres', qq( @@ -482,11 +522,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
>From 6482720e5d5e7a8d45435b51917e2864b0afa871 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 31 Aug 2026 07:54:08 +0000 Subject: [PATCH v1 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 <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/... --- 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
