On 05/10/2024 22:51, Dagfinn Ilmari Mannsåker wrote:
Heikki Linnakangas <hlinn...@iki.fi> writes:
Sadly Windows' IO::Socket::UNIX hasn't been implemented on Windows (or
at least on this perl distribution we're using in Cirrus CI):
Socket::pack_sockaddr_un not implemented on this architecture at
C:/strawberry/5.26.3.1/perl/lib/Socket.pm line 872.
so I'll have to disable this test on Windows anyway.
Socket version 2.028 (included in Perl 5.32) provides pack_sockaddr_un()
on Windows, so that can be fixed by bumping the Perl version in
https://github.com/anarazel/pg-vm-images/blob/main/packer/windows.pkr.hcl
to something more modern (such as 5.40.0.1), and only skipping the test
if on Windows if Socket is too old.
The decision to use 5.26 seems to come from the initial creation of the
CI images in 2021 (when 5.34 was current), with the comment «newer
versions don't currently work correctly for plperl». That claim is
worth revisiting, and fixing if it's still the case.
Yeah, it would be nice to update it. I wonder if commit
341f4e002d461a3c5513cb864490cddae2b43a64 fixed whatever the problem was.
In the meanwhile, here is a one more version of the test patches, with a
SKIP that checks that IO::Socket::UNIX works.
--
Heikki Linnakangas
Neon (https://neon.tech)
From 9b499e8cdf09a127d7506837d5e2a697dd342820 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue, 8 Oct 2024 00:54:30 +0300
Subject: [PATCH v6 1/3] Add test for connection limits
Reviewed-by: Andres Freund <and...@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec...@iki.fi
---
src/test/Makefile | 2 +-
src/test/meson.build | 1 +
src/test/postmaster/Makefile | 23 ++++++
src/test/postmaster/README | 27 +++++++
src/test/postmaster/meson.build | 12 +++
.../postmaster/t/001_connection_limits.pl | 79 +++++++++++++++++++
6 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 src/test/postmaster/Makefile
create mode 100644 src/test/postmaster/README
create mode 100644 src/test/postmaster/meson.build
create mode 100644 src/test/postmaster/t/001_connection_limits.pl
diff --git a/src/test/Makefile b/src/test/Makefile
index dbd3192874..abdd6e5a98 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = perl regress isolation modules authentication recovery subscription
+SUBDIRS = perl postmaster regress isolation modules authentication recovery subscription
ifeq ($(with_icu),yes)
SUBDIRS += icu
diff --git a/src/test/meson.build b/src/test/meson.build
index c3d0dfedf1..67376e4b7f 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -4,6 +4,7 @@ subdir('regress')
subdir('isolation')
subdir('authentication')
+subdir('postmaster')
subdir('recovery')
subdir('subscription')
subdir('modules')
diff --git a/src/test/postmaster/Makefile b/src/test/postmaster/Makefile
new file mode 100644
index 0000000000..dfcce9c9ee
--- /dev/null
+++ b/src/test/postmaster/Makefile
@@ -0,0 +1,23 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/postmaster
+#
+# Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/postmaster/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/postmaster
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
+clean distclean:
+ rm -rf tmp_check
diff --git a/src/test/postmaster/README b/src/test/postmaster/README
new file mode 100644
index 0000000000..7e47bf5cff
--- /dev/null
+++ b/src/test/postmaster/README
@@ -0,0 +1,27 @@
+src/test/postmaster/README
+
+Regression tests for postmaster
+===============================
+
+This directory contains a test suite for postmaster's handling of
+connections, connection limits, and startup/shutdown sequence.
+
+
+Running the tests
+=================
+
+NOTE: You must have given the --enable-tap-tests argument to configure.
+
+Run
+ make check
+or
+ make installcheck
+You can use "make installcheck" if you previously did "make install".
+In that case, the code in the installation tree is tested. With
+"make check", a temporary installation tree is built from the current
+sources and then tested.
+
+Either way, this test initializes, starts, and stops a test Postgres
+cluster.
+
+See src/test/perl/README for more info about running these tests.
diff --git a/src/test/postmaster/meson.build b/src/test/postmaster/meson.build
new file mode 100644
index 0000000000..c2de2e0eb5
--- /dev/null
+++ b/src/test/postmaster/meson.build
@@ -0,0 +1,12 @@
+# Copyright (c) 2022-2024, PostgreSQL Global Development Group
+
+tests += {
+ 'name': 'postmaster',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_connection_limits.pl',
+ ],
+ },
+}
diff --git a/src/test/postmaster/t/001_connection_limits.pl b/src/test/postmaster/t/001_connection_limits.pl
new file mode 100644
index 0000000000..f50aae4949
--- /dev/null
+++ b/src/test/postmaster/t/001_connection_limits.pl
@@ -0,0 +1,79 @@
+
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+# Test connection limits, i.e. max_connections, reserved_connections
+# and superuser_reserved_connections.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize the server with specific low connection limits
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init;
+$node->append_conf('postgresql.conf', "max_connections = 6");
+$node->append_conf('postgresql.conf', "reserved_connections = 2");
+$node->append_conf('postgresql.conf', "superuser_reserved_connections = 1");
+$node->append_conf('postgresql.conf', "log_connections = on");
+$node->start;
+
+$node->safe_psql(
+ 'postgres', qq{
+CREATE USER regress_regular LOGIN;
+CREATE USER regress_reserved LOGIN;
+GRANT pg_use_reserved_connections TO regress_reserved;
+CREATE USER regress_superuser LOGIN SUPERUSER;
+});
+
+# With the limits we set in postgresql.conf, we can establish:
+# - 3 connections for any user with no special privileges
+# - 2 more connections for users belonging to "pg_use_reserved_connections"
+# - 1 more connection for superuser
+
+sub background_psql_as_user
+{
+ my $user = shift;
+
+ return $node->background_psql(
+ 'postgres',
+ on_error_die => 1,
+ extra_params => [ '-U', $user ]);
+}
+
+my @sessions = ();
+
+push(@sessions, background_psql_as_user('regress_regular'));
+push(@sessions, background_psql_as_user('regress_regular'));
+push(@sessions, background_psql_as_user('regress_regular'));
+$node->connect_fails(
+ "dbname=postgres user=regress_regular",
+ "reserved_connections limit",
+ expected_stderr =>
+ qr/FATAL: remaining connection slots are reserved for roles with privileges of the "pg_use_reserved_connections" role/
+);
+
+push(@sessions, background_psql_as_user('regress_reserved'));
+push(@sessions, background_psql_as_user('regress_reserved'));
+$node->connect_fails(
+ "dbname=postgres user=regress_regular",
+ "reserved_connections limit",
+ expected_stderr =>
+ qr/FATAL: remaining connection slots are reserved for roles with the SUPERUSER attribute/
+);
+
+push(@sessions, background_psql_as_user('regress_superuser'));
+$node->connect_fails(
+ "dbname=postgres user=regress_superuser",
+ "superuser_reserved_connections limit",
+ expected_stderr => qr/FATAL: sorry, too many clients already/);
+
+# TODO: test that query cancellation is still possible
+
+foreach my $session (@sessions)
+{
+ $session->quit;
+}
+
+done_testing();
--
2.39.5
From 5fe25211724859bfa29bff73ac780322ab95181c Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue, 8 Oct 2024 00:54:33 +0300
Subject: [PATCH v6 2/3] Add test for dead-end backends
The code path for launching a dead-end backend because we're out of
slots was not covered by any tests, so add one. (Some tests did hit
the case of launching a dead-end backend because the server is still
starting up, though, so the gap in our test coverage wasn't as big as
it sounds.)
Reviewed-by: Andres Freund <and...@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec...@iki.fi
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 78 +++++++++++++++++++
.../postmaster/t/001_connection_limits.pl | 42 +++++++++-
2 files changed, 119 insertions(+), 1 deletion(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 30857f34bf..63c25eeb83 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -104,6 +104,7 @@ use File::Path qw(rmtree mkpath);
use File::Spec;
use File::stat qw(stat);
use File::Temp ();
+use IO::Socket::INET;
use IPC::Run;
use PostgreSQL::Version;
use PostgreSQL::Test::RecursiveCopy;
@@ -291,6 +292,83 @@ sub connstr
=pod
+=item $node->raw_connect()
+
+Open a raw TCP or Unix domain socket connection to the server. This is
+used by low-level protocol and connection limit tests.
+
+=cut
+
+sub raw_connect
+{
+ my ($self) = @_;
+ my $pgport = $self->port;
+ my $pghost = $self->host;
+
+ my $socket;
+ if ($PostgreSQL::Test::Utils::use_unix_sockets)
+ {
+ require IO::Socket::UNIX;
+ my $path = "$pghost/.s.PGSQL.$pgport";
+
+ $socket = IO::Socket::UNIX->new(
+ Type => SOCK_STREAM(),
+ Peer => $path,
+ ) or die "Cannot create socket - $IO::Socket::errstr\n";
+ }
+ else
+ {
+ $socket = IO::Socket::INET->new(
+ PeerHost => $pghost,
+ PeerPort => $pgport,
+ Proto => 'tcp'
+ ) or die "Cannot create socket - $IO::Socket::errstr\n";
+ }
+ return $socket;
+}
+
+=pod
+
+=item $node->raw_connect_works()
+
+Check if raw_connect() function works on this platform. This should
+be called to SKIP any tests that require raw_connect().
+
+This tries to connect to the server, to test whether it works or not,,
+so the server is up and running. Otherwise this can return 0 even if
+there's nothing wrong with raw_connect() itself.
+
+Notably, raw_connect() does not work on Unix domain sockets on
+Strawberry perl 5.26.3.1 on Windows, which we use in Cirrus CI images
+as of this writing. It dies with "not implemented on this
+architecture".
+
+=cut
+
+sub raw_connect_works
+{
+ my ($self) = @_;
+
+ # If we're using Unix domain sockets, we need a working
+ # IO::Socket::UNIX implementation.
+ if ($PostgreSQL::Test::Utils::use_unix_sockets)
+ {
+ diag "checking if IO::Socket::UNIX works";
+ eval {
+ my $sock = $self->raw_connect();
+ $sock->close();
+ };
+ if ($@ =~ /not implemented/)
+ {
+ diag "IO::Socket::UNIX does not work: $@";
+ return 0;
+ }
+ }
+ return 1
+}
+
+=pod
+
=item $node->group_access()
Does the data dir allow group access?
diff --git a/src/test/postmaster/t/001_connection_limits.pl b/src/test/postmaster/t/001_connection_limits.pl
index f50aae4949..f8d24bcf24 100644
--- a/src/test/postmaster/t/001_connection_limits.pl
+++ b/src/test/postmaster/t/001_connection_limits.pl
@@ -43,6 +43,7 @@ sub background_psql_as_user
}
my @sessions = ();
+my @raw_connections = ();
push(@sessions, background_psql_as_user('regress_regular'));
push(@sessions, background_psql_as_user('regress_regular'));
@@ -69,11 +70,50 @@ $node->connect_fails(
"superuser_reserved_connections limit",
expected_stderr => qr/FATAL: sorry, too many clients already/);
-# TODO: test that query cancellation is still possible
+# We can still open TCP (or Unix domain socket) connections, but
+# beyond a certain number (roughly 2x max_connections), they will be
+# "dead-end backends".
+SKIP:
+{
+ skip "this test requies working raw_connect()" unless $node->raw_connect_works();
+
+ for (my $i = 0; $i <= 20; $i++)
+ {
+ my $sock = $node->raw_connect();
+
+ # On a busy system, the server might reject connections if
+ # postmaster cannot accept() them fast enough. The exact limit
+ # and behavior depends on the platform. To make this reliable,
+ # we attempt SSL negotiation on each connection before opening
+ # next one. The server will reject the SSL negotations, but
+ # when it does so, we know that the backend has been launched
+ # and we should be able to open another connection.
+
+ # SSLRequest packet consists of packet length followed by
+ # NEGOTIATE_SSL_CODE.
+ my $negotiate_ssl_code = pack("Nnn", 8, 1234, 5679);
+ my $sent = $sock->send($negotiate_ssl_code);
+
+ # Read reply. We expect the server to reject it with 'N'
+ my $reply = "";
+ $sock->recv($reply, 1);
+ is($reply, "N", "dead-end connection $i");
+ push(@raw_connections, $sock);
+ }
+}
+
+# TODO: test that query cancellation is still possible. A dead-end
+# backend can process a query cancellation packet.
+
+# Clean up
foreach my $session (@sessions)
{
$session->quit;
}
+foreach my $socket (@raw_connections)
+{
+ $socket->close();
+}
done_testing();
--
2.39.5
