Prevent port collisions between concurrent TAP tests

Currently there is a race condition where if concurrent TAP tests both
test that they can open a port they will assume that it is free and use
it, causing one of them to fail. To prevent this we record a reservation
using an exclusive lock, and any TAP test that discovers a reservation
checks to see if the reserving process is still alive, and looks for
another free port if it is.

Ports are reserved in a directory set by the environment setting
PG_TEST_PORT_DIR, or if that doesn't exist a subdirectory of the top
build directory as set by Makefile.global, or its own
tmp_check directory.

The prove_check recipe in Makefile.global.in is extended to export
top_builddir to the TAP tests. This was already exported by the
prove_installcheck recipes.

Per complaint from Andres Freund

Backpatched from 9b4eafcaf4 to all live branches

Discussion: https://postgr.es/m/20221002164931.d57hlutrcz4d2zi7@awork3.anarazel.de
This commit is contained in:
Andrew Dunstan 2022-11-22 10:35:04 -05:00
parent 36eeb37cd6
commit 4b9013d64f
2 changed files with 60 additions and 5 deletions

View File

@ -471,6 +471,7 @@ rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && \
TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
top_builddir='$(CURDIR)/$(top_builddir)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef

View File

@ -90,9 +90,9 @@ use Carp;
use Config;
use Cwd;
use Exporter 'import';
use Fcntl qw(:mode);
use Fcntl qw(:mode :flock :seek :DEFAULT);
use File::Basename;
use File::Path qw(rmtree);
use File::Path qw(rmtree mkpath);
use File::Spec;
use File::stat qw(stat);
use File::Temp ();
@ -110,7 +110,10 @@ our @EXPORT = qw(
);
our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
$last_port_assigned, @all_nodes, $died);
$last_port_assigned, @all_nodes, $died, $portdir);
# list of file reservations made by get_free_port
my @port_reservation_files;
INIT
{
@ -126,6 +129,20 @@ INIT
# Tracking of last port value assigned to accelerate free port lookup.
$last_port_assigned = int(rand() * 16384) + 49152;
# Set the port lock directory
# If we're told to use a directory (e.g. from a buildfarm client)
# explicitly, use that
$portdir = $ENV{PG_TEST_PORT_DIR};
# Otherwise, try to use a directory at the top of the build tree
# or as a last resort use the tmp_check directory
my $build_dir = $ENV{top_builddir}
|| $TestLib::tmp_check ;
$portdir ||= "$build_dir/portlock";
$portdir =~ s!\\!/!g;
# Make sure the directory exists
mkpath($portdir) unless -d $portdir;
}
=pod
@ -1210,8 +1227,8 @@ by test cases that need to start other, non-Postgres servers.
Ports assigned to existing PostgresNode objects are automatically
excluded, even if those servers are not currently running.
XXX A port available now may become unavailable by the time we start
the desired service.
The port number is reserved so that other concurrent test programs will not
try to use the same port.
=cut
@ -1260,6 +1277,7 @@ sub get_free_port
last;
}
}
$found = _reserve_port($port) if $found;
}
}
@ -1290,6 +1308,40 @@ sub can_bind
return $ret;
}
# Internal routine to reserve a port number
# Returns 1 if successful, 0 if port is already reserved.
sub _reserve_port
{
my $port = shift;
# open in rw mode so we don't have to reopen it and lose the lock
my $filename = "$portdir/$port.rsv";
sysopen(my $portfile, $filename, O_RDWR|O_CREAT)
|| die "opening port file $filename: $!";
# take an exclusive lock to avoid concurrent access
flock($portfile, LOCK_EX) || die "locking port file $filename: $!";
# see if someone else has or had a reservation of this port
my $pid = <$portfile>;
chomp $pid;
if ($pid +0 > 0)
{
if (kill 0, $pid)
{
# process exists and is owned by us, so we can't reserve this port
flock($portfile, LOCK_UN);
close($portfile);
return 0;
}
}
# All good, go ahead and reserve the port
seek($portfile, 0, SEEK_SET);
# print the pid with a fixed width so we don't leave any trailing junk
print $portfile sprintf("%10d\n",$$);
flock($portfile, LOCK_UN);
close($portfile);
push(@port_reservation_files, $filename);
return 1;
}
# Automatically shut down any still-running nodes when the test script exits.
# Note that this just stops the postmasters (in the same order the nodes were
# created in). Any temporary directories are deleted, in an unspecified
@ -1311,6 +1363,8 @@ END
$node->clean_node if $exit_code == 0 && TestLib::all_tests_passing();
}
unlink @port_reservation_files;
$? = $exit_code;
}