Thread: port conflicts when running tests concurrently on windows.
Hi, With the meson patch applied the tests on windows run concurrently by default. Unfortunately that fails semi-regularly. The reason for this basically is that windows defaults to using TCP in tests, and that the tap-test port determination is very racy: > # When selecting a port, we look for an unassigned TCP port number, > # even if we intend to use only Unix-domain sockets. This is clearly > # necessary on $use_tcp (Windows) configurations, and it seems like a > # good idea on Unixen as well. > $port = get_free_port(); > ... > =item get_free_port() > > Locate an unprivileged (high) TCP port that's not currently bound to > anything. This is used by C<new()>, and also by some test cases that need to > start other, non-Postgres servers. > > Ports assigned to existing PostgreSQL::Test::Cluster 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. I don't think there's an easy way to make this race-free. We'd need to teach postmaster to use pre-opened socket or something like that. An alternative to that would be to specify a base port number externally. In the meson branch I already did that for the pg_regress style tests, since they don't have the automatic port thing above. But for tap tests there's currently no way to pass in a base-port that I can see. Is it perhaps time to to use unix sockets on windows by default (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? Greetings, Andres Freund
Hi, On 2021-12-08 14:45:50 -0800, Andres Freund wrote: > Is it perhaps time to to use unix sockets on windows by default > (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? On its own PG_TEST_USE_UNIX_SOCKETS doesn't work at all on windows - it fails trying to use /tmp/ as a socket directory. Using PG_REGRESS_SOCK_DIR fixes that for PG_REGRESS. But the tap tests don't look at that :(. Greetings, Andres Freund
Hi,
On 2021-12-08 16:36:14 -0800, Andres Freund wrote:
> On 2021-12-08 14:45:50 -0800, Andres Freund wrote:
> > Is it perhaps time to to use unix sockets on windows by default
> > (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows?
>
> On its own PG_TEST_USE_UNIX_SOCKETS doesn't work at all on windows - it fails
> trying to use /tmp/ as a socket directory. Using PG_REGRESS_SOCK_DIR fixes
> that for PG_REGRESS. But the tap tests don't look at that :(.
The tap failures in turn are caused by the Cluster.pm choosing a socket
directory with backslashes. Those backslashes are then treated as an escape
character both by guc.c and libpq.
I think this can be addressed by something like
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 9467a199c8f..c2a8487bbab 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -119,7 +119,15 @@ INIT
$use_tcp = !$PostgreSQL::Test::Utils::use_unix_sockets;
$test_localhost = "127.0.0.1";
$last_host_assigned = 1;
- $test_pghost = $use_tcp ? $test_localhost : PostgreSQL::Test::Utils::tempdir_short;
+ if ($use_tcp)
+ {
+ $test_pghost = $test_localhost;
+ }
+ else
+ {
+ $test_pghost = PostgreSQL::Test::Utils::tempdir_short;
+ $test_pghost =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+ }
$ENV{PGHOST} = $test_pghost;
$ENV{PGDATABASE} = 'postgres';
I wonder if we need a host2unix() helper accompanying perl2host()? Seems nicer
than sprinkling s!\\!/!g if $PostgreSQL::Test::Utils::windows_os in a growing
number of places...
Greetings,
Andres Freund
Hi, On 2021-12-08 17:03:07 -0800, Andres Freund wrote: > On 2021-12-08 16:36:14 -0800, Andres Freund wrote: > > On 2021-12-08 14:45:50 -0800, Andres Freund wrote: > > > Is it perhaps time to to use unix sockets on windows by default > > > (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? > > > > On its own PG_TEST_USE_UNIX_SOCKETS doesn't work at all on windows - it fails > > trying to use /tmp/ as a socket directory. Using PG_REGRESS_SOCK_DIR fixes > > that for PG_REGRESS. But the tap tests don't look at that :(. > > The tap failures in turn are caused by the Cluster.pm choosing a socket > directory with backslashes. Those backslashes are then treated as an escape > character both by guc.c and libpq. > > I think this can be addressed by something like > [...] That indeed seems to pass [1] where it previously failed [2]. There's another failure that I haven't diagnosed yet, but it's independent of tcp vs unix sockets. [1] https://cirrus-ci.com/task/5055530235330560?logs=ssl_test#L5 [2] https://cirrus-ci.com/task/5524596901281792?logs=ssl_test#L5 Greetings, Andres Freund
On Thu, Dec 9, 2021 at 11:46 AM Andres Freund <andres@anarazel.de> wrote: > Is it perhaps time to to use unix sockets on windows by default > (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? Makes sense. As a data point, it looks like this feature is in all supported releases of Windows. It arrived in 1803, already EOL'd, and IIUC even a Windows Server 2016 "LTSC" system that's been disconnected from the internet and refusing all updates reaches "mainstream EOL" next month. (Not a Windows person myself, but I've been looking at this stuff while contemplating various filesystem-related changes... there it's a little murkier, you need a WSL1-era kernel *and* you need to be running on top of local NTFS, which is harder for us to expect.) https://docs.microsoft.com/en-us/lifecycle/products/windows-10-enterprise-and-education https://docs.microsoft.com/en-us/windows-server/get-started/windows-server-release-info https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions
On 09.12.21 03:44, Thomas Munro wrote: > On Thu, Dec 9, 2021 at 11:46 AM Andres Freund<andres@anarazel.de> wrote: >> Is it perhaps time to to use unix sockets on windows by default >> (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? Makes sense to get this to work, at least as an option. > Makes sense. As a data point, it looks like this feature is in all > supported releases of Windows. It arrived in 1803, already EOL'd, and > IIUC even a Windows Server 2016 "LTSC" system that's been disconnected > from the internet and refusing all updates reaches "mainstream EOL" > next month. I believe the "18" in "1803" refers to 2018. We have Windows buildfarm members that mention 2016 and 2017 in their title. Would those be in trouble?
On 12/9/21 08:35, Peter Eisentraut wrote: > On 09.12.21 03:44, Thomas Munro wrote: >> On Thu, Dec 9, 2021 at 11:46 AM Andres Freund<andres@anarazel.de> >> wrote: >>> Is it perhaps time to to use unix sockets on windows by default >>> (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? > > Makes sense to get this to work, at least as an option. > >> Makes sense. As a data point, it looks like this feature is in all >> supported releases of Windows. It arrived in 1803, already EOL'd, and >> IIUC even a Windows Server 2016 "LTSC" system that's been disconnected >> from the internet and refusing all updates reaches "mainstream EOL" >> next month. > > I believe the "18" in "1803" refers to 2018. We have Windows > buildfarm members that mention 2016 and 2017 in their title. Would > those be in trouble? Probably not if they have been updated. I have Windows machines substantially older that 2018 but now running versions dated later. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 12/8/21 20:03, Andres Freund wrote: > > I wonder if we need a host2unix() helper accompanying perl2host()? Seems nicer > than sprinkling s!\\!/!g if $PostgreSQL::Test::Utils::windows_os in a growing > number of places... > Probably a good idea. I would call it canonical_path or something like that. / works quite happily as a path separator in almost all contexts on Windows - there are a handful of command line programs that choke on it - but sometimes you need to quote the path. WHen I recently provided for cross version upgrade testing on MSVC builds I just quoted everything. On Unix/Msys the shell just removes the quotes. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Hi, On 2021-12-09 14:35:37 +0100, Peter Eisentraut wrote: > On 09.12.21 03:44, Thomas Munro wrote: > > On Thu, Dec 9, 2021 at 11:46 AM Andres Freund<andres@anarazel.de> wrote: > > > Is it perhaps time to to use unix sockets on windows by default > > > (i.e. PG_TEST_USE_UNIX_SOCKETS), at least when on a new enough windows? > > Makes sense to get this to work, at least as an option. With https://github.com/anarazel/postgres/commit/046203741803da863f6129739fd215f8a32ec357 all tests pass. pg_regress requires PG_REGRESS_SOCK_DIR because it checks for TMPDIR, but windows only has TMP and TEMP. > > Makes sense. As a data point, it looks like this feature is in all > > supported releases of Windows. It arrived in 1803, already EOL'd, and > > IIUC even a Windows Server 2016 "LTSC" system that's been disconnected > > from the internet and refusing all updates reaches "mainstream EOL" > > next month. > > I believe the "18" in "1803" refers to 2018. We have Windows buildfarm > members that mention 2016 and 2017 in their title. Would those be in > trouble? Perhaps it could make sense to print the windows version somewhere as part of a windows build? Perhaps in the buildfarm client? Seems like it could be generally useful, outside of this specific issue. The most minimal thing would be to just print cmd /c ver or such. systeminfo.exe output could also be useful, but has a bit of runtime (1.5s on my windows VM). Greetings, Andres Freund
On 09.12.21 19:41, Andres Freund wrote: > Withhttps://github.com/anarazel/postgres/commit/046203741803da863f6129739fd215f8a32ec357 > all tests pass. pg_regress requires PG_REGRESS_SOCK_DIR because it checks for > TMPDIR, but windows only has TMP and TEMP. This looks reasonable so far. The commit messages 8f3ec75de4060d86176ad4ac998eeb87a39748c2 and 1d53432ff940b789c2431ba476a2a6e2db3edf84 contain some notes about what I thought at the time didn't work yet. In particular, the pg_upgrade tests don't support the use of Unix sockets on Windows. (Those tests have been rewritten since, so I don't know what the status is.) Also, the comment in pg_regress.c at remove_temp() is still there. These things should probably be addressed before we can consider making this the default.
Hi, On 2021-12-10 10:22:13 +0100, Peter Eisentraut wrote: > On 09.12.21 19:41, Andres Freund wrote: > > Withhttps://github.com/anarazel/postgres/commit/046203741803da863f6129739fd215f8a32ec357 > > all tests pass. pg_regress requires PG_REGRESS_SOCK_DIR because it checks for > > TMPDIR, but windows only has TMP and TEMP. > > This looks reasonable so far. I pushed that part, since we clearly need something like them. > The commit messages 8f3ec75de4060d86176ad4ac998eeb87a39748c2 and > 1d53432ff940b789c2431ba476a2a6e2db3edf84 contain some notes about what I > thought at the time didn't work yet. > In particular, the pg_upgrade tests > don't support the use of Unix sockets on Windows. (Those tests have been > rewritten since, so I don't know what the status is.) ISTM we still use two different implementations of the pg_upgrade tests :(. I recall there being some recent-ish work on moving it to be a tap test, but apparently not yet committed. It doesn't look like the vcregress.pl implementation respects PG_TEST_USE_UNIX_SOCKETS right now. > pg_regress.c at remove_temp() is still there. These things should probably > be addressed before we can consider making this the default. Hm, not immediately obvious what to do about this. Do you know if windows has restrictions around the length of unix domain sockets? If not, I wonder if it could be worth using the data directory as the socket path on windows instead of the separate temp directory? Greetings, Andres Freund
On 13.12.21 20:33, Andres Freund wrote: >> pg_regress.c at remove_temp() is still there. These things should probably >> be addressed before we can consider making this the default. > > Hm, not immediately obvious what to do about this. Do you know if windows has > restrictions around the length of unix domain sockets? If not, I wonder if it > could be worth using the data directory as the socket path on windows instead > of the separate temp directory? According to src/include/port/win32.h, it's the same 108 or so bytes that everyone else uses. That might not be the kernel limit, however, just the way the external API is defined. After the reading the material again, I think that comment might be overly cautious. The list of things not to do in a signal handler on Windows is not significantly different than those for say Linux, yet we do a lot of them anyway. I'm tempted to just ignore the advice and do it anyway, while ignoring errors, which is what it already does.