pg_upgrade: Test --check with a running source server

On Mon, Aug 31, 2026 at 4:19 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

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].

Thanks for the patches. I had a look at both patches.

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/, ...)

0002 looks good: a745b936507 did rename wait_for_postmaster() to
wait_for_postmaster_start(), and it shows the line you're fixing
is the only stale reference left in the tree.

[0]: /messages/by-id/apUL3N4IE934qJ08@bdtpg

Regards,

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

--
Regards,
Ewan Young

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

On Mon, Aug 31, 2026 at 7:27 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

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.

Fair point -- and thanks for the new version (and the credit).

v2 looks good to me. Re-tested both patches on top of current master:
the pg_upgrade and pg_ctl TAP suites pass, including the no-stderr
check that command_like() adds. Nothing more from me.

Regards,

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

--
Regards,
Ewan Young

On Mon, Aug 31, 2026 at 11:27:07AM +0000, Bertrand Drouvot wrote:

On Mon, Aug 31, 2026 at 06:54:52PM +0800, Ewan Young wrote:

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.

These two new tests for a live server, one failure where the new port
and old ports do not match, and one failure where we are checking the
old server with a correct new port, look like acceptable additions
here. Will test a bit more, and apply on HEAD, I guess.

+# by wait_for_postmaster_start() prevents that mistake.
+sleep 3 if ($windows_os);

In 003_logical_slots, I am not convinced that this is going to be
entirely stable all the time.. Hardcoded sleeps never finish well,
even if these are platform-dependent.

Picked up v2-0002 on a different branch where I am stacking all these
mistakes for a group commit.
--
Michael

On Thu, Sep 03, 2026 at 02:51:28PM +0900, Michael Paquier wrote:

These two new tests for a live server, one failure where the new port
and old ports do not match, and one failure where we are checking the
old server with a correct new port, look like acceptable additions
here. Will test a bit more, and apply on HEAD, I guess.

So, one day later fairywren happens to be complaining:
https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=fairywren&amp;br=master

The gist of the issue is that the cluster is not recognized as
running, not considering that a live_check should happen, and it
fails. I am guessing that this points out to an issue similar to what
you have tried to work around with in 003_logical_slots with a
hardcoded sleep, something that we do already once too much. That's,
like pg_ctl, a timing issue on WIN32 with pg_upgrade --check, and that
has been around for a long time.

Rather than adding one of these annoying 3-second sleeps, I'm thinking
about just adding a skip block for WIN32, I guess, as the test is not
showing any instability in other animals as far as I can see. And
live checks are still useful to cover.
--
Michael