From 8eba156b6ca09882b14daa91cf46e8d1c17903cb Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Wed, 14 Oct 2026 19:38:23 +0000 Subject: [PATCH v2] pg_upgrade: handle failed nonblocking connects on Windows Winsock reports a failed nonblocking connection in the exception set. Monitor it so pg_upgrade can pass the failure back to PQconnectPoll and try the next resolved address instead of waiting indefinitely. Following Heikki Linnakangas's suggestion, pass a distinct exception set through select_loop and check it independently of writability on all platforms. Leave the existing EINTR retry behavior unchanged. Add a Windows regression with successful-first IPv4 and refused-first localhost controls. Establish refusal with a bounded raw-socket Perl subprocess, preserving the connect error before socket cleanup. Discussion: https://www.postgresql.org/message-id/CAAH-eSVf=Nvwb6P_HepLiHdKPuNnZANLYgq7CHLsEj54Tau7sg@mail.gmail.com Discussion: https://www.postgresql.org/message-id/98e15036-7f0d-49a2-be0a-1bcd2e581e16@iki.fi --- src/bin/pg_upgrade/meson.build | 1 + src/bin/pg_upgrade/t/009_connect_failure.pl | 136 ++++++++++++++++++++ src/bin/pg_upgrade/task.c | 16 ++- 3 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 src/bin/pg_upgrade/t/009_connect_failure.pl diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build index ffbf6ae8d759bf8d95559c735d846ba1869347d3..9b5f2688626ff89504e089ec3399313d7db7e5da 100644 --- a/src/bin/pg_upgrade/meson.build +++ b/src/bin/pg_upgrade/meson.build @@ -69,6 +69,7 @@ tests += { 't/006_transfer_modes.pl', 't/007_multixact_conversion.pl', 't/008_extension_control_path.pl', + 't/009_connect_failure.pl', ], 'deps': [test_ext], 'test_kwargs': {'priority': 40}, # pg_upgrade tests are slow diff --git a/src/bin/pg_upgrade/t/009_connect_failure.pl b/src/bin/pg_upgrade/t/009_connect_failure.pl new file mode 100644 index 0000000000000000000000000000000000000000..55c8a62a2d3e660984ade6cd88044b4d62f2ae14 --- /dev/null +++ b/src/bin/pg_upgrade/t/009_connect_failure.pl @@ -0,0 +1,136 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Failed nonblocking connects must wake pg_upgrade's Windows task loop. +use strict; +use warnings FATAL => 'all'; + +use IPC::Run; +use Errno qw(ECONNREFUSED); +use Socket + qw(AF_UNSPEC AF_INET6 SOCK_STREAM IPPROTO_TCP NI_NUMERICHOST + getaddrinfo getnameinfo sockaddr_in6 inet_pton); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +plan skip_all => 'Windows select() connection-failure behavior' + unless $windows_os; + +# This trigger needs localhost to try IPv6 before the IPv4-only server. +# Skip explicitly when the resolver cannot supply that ordering. +my ($lookup_error, @addresses) = getaddrinfo('localhost', '5432', + { family => AF_UNSPEC, socktype => SOCK_STREAM }); +plan skip_all => 'localhost address lookup failed' if $lookup_error; +my @hosts; +for my $address (@addresses) +{ + my ($name_error, $host) = getnameinfo($address->{addr}, NI_NUMERICHOST); + plan skip_all => 'numeric localhost lookup failed' if $name_error; + push @hosts, $host; +} +plan skip_all => 'localhost must resolve to ::1 first and include 127.0.0.1' + unless @hosts && $hosts[0] eq '::1' && grep { $_ eq '127.0.0.1' } @hosts; + +my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); +my $newnode = PostgreSQL::Test::Cluster->new('new_node'); +$oldnode->init(extra => ['--locale=C']); +$newnode->init(extra => ['--locale=C']); +$oldnode->append_conf('postgresql.conf', "listen_addresses = '127.0.0.1'"); +$newnode->append_conf('postgresql.conf', "listen_addresses = '127.0.0.1'"); +$oldnode->start; + +# Confirm refusal in a bounded subprocess. Capture the native connect error +# before closing the socket; IO::Socket::IP can overwrite it during cleanup. +my $refusal_probe = <<'PERL'; +use strict; +use warnings; +use Errno qw(ECONNREFUSED); +use Socket qw(AF_INET6 SOCK_STREAM IPPROTO_TCP sockaddr_in6 inet_pton); +socket(my $socket, AF_INET6, SOCK_STREAM, IPPROTO_TCP) + or die "cannot create IPv6 socket: $!"; +my $connected = connect($socket, + sockaddr_in6($ARGV[0], inet_pton(AF_INET6, '::1'))); +my $refused = $!{ECONNREFUSED}; +my $error = "$!"; +close $socket; +if ($connected) { print "ACCEPTED\n"; exit 1; } +if ($refused) { print "REFUSED\n"; exit 0; } +print STDERR "IPv6 endpoint did not refuse the connection: $error\n"; +exit 2; +PERL +my ($probe_stdout, $probe_stderr, $probe_status); +my $probe_ok = eval { + IPC::Run::run([$^X, '-e', $refusal_probe, $oldnode->port], + '>' => \$probe_stdout, '2>' => \$probe_stderr, + IPC::Run::timeout(3)); + $probe_status = $?; + 1; +}; +my $probe_error = $@; +if (!$probe_ok || !defined $probe_status || $probe_status != 0 + || $probe_stdout ne "REFUSED\n") +{ + $oldnode->stop; + plan skip_all => "IPv6 refusal precondition not established: " + . ($probe_error || $probe_stderr || $probe_stdout); +} + +# pg_upgrade writes its logs relative to its working directory. +chdir $PostgreSQL::Test::Utils::tmp_check; + +my @command = ( + 'pg_upgrade', '--check', '--no-sync', '--jobs=2', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir, + '--old-bindir' => $oldnode->config_data('--bindir'), + '--new-bindir' => $newnode->config_data('--bindir'), + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port); +my @checks; +for my $host ('127.0.0.1', 'localhost') +{ + my ($stdout, $stderr) = ('', ''); + my ($harness, $error, $status); + { + # IPv4 is the successful-first control; localhost must fall back. + local $ENV{PGHOST} = $host; + local $ENV{PGHOSTADDR}; + delete $ENV{PGHOSTADDR}; + my $timer = + IPC::Run::timeout($PostgreSQL::Test::Utils::timeout_default); + eval { + $harness = IPC::Run::start( + \@command, + '>' => \$stdout, + '2>' => \$stderr, + $timer); + $harness->finish; + $status = $?; + }; + $error = $@; + $harness->kill_kill if $error && defined $harness; + } + push @checks, [ $host, $stdout, $stderr, $error, $status ]; + last if $error || !defined $status || $status; +} + +# Cleanup precedes assertions and includes a server left after a timeout. +$newnode->_update_pid(-1); +$newnode->stop; +$oldnode->stop; + +for my $check (@checks) +{ + my ($host, $stdout, $stderr, $error, $status) = @$check; + ok( !$error && defined $status && $status == 0, + "pg_upgrade --check completes with host=$host"); + like( + $stdout, + qr/Clusters are compatible/, + "pg_upgrade checks both clusters with host=$host"); + diag("$error\n$stdout\n$stderr") if $error || !defined $status || $status; +} +is(scalar @checks, 2, 'both successful-first and refused-first cases ran'); +ok(!-e $oldnode->data_dir . '/postmaster.pid', 'old server is stopped'); +ok(!-e $newnode->data_dir . '/postmaster.pid', 'new server is stopped'); +done_testing(); diff --git a/src/bin/pg_upgrade/task.c b/src/bin/pg_upgrade/task.c index b6eb29e1f3a635870e5c3abd4077d450be389099..12e118bfcab35b95b3baf6cf02aeae222ffafe23 100644 --- a/src/bin/pg_upgrade/task.c +++ b/src/bin/pg_upgrade/task.c @@ -329,10 +329,11 @@ process_slot(const ClusterInfo *cluster, UpgradeTaskSlot *slot, const UpgradeTas * Returns -1 on error, else the number of ready descriptors. */ static int -select_loop(int maxFd, fd_set *input, fd_set *output) +select_loop(int maxFd, fd_set *input, fd_set *output, fd_set *exceptions) { fd_set save_input = *input; fd_set save_output = *output; + fd_set save_exceptions = *exceptions; if (maxFd == 0) return 0; @@ -344,7 +345,8 @@ select_loop(int maxFd, fd_set *input, fd_set *output) *input = save_input; *output = save_output; - i = select(maxFd + 1, input, output, NULL, NULL); + *exceptions = save_exceptions; + i = select(maxFd + 1, input, output, exceptions, NULL); #ifndef WIN32 if (i < 0 && errno == EINTR) @@ -366,10 +368,12 @@ wait_on_slots(UpgradeTaskSlot *slots, int numslots) { fd_set input; fd_set output; + fd_set exceptions; int maxFd = 0; FD_ZERO(&input); FD_ZERO(&output); + FD_ZERO(&exceptions); for (int i = 0; i < numslots; i++) { @@ -396,13 +400,16 @@ wait_on_slots(UpgradeTaskSlot *slots, int numslots) if (slots[i].sock < 0) pg_fatal("invalid socket"); FD_SET(slots[i].sock, slots[i].select_mode ? &input : &output); + /* Failed nonblocking connects are exceptional on Windows. */ + if (!slots[i].select_mode) + FD_SET(slots[i].sock, &exceptions); maxFd = Max(maxFd, slots[i].sock); } /* * If we found socket(s) to wait on, wait. */ - if (select_loop(maxFd, &input, &output) == -1) + if (select_loop(maxFd, &input, &output, &exceptions) == -1) pg_fatal("%s() failed: %m", "select"); /* @@ -410,7 +417,8 @@ wait_on_slots(UpgradeTaskSlot *slots, int numslots) */ for (int i = 0; i < numslots; i++) slots[i].ready |= (FD_ISSET(slots[i].sock, &input) || - FD_ISSET(slots[i].sock, &output)); + FD_ISSET(slots[i].sock, &output) || + FD_ISSET(slots[i].sock, &exceptions)); } /* base-commit: 158b6d5109fad9d5f0c30693fa85041a1c621fe1