Re: pg_ctl and port number detection

pg_ctl.c::test_postmaster_connection() has some fragile code that tries
to detect the server port number by looking in the pg_ctl -o string,
postgresql.conf, the PGPORT environment variable, and finally using the
default port number.

I think a simpler solution would be to look in postmaster.pid:

10231
/u/pgsql/data
5432001 45481984

pg_ctl already knows the data directory. If the file is missing, the
server is not running. If the file exists, the first number on the last
line, divided by 1000, is the port number. We can then use this port
number for libpq to check for connectivity.

Comments?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> pg_ctl.c::test_postmaster_connection() has some fragile code that tries
> to detect the server port number by looking in the pg_ctl -o string,

It may be fragile, but it works; or at least I've not heard complaints
about it lately.

> I think a simpler solution would be to look in postmaster.pid:
> pg_ctl already knows the data directory. If the file is missing, the
> server is not running. If the file exists, the first number on the last
> line, divided by 1000, is the port number.

That's somewhere between fragile and outright wrong.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > pg_ctl.c::test_postmaster_connection() has some fragile code that tries
> > to detect the server port number by looking in the pg_ctl -o string,
>
> It may be fragile, but it works; or at least I've not heard complaints
> about it lately.

True.

> > I think a simpler solution would be to look in postmaster.pid:
> > pg_ctl already knows the data directory. If the file is missing, the
> > server is not running. If the file exists, the first number on the last
> > line, divided by 1000, is the port number.
>
> That's somewhere between fragile and outright wrong.

Please explain why my idea is not an improvement.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>>> pg_ctl already knows the data directory. If the file is missing, the
>>> server is not running. If the file exists, the first number on the last
>>> line, divided by 1000, is the port number.

>> That's somewhere between fragile and outright wrong.

> Please explain why my idea is not an improvement.

Because it's assuming that those numbers are sysv shmem keys derived in
a particular way. We have platforms on which that is wrong, Windows
being the most obvious example. Reading the shmem key assignment code
closely will suggest to you other ways that this could fail. Not to
mention that people propose getting rid of sysv shmem approximately
every other month, and perhaps someday that will actually happen;
whereupon whatever might get logged in postmaster.pid could be something
completely different.

If you really think that pulling a port number out of the pid file is an
improvement over what pg_ctl does now, then you need to start by storing
the port number, as such, in the pid file. Not something that might or
might not be related to the port number. But what we have to discuss
before that is whether we mind having a significant postmaster version
dependency in pg_ctl.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >>> pg_ctl already knows the data directory. If the file is missing, the
> >>> server is not running. If the file exists, the first number on the last
> >>> line, divided by 1000, is the port number.
>
> >> That's somewhere between fragile and outright wrong.
>
> > Please explain why my idea is not an improvement.
>
> Because it's assuming that those numbers are sysv shmem keys derived in
> a particular way. We have platforms on which that is wrong, Windows
> being the most obvious example. Reading the shmem key assignment code
> closely will suggest to you other ways that this could fail. Not to
> mention that people propose getting rid of sysv shmem approximately
> every other month, and perhaps someday that will actually happen;
> whereupon whatever might get logged in postmaster.pid could be something
> completely different.

Yeah, I was afraid of Windows.

> If you really think that pulling a port number out of the pid file is an
> improvement over what pg_ctl does now, then you need to start by storing
> the port number, as such, in the pid file. Not something that might or
> might not be related to the port number. But what we have to discuss
> before that is whether we mind having a significant postmaster version
> dependency in pg_ctl.

OK, good point on the version issue. Let's see if we get more
complaints before changing this. Thanks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 12/18/2010 06:23 PM, Bruce Momjian wrote:
>
>> If you really think that pulling a port number out of the pid file is an
>> improvement over what pg_ctl does now, then you need to start by storing
>> the port number, as such, in the pid file. Not something that might or
>> might not be related to the port number. But what we have to discuss
>> before that is whether we mind having a significant postmaster version
>> dependency in pg_ctl.
> OK, good point on the version issue. Let's see if we get more
> complaints before changing this. Thanks.
>

Wasn't there a proposal to provide an explicit port parameter to pg_ctl,
instead of relying on PGPORT? That would probably be a small advance.

cheers

andrew

Andrew Dunstan wrote:
>
>
> On 12/18/2010 06:23 PM, Bruce Momjian wrote:
> >
> >> If you really think that pulling a port number out of the pid file is an
> >> improvement over what pg_ctl does now, then you need to start by storing
> >> the port number, as such, in the pid file. Not something that might or
> >> might not be related to the port number. But what we have to discuss
> >> before that is whether we mind having a significant postmaster version
> >> dependency in pg_ctl.
> > OK, good point on the version issue. Let's see if we get more
> > complaints before changing this. Thanks.
> >
>
> Wasn't there a proposal to provide an explicit port parameter to pg_ctl,
> instead of relying on PGPORT? That would probably be a small advance.

I do not remember that suggestion.

I wonder if we should write the port number as the 4th line in
postmaster.pid and return in a few major releases and use that. We
could fall back and use our existing code if there is no 4th line.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Dec19, 2010, at 00:54 , Bruce Momjian wrote:
> I wonder if we should write the port number as the 4th line in
> postmaster.pid and return in a few major releases and use that. We
> could fall back and use our existing code if there is no 4th line.

What if the postmaster instead created a second unix socket in its
data directory? For security reason, it'd probably need to set
the permissions to 0600, but it'd still allow maintenance tools to
connect reliably if they only knew the data directory.

Don't know if that'd work on windows, though - I have no idea if
we even support something similar to unix sockets there, and if so,
it it lives in the filesystem.

best regards,
Florian Pflug

On Sun, Dec 19, 2010 at 20:16, Florian Pflug <fgp(at)phlo(dot)org> wrote:
> On Dec19, 2010, at 00:54 , Bruce Momjian wrote:
>> I wonder if we should write the port number as the 4th line in
>> postmaster.pid and return in a few major releases and use that.  We
>> could fall back and use our existing code if there is no 4th line.
>
> What if the postmaster instead created a second unix socket in its
> data directory? For security reason, it'd probably need to set
> the permissions to 0600, but it'd still allow maintenance tools to
> connect reliably if they only knew the data directory.
>
> Don't know if that'd work on windows, though - I have no idea if
> we even support something similar to unix sockets there, and if so,
> it it lives in the filesystem.

We don't, and AFAIK there's nothing that lives in the filesystem. You
have named pipes that live in the namespace, but not within
directories in the filesystem.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On Dec19, 2010, at 21:10 , Magnus Hagander wrote:
> On Sun, Dec 19, 2010 at 20:16, Florian Pflug <fgp(at)phlo(dot)org> wrote:
>> On Dec19, 2010, at 00:54 , Bruce Momjian wrote:
>>> I wonder if we should write the port number as the 4th line in
>>> postmaster.pid and return in a few major releases and use that. We
>>> could fall back and use our existing code if there is no 4th line.
>>
>> What if the postmaster instead created a second unix socket in its
>> data directory? For security reason, it'd probably need to set
>> the permissions to 0600, but it'd still allow maintenance tools to
>> connect reliably if they only knew the data directory.
>>
>> Don't know if that'd work on windows, though - I have no idea if
>> we even support something similar to unix sockets there, and if so,
>> it it lives in the filesystem.
>
> We don't, and AFAIK there's nothing that lives in the filesystem. You
> have named pipes that live in the namespace, but not within
> directories in the filesystem.

Hm, OK, that pretty much kills the idea. Having to special-case
windows seems less appealing than the other options.

best regards,
Florian Pflug

Bruce Momjian wrote:
> Andrew Dunstan wrote:
> >
> >
> > On 12/18/2010 06:23 PM, Bruce Momjian wrote:
> > >
> > >> If you really think that pulling a port number out of the pid file is an
> > >> improvement over what pg_ctl does now, then you need to start by storing
> > >> the port number, as such, in the pid file. Not something that might or
> > >> might not be related to the port number. But what we have to discuss
> > >> before that is whether we mind having a significant postmaster version
> > >> dependency in pg_ctl.
> > > OK, good point on the version issue. Let's see if we get more
> > > complaints before changing this. Thanks.
> > >
> >
> > Wasn't there a proposal to provide an explicit port parameter to pg_ctl,
> > instead of relying on PGPORT? That would probably be a small advance.
>
> I do not remember that suggestion.
>
> I wonder if we should write the port number as the 4th line in
> postmaster.pid and return in a few major releases and use that. We
> could fall back and use our existing code if there is no 4th line.

OK, here is a modified idea. For 9.1, we have the postmaster write the
port number as the fourth line in postmaster.pid. pg_ctl will use that
fourth line if it exists, i.e. the postmaster is 9.1+.

If the fourth line is missing, we use the first number of the third line
on Unix and divide that by 1000 to get the port number. That file
format is not going to change because it is pre-9.1. If the third line
is empty (e.g. Windows) we either use PGPORT or throw an error.

So, we have a fine solution for 9.1+ servers, and all Unix servers, and
if you want to use a 9.1+ pg_ctl on a pre-9.1 server on Windows and use
the -w flag and a non-standard port number, you must specify PGPORT.
Based on the fact that most Windows users use the one-click installer
that will use the matching pg_ctl version, it seems this will work just
fine.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> I wonder if we should write the port number as the 4th line in
>> postmaster.pid and return in a few major releases and use that. We
>> could fall back and use our existing code if there is no 4th line.

No. If it goes in, it should go in as the third line. The shmem key
data is private to the server --- we do not want external programs
assuming anything at all about the private part of postmaster.pid.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> I wonder if we should write the port number as the 4th line in
> >> postmaster.pid and return in a few major releases and use that. We
> >> could fall back and use our existing code if there is no 4th line.
>
> No. If it goes in, it should go in as the third line. The shmem key
> data is private to the server --- we do not want external programs
> assuming anything at all about the private part of postmaster.pid.

OK, so you are suggesting having it as a third value on the third line?

10231
/u/pgsql/data
5432001 45481984 port_here
^^^^^^^^^
I like that better because it simplifies the test and limits the
possibility of non-atomic multi-line writes. For Win32, we would just
have the port number because the line is normally empty.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> No. If it goes in, it should go in as the third line. The shmem key
>> data is private to the server --- we do not want external programs
>> assuming anything at all about the private part of postmaster.pid.

> OK, so you are suggesting having it as a third value on the third line?

> 10231
> /u/pgsql/data
> 5432001 45481984 port_here
> ^^^^^^^^^

I'm not sure why you're having such a hard time grasping this concept.
We do not want pg_ctl looking at the shmem key information, not even to
the extent of assuming a particular format for it. Therefore the port
number has to go before it not after it. What I'm thinking of is

pid
datadir
port
... here be dragons ...

Actually, if we're going to do this at all, we should do

pid
datadir
port
socketdir
... here be dragons ...

so that pg_ctl doesn't have to assume the server is running with a
default value of unix_socket_dir. Not sure what to put in the fourth
line on Windows though ... maybe just leave it empty?

regards, tom lane

On 12/20/2010 12:41 PM, Tom Lane wrote:
>
> Actually, if we're going to do this at all, we should do
>
> pid
> datadir
> port
> socketdir
> ... here be dragons ...
>
> so that pg_ctl doesn't have to assume the server is running with a
> default value of unix_socket_dir. Not sure what to put in the fourth
> line on Windows though ... maybe just leave it empty?
>
>

Yes, that seems reasonable.

cheers

andrew

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> No. If it goes in, it should go in as the third line. The shmem key
> >> data is private to the server --- we do not want external programs
> >> assuming anything at all about the private part of postmaster.pid.
>
> > OK, so you are suggesting having it as a third value on the third line?
>
> > 10231
> > /u/pgsql/data
> > 5432001 45481984 port_here
> > ^^^^^^^^^
>
> I'm not sure why you're having such a hard time grasping this concept.
> We do not want pg_ctl looking at the shmem key information, not even to
> the extent of assuming a particular format for it. Therefore the port
> number has to go before it not after it. What I'm thinking of is
>
> pid
> datadir
> port
> ... here be dragons ...
>
> Actually, if we're going to do this at all, we should do
>
> pid
> datadir
> port
> socketdir
> ... here be dragons ...
>
> so that pg_ctl doesn't have to assume the server is running with a
> default value of unix_socket_dir. Not sure what to put in the fourth
> line on Windows though ... maybe just leave it empty?

OK. I was hesitant to modify the existing postmaster.pid format and was
trying to just add on the end. It is certainly easier to put it before
the shared memory stuff. I will work on a patch.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Tom Lane wrote:
> Actually, if we're going to do this at all, we should do
>
> pid
> datadir
> port
> socketdir
> ... here be dragons ...
>
> so that pg_ctl doesn't have to assume the server is running with a
> default value of unix_socket_dir. Not sure what to put in the fourth
> line on Windows though ... maybe just leave it empty?

I am curious about the use of the socketdir. What can that be used for?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Dec21, 2010, at 03:04 , Bruce Momjian wrote:
> Tom Lane wrote:
>> Actually, if we're going to do this at all, we should do
>>
>> pid
>> datadir
>> port
>> socketdir
>> ... here be dragons ...
>>
>> so that pg_ctl doesn't have to assume the server is running with a
>> default value of unix_socket_dir. Not sure what to put in the fourth
>> line on Windows though ... maybe just leave it empty?
>
> I am curious about the use of the socketdir. What can that be used for?

If it's non-default and you want to connect via unix sockets, just knowing
the port won't help much.

best regards,
Florian Pflug

Florian Pflug wrote:
> On Dec21, 2010, at 03:04 , Bruce Momjian wrote:
> > Tom Lane wrote:
> >> Actually, if we're going to do this at all, we should do
> >>
> >> pid
> >> datadir
> >> port
> >> socketdir
> >> ... here be dragons ...
> >>
> >> so that pg_ctl doesn't have to assume the server is running with a
> >> default value of unix_socket_dir. Not sure what to put in the fourth
> >> line on Windows though ... maybe just leave it empty?
> >
> > I am curious about the use of the socketdir. What can that be used for?
>
> If it's non-default and you want to connect via unix sockets, just knowing
> the port won't help much.

Ah, so pg_ctl is going to use that information too --- good point!

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Tom Lane wrote:
> Actually, if we're going to do this at all, we should do
>
> pid
> datadir
> port
> socketdir
> ... here be dragons ...
>
> so that pg_ctl doesn't have to assume the server is running with a
> default value of unix_socket_dir. Not sure what to put in the fourth
> line on Windows though ... maybe just leave it empty?

OK, here is a patch that adds the port number and optionally socket
directory location to postmaster.pid, and modifies pg_ctl to use that
information. I throw an error on using Win32 with pre-9.1 servers
because we can't get the port number from that file.

This removes some crufty code from pg_ctl and removes dependency on
serveral user-configurable settings that we added as a work-around.

This will allow pg_ctl -w to work more reliabily than it did in the
past.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian wrote:
> Tom Lane wrote:
> > Actually, if we're going to do this at all, we should do
> >
> > pid
> > datadir
> > port
> > socketdir
> > ... here be dragons ...
> >
> > so that pg_ctl doesn't have to assume the server is running with a
> > default value of unix_socket_dir. Not sure what to put in the fourth
> > line on Windows though ... maybe just leave it empty?
>
> OK, here is a patch that adds the port number and optionally socket
> directory location to postmaster.pid, and modifies pg_ctl to use that
> information. I throw an error on using Win32 with pre-9.1 servers
> because we can't get the port number from that file.
>
> This removes some crufty code from pg_ctl and removes dependency on
> serveral user-configurable settings that we added as a work-around.
>
> This will allow pg_ctl -w to work more reliabily than it did in the
> past.

Applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Fri, Dec 24, 2010 at 11:47 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Applied.

storage.sgml seems to need to be updated.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:
> On Fri, Dec 24, 2010 at 11:47 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Applied.
>
> storage.sgml seems to need to be updated.

Ah, I see that now, thanks. Patch attached and applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +