Обсуждение: [PATCH] Allow Postgres to pick an unused port to listen

Поиск
Список
Период
Сортировка

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

От
Yurii Rashkovskii
Дата:
Hi,

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.

The main motivation is running colocated instances of Postgres (such as test benches) without having to coordinate port allocation in an unnecessarily complicated way.

Instead, with this patch, one can specify `port` as `0` (the "wildcard" port) and retrieve the assigned port from postmaster.pid

I believe there is no significant performance or another impact as it is a tiny bit of conditional functionality executed during startup.

The patch builds and `make check` succeeds. The patch does not add a test; however, I am trying to figure out if this behaviour can be tested automatically.
Вложения

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

От
Tom Lane
Дата:
Yurii Rashkovskii <yrashk@gmail.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.

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.

            regards, tom lane



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

От
Yurii Rashkovskii
Дата:
Hi Tom,

Thank you for your feedback. Below are my comments.

On Wed, Mar 29, 2023 at 6:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yurii Rashkovskii <yrashk@gmail.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). 

--

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

От
Yurii Rashkovskii
Дата:
Hi Tom,

On Wed, Mar 29, 2023 at 6:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yurii Rashkovskii <yrashk@gmail.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.

I answered you before (https://www.postgresql.org/message-id/CA+RLCQwYw-Er-E_RGNCDfA514w+1YL8HGhNstxX=y1gLAABFdA@mail.gmail.com), but I am wondering whether you missed that response. I would really be interested to learn why you think reading port from the pid file is a "horrid way" to find out what was picked.

I've outlined my reasoning for this feature in detail in the referenced message. Hope you can consider it.

--
http://omnigres.org
Yurii

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

От
Andrew Dunstan
Дата:


On 2023-03-29 We 07:55, Tom Lane wrote:
Yurii Rashkovskii <yrashk@gmail.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.

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.


For TAP tests we have pretty much resolved the port collisions issue for TCP ports too. See commit 9b4eafcaf4

Perhaps the OP could adapt that logic to his use case.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

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

От
Yurii Rashkovskii
Дата:
Hi Andrew,

On Fri, Apr 7, 2023, 7:07 p.m. Andrew Dunstan <andrew@dunslane.net> wrote:


On 2023-03-29 We 07:55, Tom Lane wrote:
Yurii Rashkovskii <yrashk@gmail.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.

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.


For TAP tests we have pretty much resolved the port collisions issue for TCP ports too. See commit 9b4eafcaf4

Perhaps the OP could adapt that logic to his use case.


Thank you for referencing this commit. The point why I am suggesting my patch is that I believe that my solution is a much better way to avoid collisions in the first place. Implementing an algorithm similar to the one in the referenced commit is error-pfone and can be difficult in environments like shell script.

I'm trying to understand what's wrong with reading port from the pid file (if Postgres writes information there, it's surely so that somebody can read it, otherwise, why write it in the first placd)? The proposed solution uses operating system's functionality to achieve collision-free mechanics with none of the complexity introduced in the commit.

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

От
Robert Haas
Дата:
On Fri, Apr 7, 2023 at 5:34 PM Yurii Rashkovskii <yrashk@gmail.com> wrote:
> I'm trying to understand what's wrong with reading port from the pid file (if Postgres writes information there, it's
surelyso that somebody can read it, otherwise, why write it in the first placd)? The proposed solution uses operating
system'sfunctionality to achieve collision-free mechanics with none of the complexity introduced in the commit. 

I agree. We don't document the exact format of the postmaster.pid file
to my knowledge, but storage.sgml lists all the things it contains,
and runtime.sgml documents that the first line contains the postmaster
PID, so this is clearly not some totally opaque file that nobody
should ever touch. Consequently, I don't agree with Tom's statement
that this would be a "a horrid way to find out what was picked." There
is some question in my mind about whether this is a feature that we
want PostgreSQL to have, and if we do want it, there may be some room
for debate about how it's implemented, but I reject the idea that
extracting the port number from postmaster.pid is intrinsically a
terrible plan. It seems like a perfectly reasonable plan.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

От
Yurii Rashkovskii
Дата:
Hi Robert,

On Tue, Apr 11, 2023 at 12:54 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Apr 7, 2023 at 5:34 PM Yurii Rashkovskii <yrashk@gmail.com> wrote:
> I'm trying to understand what's wrong with reading port from the pid file (if Postgres writes information there, it's surely so that somebody can read it, otherwise, why write it in the first placd)? The proposed solution uses operating system's functionality to achieve collision-free mechanics with none of the complexity introduced in the commit.

I agree. We don't document the exact format of the postmaster.pid file
to my knowledge, but storage.sgml lists all the things it contains,
and runtime.sgml documents that the first line contains the postmaster
PID, so this is clearly not some totally opaque file that nobody
should ever touch. Consequently, I don't agree with Tom's statement
that this would be a "a horrid way to find out what was picked." There
is some question in my mind about whether this is a feature that we
want PostgreSQL to have, and if we do want it, there may be some room
for debate about how it's implemented, but I reject the idea that
extracting the port number from postmaster.pid is intrinsically a
terrible plan. It seems like a perfectly reasonable plan.


I appreciate your support on the pid file concern. What questions do you have about this feature with regard to its desirability and/or implementation? I'd love to learn from your insight and address any of those if I can.

--
Y.

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

От
Robert Haas
Дата:
On Tue, Apr 11, 2023 at 10:17 PM Yurii Rashkovskii <yrashk@gmail.com> wrote:
> I appreciate your support on the pid file concern. What questions do you have about this feature with regard to its
desirabilityand/or implementation? I'd love to learn from your insight and address any of those if I can. 

I don't have any particularly specific concerns. But, you know, if a
bunch of other people, especially people already known the community
showed up on this thread to say "hey, I'd like that too" or "that
would be better than what we have now," well then that would make me
think "hey, we should probably move forward with this thing." But so
far the only people to comment are Tom and Andrew. Tom, in addition to
complaining about the PID file thing, also basically said that the
feature didn't seem necessary to him, and Andrew's comments seem to me
to suggest the same thing. So it kind of seems like you've convinced
zero people that this is a thing we should have, and that's not very
many.

It happens from time to time on this mailing list that somebody shows
up to propose a feature where I say to myself "hmm, that doesn't sound
like an intrinsically terrible idea, but it sounds like it might be
specific enough that only the person proposing it would ever use it."
For instance, someone might propose a new backslash command for psql
that runs an SQL query that produces some output which that person
finds useful. There is no big design problem there, but psql is
already pretty cluttered with commands that look like line noise, so
we shouldn't add a new one on the strength of one person wanting it.
Each feature, even if it's minor, has some cost. New releases need to
keep it working, which may mean that it needs a test, and then the
test is another thing that you have to keep working, and it also takes
time to run every time anyone does make check-world. These aren't big
costs and don't set a high bar for adding new features, but they do
mean, at least IMHO, that one person wanting a feature that isn't
obviously of general utility is not good enough. I think all of that
also applies to this feature.

I haven't reviewed the code in detail. It at least has some style
issues, but worrying about that seems premature.

I mostly just wanted to say that I disagreed with Tom about the
particular point on postmaster.pid, without really expressing an
opinion about anything else.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

От
Andrew Dunstan
Дата:


On 2023-04-12 We 11:01, Robert Haas wrote:
On Tue, Apr 11, 2023 at 10:17 PM Yurii Rashkovskii <yrashk@gmail.com> wrote:
I appreciate your support on the pid file concern. What questions do you have about this feature with regard to its desirability and/or implementation? I'd love to learn from your insight and address any of those if I can.
I don't have any particularly specific concerns. But, you know, if a
bunch of other people, especially people already known the community
showed up on this thread to say "hey, I'd like that too" or "that
would be better than what we have now," well then that would make me
think "hey, we should probably move forward with this thing." But so
far the only people to comment are Tom and Andrew. Tom, in addition to
complaining about the PID file thing, also basically said that the
feature didn't seem necessary to him, and Andrew's comments seem to me
to suggest the same thing. So it kind of seems like you've convinced
zero people that this is a thing we should have, and that's not very
many.


Not quite, I just suggested looking at a different approach.  I'm not opposed to the idea in principle.

I agree with you that parsing the pid file shouldn't be hard or unreasonable.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

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

От
Mark Dilger
Дата:

> On Apr 12, 2023, at 8:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> "hey, I'd like that too"

I like the idea in principle.  I have been developing a testing infrastructure in my spare time and would rather not
duplicateAndrew's TAP logic.  If we get this pushed down into the server itself, all the test infrastructure can use a
single,shared solution. 

As for the implementation, I just briefly scanned the patch.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






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

От
Jacob Champion
Дата:
On Fri, Apr 7, 2023 at 5:07 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> For TAP tests we have pretty much resolved the port collisions issue for TCP ports too. See commit 9b4eafcaf4

The Cirrus config still has the following for the Windows tests:

    # Avoids port conflicts between concurrent tap test runs
    PG_TEST_USE_UNIX_SOCKETS: 1

Is that comment out of date, or would this proposal improve the
Windows situation too?

--Jacob



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

От
Greg Stark
Дата:
On Wed, 12 Apr 2023 at 11:02, Robert Haas <robertmhaas@gmail.com> wrote:
>
> I mostly just wanted to say that I disagreed with Tom about the
> particular point on postmaster.pid, without really expressing an
> opinion about anything else.

I don't object to using the pid file as the mechanism -- but it is a
bit of an awkward UI for shell scripting. I imagine it would be handy
if pg_ctl had an option to just print the port number so you could get
it with a simple port=`pg_ctl -D <dir> status-port`

-- 
greg



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

От
Robert Haas
Дата:
On Wed, Apr 12, 2023 at 1:31 PM Greg Stark <stark@mit.edu> wrote:
> I don't object to using the pid file as the mechanism -- but it is a
> bit of an awkward UI for shell scripting. I imagine it would be handy
> if pg_ctl had an option to just print the port number so you could get
> it with a simple port=`pg_ctl -D <dir> status-port`

That's not a bad idea, and would provide some additional isolation to
reduce direct dependency on the PID file format.

However, awk 'NR==4' $PGDATA/postmaster.pid is hardly insanity. If it
can be done with a 5-character awk script, it's not too hard. The kind
of thing you're talking about is much more important with things like
pg_control or postgresql.conf that have much more complicated formats.
The format of the PID file is intentionally simple. But that's not to
say that I'm objecting.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 12, 2023 at 1:31 PM Greg Stark <stark@mit.edu> wrote:
>> I don't object to using the pid file as the mechanism -- but it is a
>> bit of an awkward UI for shell scripting. I imagine it would be handy
>> if pg_ctl had an option to just print the port number so you could get
>> it with a simple port=`pg_ctl -D <dir> status-port`

> That's not a bad idea, and would provide some additional isolation to
> reduce direct dependency on the PID file format.

Yeah.  My main concern here is with limiting our ability to change
the pidfile format in future.  If we can keep the dependencies on that
localized to code we control, it'd be much better.

            regards, tom lane



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

От
Robert Haas
Дата:
On Wed, Apr 12, 2023 at 1:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah.  My main concern here is with limiting our ability to change
> the pidfile format in future.  If we can keep the dependencies on that
> localized to code we control, it'd be much better.

I don't know if it's considered officially supported, but I often use
pg_ctl stop on a directory without worrying about whether I'm doing it
with the same server version that's running in that directory. I'd be
reluctant to break that property. So I bet our ability to modify the
file format is already quite limited.

But again, no issue with having a way for pg_ctl to fish the
information out of there.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 12, 2023 at 1:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah.  My main concern here is with limiting our ability to change
>> the pidfile format in future.  If we can keep the dependencies on that
>> localized to code we control, it'd be much better.

> I don't know if it's considered officially supported, but I often use
> pg_ctl stop on a directory without worrying about whether I'm doing it
> with the same server version that's running in that directory. I'd be
> reluctant to break that property. So I bet our ability to modify the
> file format is already quite limited.

IMO, the only aspect we consider "officially supported" for outside
use is that the first line contains the postmaster's PID.  All the
rest is private (and has changed as recently as v10).  Without
having actually checked the code, I think that "pg_ctl stop" relies
only on that aspect, or at least it could be made to do so at need.
So I think your example would survive other changes in the format.

I don't really want external code knowing that line 4 is the port,
because I foresee us breaking that someday --- what will happen
when we want to allow one postmaster to support multiple ports?
Maybe we'll decide that we don't have to reflect that in the
pidfile, but let's not constrain our decisions ahead of time.

            regards, tom lane



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

От
Michael Paquier
Дата:
On Wed, Apr 12, 2023 at 02:24:30PM -0400, Tom Lane wrote:
> I don't really want external code knowing that line 4 is the port,
> because I foresee us breaking that someday --- what will happen
> when we want to allow one postmaster to support multiple ports?
> Maybe we'll decide that we don't have to reflect that in the
> pidfile, but let's not constrain our decisions ahead of time.

In the same fashion as something mentioned upthread, the format
portability would not matter much if all the information from the PID
file is wrapped around a pg_ctl command or something equivalent that
controls its output format, say:
pg_ctl -D $PGDATA --format={json,what_you_want} postmaster_file

To be more precise, storage.sgml documents the format of the PID file
in what seems like the correct order for each item, some of them being
empty depending on the setup.
--
Michael

Вложения

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

От
Yurii Rashkovskii
Дата:
Tom, Robert, Greg, Andrew,

On Thu, Apr 13, 2023 at 12:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 12, 2023 at 1:31 PM Greg Stark <stark@mit.edu> wrote:
>> I don't object to using the pid file as the mechanism -- but it is a
>> bit of an awkward UI for shell scripting. I imagine it would be handy
>> if pg_ctl had an option to just print the port number so you could get
>> it with a simple port=`pg_ctl -D <dir> status-port`

> That's not a bad idea, and would provide some additional isolation to
> reduce direct dependency on the PID file format.

Yeah.  My main concern here is with limiting our ability to change
the pidfile format in future.  If we can keep the dependencies on that
localized to code we control, it'd be much better.


Thank you all for the feedback. It's quite useful. I think it is important to separate this into two concerns:

1. Letting Postgres pick an unused port.
2. Retrieving the port it picked.

If I get this right, there's no significant opposition to (1) as this is common functionality we're relying on. The most contention is around (2) because I suggested using postmaster.pid
file, which may be considered private for the most part, at least for the time being.

With this in mind, I still think that proceeding with (1) is a good idea, as retrieving the port being listened on is still much easier than involving a more complex lock file script. For example, on UNIX-like systems, `lsof` can be typically used to do this:

```
# For IPv4
lsof  -a -w -FPn -p $(head -n 1 postmaster.pid) -i4TCP -sTCP:LISTEN -P -n | tail -n 1 | awk -F: '{print $NF}'
# For IPv6
lsof  -a -w -FPn -p $(head -n 1postmaster.pid) -i6TCP -sTCP:LISTEN -P -n | tail -n 1 | awk -F: '{print $NF}'
```

(There are also other tools that can be used to achieve much of the same)

On Windows, this can be done using PowerShell (and perhaps netstat, too):

```
# IPv4
PS> Get-NetTCPConnection -State Listen -OwningProcess (Get-Content "postmaster.pid" -First 1) | Where-Object { $_.LocalAddress -notmatch ':' } | Select-Object -ExpandProperty LocalPort
5432
PS> Get-NetTCPConnection -State Listen -OwningProcess (Get-Content "postmaster.pid" -First 1) | Where-Object { $_.LocalAddress -match ':' } | Select-Object -ExpandProperty LocalPort
5432
```

The above commands can be worked on to extract multiple ports should that ever become a feature.

The bottom line is this decouples (1) from (2), and we can resolve them separately if there's too much (understandable) hesitation to commit to a particular approach to it (documenting postmaster.pid, changing its format, amending pg_ctl functionality, etc.) I will be happy to participate in the discovery and resolution of (2) as well.

This would allow people like myself or Mark (above in the thread) to let Postgres pick the unused port and extract it using a oneliner for the time being. When a better approach for server introspection will be agreed on, we can use that.

I'll be happy to address any [styling or other] issues with the currently proposed patch. 


--

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

От
Tom Lane
Дата:
Yurii Rashkovskii <yrashk@gmail.com> writes:
> Thank you all for the feedback. It's quite useful. I think it is important
> to separate this into two concerns:

> 1. Letting Postgres pick an unused port.
> 2. Retrieving the port it picked.

Yeah, those are distinguishable implementation concerns, but ...

> The bottom line is this decouples (1) from (2), and we can resolve them
> separately if there's too much (understandable) hesitation to commit to a
> particular approach to it (documenting postmaster.pid, changing its format,
> amending pg_ctl functionality, etc.)

... AFAICS, there is exactly zero value in committing a solution for (1)
without also committing a solution for (2).  I don't think any of the
alternative methods you proposed are attractive or things we should
recommend.

            regards, tom lane



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

От
Yurii Rashkovskii
Дата:
Hi Tom,

On Thu, Apr 13, 2023 at 9:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yurii Rashkovskii <yrashk@gmail.com> writes:
> Thank you all for the feedback. It's quite useful. I think it is important
> to separate this into two concerns:

> 1. Letting Postgres pick an unused port.
> 2. Retrieving the port it picked.

Yeah, those are distinguishable implementation concerns, but ...

> The bottom line is this decouples (1) from (2), and we can resolve them
> separately if there's too much (understandable) hesitation to commit to a
> particular approach to it (documenting postmaster.pid, changing its format,
> amending pg_ctl functionality, etc.)

... AFAICS, there is exactly zero value in committing a solution for (1)
without also committing a solution for (2).  I don't think any of the
alternative methods you proposed are attractive or things we should
recommend.

I disagree that zero value exists in (1) without (2). As my examples show, they make it possible to pick a port without synchronization scripting. Are they perfect? Of course, not. But they are better than lock file-based scripts IMO. They are not exposed to race conditions.

But getting your agreement is important to get this in; I am willing to play along and resolve both (1) and (2) in one go. As for the implementation approach for (2), which of the following options would you prefer?

a) Document postmaster.pid as it stands today
b) Expose the port number through pg_ctl (*my personal favorite)
c) Redesign its content below line 1 to make it extensible (make unnamed lines named, for example)

If none of the above options suit you, do you have a strategy you'd prefer?

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

От
Peter Eisentraut
Дата:
On 13.04.23 04:45, Yurii Rashkovskii wrote:
> But getting your agreement is important to get this in; I am willing to 
> play along and resolve both (1) and (2) in one go. As for the 
> implementation approach for (2), which of the following options would 
> you prefer?
> 
> a) Document postmaster.pid as it stands today
> b) Expose the port number through pg_ctl (*my personal favorite)
> c) Redesign its content below line 1 to make it extensible (make unnamed 
> lines named, for example)
> 
> If none of the above options suit you, do you have a strategy you'd prefer?

You could just drop another file into the data directory that just 
contains the port number ($PGDATA/port).  However, if we ever do 
multiple ports, that would still require a change in the format of that 
file, so I don't know if that's actually better than a).

I don't think involving pg_ctl is necessary or desirable, since it would 
make any future changes like that even more complicated.



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

От
Stephen Frost
Дата:
Greetings,

* Peter Eisentraut (peter.eisentraut@enterprisedb.com) wrote:
> On 13.04.23 04:45, Yurii Rashkovskii wrote:
> > But getting your agreement is important to get this in; I am willing to
> > play along and resolve both (1) and (2) in one go. As for the
> > implementation approach for (2), which of the following options would
> > you prefer?
> >
> > a) Document postmaster.pid as it stands today
> > b) Expose the port number through pg_ctl (*my personal favorite)
> > c) Redesign its content below line 1 to make it extensible (make unnamed
> > lines named, for example)
> >
> > If none of the above options suit you, do you have a strategy you'd prefer?
>
> You could just drop another file into the data directory that just contains
> the port number ($PGDATA/port).  However, if we ever do multiple ports, that
> would still require a change in the format of that file, so I don't know if
> that's actually better than a).

If we did a port per line then it wouldn't be changing the format of the
first line, so that might not be all that bad.

> I don't think involving pg_ctl is necessary or desirable, since it would
> make any future changes like that even more complicated.

I'm a bit confused by this- if pg_ctl is invoked then we have
more-or-less full control over parsing and reporting out the answer, so
while it might be a bit more complicated for us, it seems surely simpler
for the end user.  Or maybe you're referring to something here that I'm
not thinking of?

Independent of the above though ... this hand-wringing about what we
might do in the relative near-term when we haven't done much in the past
many-many years regarding listen_addresses or port strikes me as
unlikely to be necessary.  Let's pick something and get it done and
accept that we may have to change it at some point in the future, but
that's kinda what major releases are for, imv anyway.

Thanks!

Stpehen

Вложения

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

От
Yurii Rashkovskii
Дата:
Stephen,

> You could just drop another file into the data directory that just contains
> the port number ($PGDATA/port).  However, if we ever do multiple ports, that
> would still require a change in the format of that file, so I don't know if
> that's actually better than a).

I find it difficult to get anything done under the restriction of "what if we ever need to change X?" as it is difficult to address something that doesn't exist or hasn't been planned.

A fine and delicate balance of anticipating what may happen theoretically and what's more likely happen is an art. It's also important to consider the impact of a breaking change. It's one thing if we have to break, say, an SQL function signature or SQL syntax itself, and another if it is a relatively small feature related to the administration of a server (in this case, more like scripting a test bench).
 

If we did a port per line then it wouldn't be changing the format of the
first line, so that might not be all that bad.

If we consider this path, then (if we assume the format of the file is still to be private), we can make the port line accept multiple ports using a delimiter like `:` so that the next line still remains the same. 

That being said, if the format is private to Postgres, it's all minor considerations.


> I don't think involving pg_ctl is necessary or desirable, since it would
> make any future changes like that even more complicated.

I'm a bit confused by this- if pg_ctl is invoked then we have
more-or-less full control over parsing and reporting out the answer, so
while it might be a bit more complicated for us, it seems surely simpler
for the end user.  Or maybe you're referring to something here that I'm
not thinking of?

I would love to learn about this as well.
 

Independent of the above though ... this hand-wringing about what we
might do in the relative near-term when we haven't done much in the past
many-many years regarding listen_addresses or port strikes me as
unlikely to be necessary.  Let's pick something and get it done and
accept that we may have to change it at some point in the future, but
that's kinda what major releases are for, imv anyway.

That's how I see it, too. I tried to make this change as small as possible to appreciate the fact that all of this may change one day if or when that portion of Postgres will be due for a major redesign. I'd be happy to contribute to that process, but in the meantime, I am looking for the simplest reasonable way to achieve a relatively specific use case. 

Personally, I am fine with reading the `.pid` file and accepting that it _may_ change in the future; I am also fine with amending the patch to add functionality to pg_ctl or adding a new file.

To keep everybody's cognitive load low, I'd rather not flood the thread with multiple alternative implementations (unless that's desirable) and just go for something we can agree on.

(I consider this feature so small that it doesn't deserve such a lengthy discussion. However, I also get Tom's point about how we document this feature's use, which is very valid and valuable. If it was up to me entirely, I'd probably just document `postmaster.pid` and call it a day. If it ever breaks, that's a major release territory. Otherwise, amending `pg_ctl` to access information like this in a uniform way is also a good approach if we want to keep the format of the pid file private.)
 
--
Y.

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

От
Aleksander Alekseev
Дата:
Hi,

Here are my two cents.

> > 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.

What personally I dislike about this approach is the fact that it is
not guaranteed to work in the general case.

Let's say the test framework started Postgres on a random port. Then
the framework started to do something else, building a Docker
container for instance. While the framework is busy PostgreSQL crashes
(crazy, I know, but not impossible). Both PID and the port will be
reused eventually by another process. How soon is the implementation
detail of the given OS and its setting.

A bullet-proof approach would be (approximately) for the test
framework to lease the ports on the given machine, for instance by
using a KV value with CAS support like Consul or etcd (or another
PostgreSQL instance), as this is done for leader election in
distributed systems (so called leader lease). After leasing the port
the framework knows no other testing process on the given machine will
use it (and also it keeps telling the KV storage that the port is
still leased) and specifies it in postgresql.conf as usual.

I realize this is a complicated problem to solve in a general case,
but it doesn't look like the proposed patch is the right solution for
the named problem.

-- 
Best regards,
Aleksander Alekseev



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

От
Yurii Rashkovskii
Дата:
Alexander,

On Wed, Apr 19, 2023 at 11:44 PM Aleksander Alekseev <aleksander@timescale.com> wrote:
Hi,

Here are my two cents.

> > 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.

What personally I dislike about this approach is the fact that it is
not guaranteed to work in the general case.

Let's say the test framework started Postgres on a random port. Then
the framework started to do something else, building a Docker
container for instance. While the framework is busy PostgreSQL crashes
(crazy, I know, but not impossible). Both PID and the port will be
reused eventually by another process. How soon is the implementation 
detail of the given OS and its setting.

Let's say Postgres crashed, and the port was not reused. In this case, the connection will fail. The test bench script can then, at the very least, try checking the log files to see if there's any indication of a crash there and report if one occurred. If the port was reused by something other than Postgres, the script should (ideally) fail to communicate with it using Postgres protocol.  If it was reused by another Postgres instance, it gets a bit tougher, but then the test bench can, upon connection, verify that it is the same system by comparing the system identifier on the file system (retrieved using pg_controldata) and over the wire (retrieved using `select system_identifier from pg_control_system()`)

I also suspect that this problem has a bigger scope than port retrieval. If one is to use postmaster.pid only for PID retrieval, then there's still no guarantee that between the time we retrieved the PID from the file and used it,
Postgres didn't crash, and the PID was not re-used by a different process, potentially even another postgres process launched in parallel by the test bench.

There are tools mentioned previously by me in the thread that allow inspecting which ports are opened by a given PID, and one can use those to provide an extra determination as to whether we're still on the right track. These tools
can also tell us what is the process name.

Ultimately, there's no transactionality in POSIX API, so we're always exposed to the chance of discrepancies between the inspection time and the next step.

A bullet-proof approach would be (approximately) for the test
framework to lease the ports on the given machine, for instance by
using a KV value with CAS support like Consul or etcd (or another
PostgreSQL instance), as this is done for leader election in
distributed systems (so called leader lease). After leasing the port
the framework knows no other testing process on the given machine will
use it (and also it keeps telling the KV storage that the port is
still leased) and specifies it in postgresql.conf as usual.

The approach you suggest introduces a significant amount of complexity but seemingly fails to address one of the core issues: using a KV store to lease a port does not guarantee the port's availability. I don't believe this is a sound way to address this issue, let alone a bulletproof one.

Also, I don't think there's a case for distributed systems here because we're only managing a single computer's resource: the allocation of local ports.

If I were to go for a more bulletproof approach, I would probably consider a different technique that would not necessitate provisioning and running additional software for port leasing. 

For example, I'd suggest adding an option to Postgres to receive sockets it should listen on from a UNIX socket (using SCM_RIGHTS message) and then have another program acquire the sockets using whatever algorithm (picking pre-set one, unused wildcard port, etc.) and start Postgres passing the sockets using the aforementioned UNIX socket. This program will be your leaseholder and can perhaps print out the PID so that the testing scripts can immediately use it. The leaseholder should watch for the Postgres process to crash. This is still a fairly complicated solution that needs some refining, but it does allocate ports flawlessly, relying on OS being the actual leaseholder and not requiring fighting against race conditions. I didn't go for anything like this because of the sheer complexity of it.

The proposed solution is, I believe, a simple one that gets you there in an awful majority of cases. If one starts running out in the error cases like port reuse or listener disappearance, the logic I described above may get them a step further. 
 

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

От
Denis Laxalde
Дата:
Hi,

Yurii Rashkovskii a écrit :
> On Wed, Apr 19, 2023 at 11:44 PM Aleksander Alekseev <
> aleksander@timescale.com> wrote:
>>>> 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.
[...]
>> A bullet-proof approach would be (approximately) for the test
>> framework to lease the ports on the given machine, for instance by
>> using a KV value with CAS support like Consul or etcd (or another
>> PostgreSQL instance), as this is done for leader election in
>> distributed systems (so called leader lease). After leasing the port
>> the framework knows no other testing process on the given machine will
>> use it (and also it keeps telling the KV storage that the port is
>> still leased) and specifies it in postgresql.conf as usual.
>>
> 
> The approach you suggest introduces a significant amount of complexity but
> seemingly fails to address one of the core issues: using a KV store to
> lease a port does not guarantee the port's availability. I don't believe
> this is a sound way to address this issue, let alone a bulletproof one.
> 
> Also, I don't think there's a case for distributed systems here because
> we're only managing a single computer's resource: the allocation of local
> ports.

For this (local computer) use case, a tool such as 
https://github.com/kmike/port-for/ would do the job if I understand 
correctly (the lease thing, locally). And it would work for "anything", 
not just Postgres.

I am curious, Yurii, is Postgres the only service that need an unused 
port for listening in your testing/application environment? Otherwise, 
how is this handled in other software?

Cheers,
Denis



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

От
Aleksander Alekseev
Дата:
Hi,

> Also, I don't think there's a case for distributed systems here because we're only managing a single computer's
resource:the allocation of local ports.
 

I don't suggest building a distributed system but rather using
well-known solutions from this area. For the described case the
"resource manager" will be as simple a single Consul instance (a
single binary file, since Consul is written in Go) running locally.
The "complexity" would be for the test framework to use a few extra
REST queries. Arguably not that complicated.

> using a KV store to lease a port does not guarantee the port's availability

I assume you don't have random processes doing random things (like
listening random ports) on a CI machine. You know that certain ports
are reserved for the tests and are going to be used only for this
purpose using the same leasing protocol.

If there are random things happening on CI you have no control of, you
are having a problem with the CI infrastructure, not with Postgres.

> For example, I'd suggest adding an option to Postgres to receive sockets it should listen [...]

Not sure if I fully understood the idea, but it looks like you are
suggesting to build in certain rather complicated functionality for an
arguably rare use case so that a QA engineer didn't have one extra
small dependency to worry about in this rare case. I'm quite skeptical
that this is going to happen.

> I am curious, Yurii, is Postgres the only service that need an unused
> port for listening in your testing/application environment? Otherwise,
> how is this handled in other software?

That's a very good point.

To clarify, there is nothing wrong with the patch per se. It's merely
an unreliable solution for a problem it is supposed to address. I
don't think we should encourage the users to build unreliable systems.

-- 
Best regards,
Aleksander Alekseev



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

От
Yurii Rashkovskii
Дата:
Aleksander,

On Thu, Apr 20, 2023 at 1:22 PM Aleksander Alekseev <aleksander@timescale.com> wrote:
Hi,

> Also, I don't think there's a case for distributed systems here because we're only managing a single computer's resource: the allocation of local ports.

I don't suggest building a distributed system but rather using
well-known solutions from this area. For the described case the
"resource manager" will be as simple a single Consul instance (a
single binary file, since Consul is written in Go) running locally.
The "complexity" would be for the test framework to use a few extra
REST queries. Arguably not that complicated.

Bringing in a process that works over REST API (requiring itself to have a port, by the way) and increasing the rollout of such an environment is antithetical to simplicity
and, thus, will make it only worse. If this is the alternative, I'd rather have a few retries or some other small hacks.

Bringing in a new dependency with Python is also a heavy solution I'd rather avoid. I find that this is rather a problem with a relatively small surface. If the patch won't go through,
I'll just find a workaround to live with, but I'd rather stay away from blowing the development environment out of proportion for something so minuscule.
 

> using a KV store to lease a port does not guarantee the port's availability

I assume you don't have random processes doing random things (like
listening random ports) on a CI machine. You know that certain ports
are reserved for the tests and are going to be used only for this
purpose using the same leasing protocol.

This is intended to be used by CI and development workstations, where all bets are kind of off.
 

> For example, I'd suggest adding an option to Postgres to receive sockets it should listen [...]

Not sure if I fully understood the idea, but it looks like you are
suggesting to build in certain rather complicated functionality for an
arguably rare use case so that a QA engineer didn't have one extra
small dependency to worry about in this rare case. I'm quite skeptical
that this is going to happen.

My suggestion was to simply allow listening for a wildcard port and be able to read it out in some way. Nothing particularly complicated. The fact that the process may die before it is connected to is rather a strange argument as the same can happen outside of this use case.


--
Y.

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

От
Peter Eisentraut
Дата:
On 19.04.23 06:21, Stephen Frost wrote:
>> I don't think involving pg_ctl is necessary or desirable, since it would
>> make any future changes like that even more complicated.
> I'm a bit confused by this- if pg_ctl is invoked then we have
> more-or-less full control over parsing and reporting out the answer, so
> while it might be a bit more complicated for us, it seems surely simpler
> for the end user.  Or maybe you're referring to something here that I'm
> not thinking of?

Getting pg_ctl involved just requires a lot more work.  We need to write 
actual code, documentation, tests, help output, translations, etc.  If 
we ever change anything, then we need to transition the command-line 
arguments somehow, add more documentation, etc.

A file is a much simpler interface: You just write to it, write two 
sentences of documentation, that's all.

Or to put it another way, if we don't think a file is an appropriate 
interface, then why is a PID file appropriate?

> Independent of the above though ... this hand-wringing about what we
> might do in the relative near-term when we haven't done much in the past
> many-many years regarding listen_addresses or port strikes me as
> unlikely to be necessary.  Let's pick something and get it done and
> accept that we may have to change it at some point in the future, but
> that's kinda what major releases are for, imv anyway.

Right.  I'm perfectly content with just allowing port number 0 and 
leaving it at that.




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

От
Cary Huang
Дата:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, failed
Spec compliant:           not tested
Documentation:            not tested

Hello

This is one of those features that is beneficial to a handful of people in specific test cases. It may not benefit the
majorityof the users but is certainly not useless either. As long as it can be disabled and enough tests have been run
toensure it won't have a significant impact on working components while disabled, it should be fine in my opinion.
Regardingwhere the selected port shall be saved (postmaster.pid, read by pg_ctl or saved in a dedicated file), I see
thatpostmaster.pid already contains a port number in line number 4, so adding a port number into there is nothing new;
portnumber is already there and we can simply replace the port number with the one selected by the system. 
 

I applied and tested the patch and found that the system can indeed start when port is set to 0, but line 4 of
postmaster.piddoes not store the port number selected by the system, rather, it stored 0, which is the same as
configured.So I am actually not able to find out the port number that my PG is running on, at least not in a
straight-forwardway. 
 

thank you
==================
Cary Huang
HighGo Software
www.highgo.ca

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

От
Yurii Rashkovskii
Дата:
Hi Cary,

Thank you so much for the review. It's very valuable, and you caught an important issue with it that I somehow missed (not updating the .pid file with the selected port number). I'm not sure how it escaped me (perhaps I was focusing too much on the log file to validate the behaviour).

I've amended the patch to ensure the port number is in the lock file. I've attached V2.

Yurii


On Sat, May 6, 2023 at 12:31 AM Cary Huang <cary.huang@highgo.ca> wrote:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, failed
Spec compliant:           not tested
Documentation:            not tested

Hello

This is one of those features that is beneficial to a handful of people in specific test cases. It may not benefit the majority of the users but is certainly not useless either. As long as it can be disabled and enough tests have been run to ensure it won't have a significant impact on working components while disabled, it should be fine in my opinion. Regarding where the selected port shall be saved (postmaster.pid, read by pg_ctl or saved in a dedicated file), I see that postmaster.pid already contains a port number in line number 4, so adding a port number into there is nothing new; port number is already there and we can simply replace the port number with the one selected by the system.

I applied and tested the patch and found that the system can indeed start when port is set to 0, but line 4 of postmaster.pid does not store the port number selected by the system, rather, it stored 0, which is the same as configured. So I am actually not able to find out the port number that my PG is running on, at least not in a straight-forward way.

thank you
==================
Cary Huang
HighGo Software
www.highgo.ca


--
Y.

Вложения

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

От
Robert Haas
Дата:
On Mon, Apr 24, 2023 at 10:16 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
> Right.  I'm perfectly content with just allowing port number 0 and
> leaving it at that.

That seems fine to me, too. If somebody wants to add a pg_ctl feature
to extract this or any other information from the postmaster.pid file,
that can be a separate patch. But it's not necessarily the case that
users would even prefer that interface. Some might, some might not. Or
so it seems to me, anyway.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

От
Alvaro Herrera
Дата:
On 2023-Apr-19, Yurii Rashkovskii wrote:

> If we consider this path, then (if we assume the format of the file is
> still to be private), we can make the port line accept multiple ports using
> a delimiter like `:` so that the next line still remains the same.

This made me wonder if storing the unadorned port number is really the
best way.  Suppose we did extend things so that we listen on different
ports on different interfaces; how would this scheme work at all?  I
suspect it would be simpler to store both the interface address and the
port, perhaps separated by :.  You would keep it to one pair per line,
so you'd get the IPv6 address/port separately from the IPv4 address, for
example.  And if you happen to have multiple addresses, you know exactly
which ones you're listening on.

To match a problem that has been discussed in the lists previously,
suppose you have listen_addresses='localhost' and the resolver does
funny things with that name (say you mess up /etc/hosts after starting).
Things would be much simpler if you knew exactly what the resolver did
at postmaster start time.

> (I consider this feature so small that it doesn't deserve such a lengthy
> discussion. However, I also get Tom's point about how we document this

You should see the discussion that led to the addition of psql's 'exit'
command sometime:

https://www.postgresql.org/message-id/flat/CALVFHFb-C_5_94hueWg6Dd0zu7TfbpT7hzsh9Zf0DEDOSaAnfA%40mail.gmail.com#949321e44856b7fa295834d6a3997ab4


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Every machine is a smoke machine if you operate it wrong enough."
https://twitter.com/libseybieda/status/1541673325781196801



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

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> This made me wonder if storing the unadorned port number is really the
> best way.  Suppose we did extend things so that we listen on different
> ports on different interfaces; how would this scheme work at all?

Yeah, the probability that that will happen someday is one of the
things bothering me about this proposal.  I'd rather change the
file format to support that first (it can be dummy for now, with
all lines showing the same port), and then document it second.

            regards, tom lane



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

От
Denis Laxalde
Дата:
The documentation fails to build for me:

$ ninja docs
[1/2] Generating doc/src/sgml/postgres-full.xml with a custom command
FAILED: doc/src/sgml/postgres-full.xml
/usr/bin/python3 ../postgresql/doc/src/sgml/xmltools_dep_wrapper 
--targetname doc/src/sgml/postgres-full.xml --depfile 
doc/src/sgml/postgres-full.xml.d --tool /usr/bin/xmllint -- --nonet 
--noent --valid --path doc/src/sgml -o doc/src/sgml/postgres-full.xml 
../postgresql/doc/src/sgml/postgres.sgml
../postgresql/doc/src/sgml/postgres.sgml:685: element para: validity 
error : Element entry is not declared in para list of possible children
ninja: build stopped: subcommand failed.


Removing the <entry> tag resolves the issue:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cd07bad3b5..f71859f710 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -684,7 +684,7 @@ include_dir 'conf.d'
         </para>
         <para>
          The port can be set to 0 to make Postgres pick an unused port 
number.
-        The assigned port number can be then retrieved from 
<entry><filename>postmaster.pid</filename></entry>.
+        The assigned port number can be then retrieved from 
<filename>postmaster.pid</filename>.
         </para>
        </listitem>
       </varlistentry>




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

От
Yurii Rashkovskii
Дата:
Hi Denis,

Great catch. I've amended the patch to fix this issue with the documentation (V3).



On Tue, May 9, 2023 at 2:25 PM Denis Laxalde <denis.laxalde@dalibo.com> wrote:
The documentation fails to build for me:

$ ninja docs
[1/2] Generating doc/src/sgml/postgres-full.xml with a custom command
FAILED: doc/src/sgml/postgres-full.xml
/usr/bin/python3 ../postgresql/doc/src/sgml/xmltools_dep_wrapper
--targetname doc/src/sgml/postgres-full.xml --depfile
doc/src/sgml/postgres-full.xml.d --tool /usr/bin/xmllint -- --nonet
--noent --valid --path doc/src/sgml -o doc/src/sgml/postgres-full.xml
../postgresql/doc/src/sgml/postgres.sgml
../postgresql/doc/src/sgml/postgres.sgml:685: element para: validity
error : Element entry is not declared in para list of possible children
ninja: build stopped: subcommand failed.


Removing the <entry> tag resolves the issue:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cd07bad3b5..f71859f710 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -684,7 +684,7 @@ include_dir 'conf.d'
         </para>
         <para>
          The port can be set to 0 to make Postgres pick an unused port
number.
-        The assigned port number can be then retrieved from
<entry><filename>postmaster.pid</filename></entry>.
+        The assigned port number can be then retrieved from
<filename>postmaster.pid</filename>.
         </para>
        </listitem>
       </varlistentry>



--
Y.

Вложения

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

От
Yurii Rashkovskii
Дата:
Alvaro, Tom,

On Mon, May 8, 2023 at 4:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> This made me wonder if storing the unadorned port number is really the
> best way.  Suppose we did extend things so that we listen on different
> ports on different interfaces; how would this scheme work at all?

Yeah, the probability that that will happen someday is one of the
things bothering me about this proposal.  I'd rather change the
file format to support that first (it can be dummy for now, with
all lines showing the same port), and then document it second.

How soon do you think the change will occur that will allow for choosing different ports on different interfaces? I am happy to help address this.

Relying on a variable number of lines may be counter-productive here if we want postmaster.pid to be easily readable by shell scripts. What if we
improved the port line to be something like this?

```
127.0.0.1=5432 ::1=54321
```

Basically, a space-delimited set of address/port pairs (delimited by `=` to allow IPv6 addresses to use a colon). If we allow the address side to be dropped, the current format (`5432`) will also be correct parsing-wise.

--
Y.

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

От
Alvaro Herrera
Дата:
On 2023-May-11, Yurii Rashkovskii wrote:

> Relying on a variable number of lines may be counter-productive here if we
> want postmaster.pid to be easily readable by shell scripts.

Oh, I was thinking in Peter E's proposal to list the interface/port
number pairs in a separate file named 'ports' or something like that.

> ```
> 127.0.0.1=5432 ::1=54321
> ```
> 
> Basically, a space-delimited set of address/port pairs (delimited by `=` to
> allow IPv6 addresses to use a colon).

This seems a bit too creative.  I'd rather have the IPv6 address in
square brackets, which clues the parser immediately as to the address
family and use colons to separate the port number.  If we do go with a
separate file, which to me sounds easier than cramming it into the PID
file, then one per line is likely better, if only because line-oriented
Unix text tooling has an easier time that way.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Having your biases confirmed independently is how scientific progress is
made, and hence made our great society what it is today" (Mary Gardiner)



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

От
Yurii Rashkovskii
Дата:
On Thu, May 11, 2023 at 10:36 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2023-May-11, Yurii Rashkovskii wrote:

> ```
> 127.0.0.1=5432 ::1=54321
> ```
>
> Basically, a space-delimited set of address/port pairs (delimited by `=` to
> allow IPv6 addresses to use a colon).

This seems a bit too creative.  I'd rather have the IPv6 address in
square brackets, which clues the parser immediately as to the address
family and use colons to separate the port number.  If we do go with a
separate file, which to me sounds easier than cramming it into the PID
file, then one per line is likely better, if only because line-oriented
Unix text tooling has an easier time that way.

Just a general caution here that using square brackets to denote IPv6 addresses will make it (unnecessarily?) harder to process this with a shell script.

--
Y.

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

От
Tristen Raab
Дата:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            tested, passed

Hello Yurii,

I've retested your latest patch and tested building the documentation. 

I agree with the general sentiment that this is an interesting, albeit specific feature. Nevertheless, I would still
liketo see this integrated. My only concern, like many others have voiced, is in regard to the port number. When I was
reviewingthis patch I found it quite unintuitive to rummage through the postmaster.pid to find the correct port. I
thinkeither a specific pg_ctl command to return the port like Greg had initially mentioned or simply a separate file to
storethe port numbers would be ideal. The standalone file being the simpler option, this would free up postmaster.pid
toallow any future alterations and still be able to reliably get the port number when using this wildcard. We can also
buildon this file later to allow for multiple ports to be listened on as previously suggested.
 

Kind regards,
Tristen

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

От
Aleksander Alekseev
Дата:
Hi,

> I think either a specific pg_ctl command to return the port like Greg had initially mentioned or simply a separate
fileto store the port numbers would be ideal.
 

+1, if we are going to do this we definitely need a pg_ctl command
and/or a file.

> The standalone file being the simpler option

Agree. However, I think we will have to add the display of the port
number to "pg_ctl status" too, for the sake of consistency [1].

[1]: https://www.postgresql.org/docs/current/app-pg-ctl.html

-- 
Best regards,
Aleksander Alekseev



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

От
Daniel Gustafsson
Дата:
> On 11 May 2023, at 13:24, Yurii Rashkovskii <yrashk@gmail.com> wrote:
>
> On Thu, May 11, 2023 at 10:36 AM Alvaro Herrera <alvherre@alvh.no-ip.org <mailto:alvherre@alvh.no-ip.org>> wrote:
> On 2023-May-11, Yurii Rashkovskii wrote:
>
> > ```
> > 127.0.0.1=5432 ::1=54321
> > ```
> >
> > Basically, a space-delimited set of address/port pairs (delimited by `=` to
> > allow IPv6 addresses to use a colon).
>
> This seems a bit too creative.  I'd rather have the IPv6 address in
> square brackets, which clues the parser immediately as to the address
> family and use colons to separate the port number.  If we do go with a
> separate file, which to me sounds easier than cramming it into the PID
> file, then one per line is likely better, if only because line-oriented
> Unix text tooling has an easier time that way.
>
> Just a general caution here that using square brackets to denote IPv6 addresses will make it (unnecessarily?) harder
toprocess this with a shell script. 

This patch is Waiting on Author in the current commitfest with no new patch
presented following the discussion here.  Is there an update ready or should we
close it in this CF in favour of a future one?

--
Daniel Gustafsson




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

От
Daniel Gustafsson
Дата:
> On 10 Jul 2023, at 14:27, Daniel Gustafsson <daniel@yesql.se> wrote:

> This patch is Waiting on Author in the current commitfest with no new patch
> presented following the discussion here.  Is there an update ready or should we
> close it in this CF in favour of a future one?

Since the thread stalled here with the patch waiting on author since May I will
go ahead and mark it returned with feedback in this CF.  Please feel free to
re-open a new entry in a future CF.

--
Daniel Gustafsson