Re: [PATCH] Allow Postgres to pick an unused port to listen

Hi Tom,

Thank you for your feedback. Below are my comments.

On Wed, Mar 29, 2023 at 6:55 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Yurii Rashkovskii <yrashk(at)gmail(dot)com> writes:
> > I would like to suggest a patch against master (although it may be worth
> > backporting it) that makes it possible to listen on any unused port.
>
> I think this is a bad idea, mainly because this:
>
> > Instead, with this patch, one can specify `port` as `0` (the "wildcard"
> > port) and retrieve the assigned port from postmaster.pid
>
> is a horrid way to find out what was picked, and yet there could
> be no other.
>

Can you elaborate on why reading postmaster.pid is a horrid way to discover
the port, given that it is a pretty simple format with a fixed line number
for the port?

> Our existing design for this sort of thing is to let the testing
> framework choose the port, and I don't really see what's wrong
> with that approach. Yes, I know it's theoretically subject to
> race conditions, but that hasn't seemed to be a problem in
> practice. It's especially not a problem given that modern
> testing practice tends to not open any TCP port at all, just
> a Unix socket in a test-private directory, so that port
> conflicts are a non-issue.
>

I keep running into this race condition nearly daily, which is why I
proposed to address it with this patch. Yes, I know that one can get around
this with UNIX sockets,
but they have limited capabilities (not accessible outside of the local
machine, to begin with). Here's a real-world example of why I need to be
able to use TCP ports:

I have an extension that allows managing the lifecycle of [Docker/OCI]
containers, and it passes Postgres connection details to these containers
as environment variables.
These containers can now connect to Postgres using any program that can
communicate using the wire protocol. I test this functionality in an
automated test that is executed
concurrently with others. Testing that the extension can supply the correct
connection information to the containers is important.

If we forget the importance of testing this specific part of the
functionality for a bit, the rest of my issue can be _theoretically_
resolved by passing the UNIX socket in `PGHOST` instead.

However, it won't work in a typical Docker Desktop for macOS setup as it
utilizes a virtual machine, and therefore, I can't simply use a UNIX socket
between them.

Here's an example:

1. Create a UNIX socket listener:

```
socat unix-l:test.sock,fork system:'echo hello'
```

2. Verify that it works locally:

```
$ socat test.sock -
hello
```

3. Now, while being on macOS and using Docker Desktop, let Docker mount the
directory with the socket and try to connect it from there:

```
$ docker run -v /path/to/sock/dir:/sock -ti ubuntu bash
# apt update && apt install -y socat
# socat /sock/test.sock -
2023/03/29 23:34:48 socat[451] E connect(5, AF=1 "/sock/test.sock", 17):
Connection refused
```

I get that the UNIX socket around works for many cases, but it does not
work for mine. Hence the proposal. Allowing a (fairly common) practice of a
wildcard port with the discovery of it via
postmaster.pid resolves all the above concerns without having to resort to
a rather race-condition-prone way to pick a port (or a complicated way to
do so with proper locking).

--
http://omnigres.org
Yurii