duplicate connection failure messages

On Wed, Oct 13, 2010 at 21:21, Peter Eisentraut <peter_e@gmx.net> wrote:

This surprised me:

psql -p 55555 -h localhost
psql: could not connect to server: Connection refused
       Is the server running on host "localhost" and accepting
       TCP/IP connections on port 55555?
could not connect to server: Connection refused
       Is the server running on host "localhost" and accepting
       TCP/IP connections on port 55555?

It shows the same error message twice.  I can reproduce this back to PG
8.2.

It appears to have something to do with localhost resolving to an IPv4
and an IPv6 address, since it doesn't happen with other host names that
only resolve to one address.  However, fe-connect.c claims:

   /*
    * Try to initiate a connection to one of the addresses
    * returned by pg_getaddrinfo_all().  conn->addr_cur is the
    * next one to try. We fail when we run out of addresses
    * (reporting the error returned for the *last* alternative,
    * which may not be what users expect :-().
    */

That comment is out of date. I implemented the "keep all error
messages and append them to each other" feature to make sure it didn't
throw away the interesting error message and replaced it with a
generic one later, must've missed that comment - at least it should be
changed. And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

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

Excerpts from Magnus Hagander's message of jue oct 14 02:30:36 -0300 2010:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

+1

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On tor, 2010-10-14 at 07:30 +0200, Magnus Hagander wrote:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

That looks good.

Peter Eisentraut wrote:

On tor, 2010-10-14 at 07:30 +0200, Magnus Hagander wrote:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

That looks good.

I have developed the attached patch to report whether IPv4 or IPv6 are
being used. I could not find the numeric value as alwasy populated, and
this seems clearer too:

$ pql -h localhost test
psql: could not connect to server: Connection refused
Is the server running on host "localhost" (IPv4) and accepting
TCP/IP connections on port 5432?

$ psql -h 127.0.0.1 test
psql: could not connect to server: Connection refused
Is the server running on host "127.0.0.1" (IPv4) and accepting
TCP/IP connections on port 5432?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch to report whether IPv4 or IPv6 are
being used.

What's the use of that exactly? It doesn't really respond to Peter's
concern, I think.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch to report whether IPv4 or IPv6 are
being used.

What's the use of that exactly? It doesn't really respond to Peter's
concern, I think.

Peter liked:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

What this will show is:

localhost (IPv4)
localhost (IPv6)

Is that good? I can't figure out how to do ::1 because when you supply
a host _name_, there is no reverse mapping done. Looking at the code,
we test for a host name, then a host ip, and don't assume they are both
set.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

On Fri, Nov 12, 2010 at 15:02, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch to report whether IPv4 or IPv6 are
being used.

What's the use of that exactly?  It doesn't really respond to Peter's
concern, I think.

Peter liked:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

What this will show is:

       localhost (IPv4)
       localhost (IPv6)

Is that good?  I can't figure out how to do ::1 because when you supply
a host _name_, there is no reverse mapping done.  Looking at the code,
we test for a host name, then a host ip, and don't assume they are both
set.

The address is in conn->raddr, no? When you've put in a host name, we
do a forward lookup, so conn->raddr should contain ::1 already? You
only need the reverse mapping to get the "localhost" part, if I read
the code correctly?

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

Magnus Hagander wrote:

On Fri, Nov 12, 2010 at 15:02, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch to report whether IPv4 or IPv6 are
being used.

What's the use of that exactly? ?It doesn't really respond to Peter's
concern, I think.

Peter liked:

And I agree it's not very friendly in this specific case - I
wonder if we should log it as "localhost (127.0.0.1) and "localhost
(::1)" (and similar for any other case that returns more than one
address).

What this will show is:

? ? ? ?localhost (IPv4)
? ? ? ?localhost (IPv6)

Is that good? ?I can't figure out how to do ::1 because when you supply
a host _name_, there is no reverse mapping done. ?Looking at the code,
we test for a host name, then a host ip, and don't assume they are both
set.

The address is in conn->raddr, no? When you've put in a host name, we
do a forward lookup, so conn->raddr should contain ::1 already? You
only need the reverse mapping to get the "localhost" part, if I read
the code correctly?

OK, I found out how to get the IP address with the attached patch. The
problem is that only pghost is set, never pghostaddr. I am not even
sure how that would get set for this code because my tests show it is
not:

$ psql -h localhost test
pghost = localhost
--> pghostaddr = (null)
psql: could not connect to server: Connection refused
Is the server running on host "localhost" (127.0.0.1) and accepting
TCP/IP connections on port 5432?

$ psql -h 127.0.0.1 test
pghost = 127.0.0.1
pghostaddr = (null)
psql: could not connect to server: Connection refused
Is the server running on host "127.0.0.1" and accepting
TCP/IP connections on port 5432?

To get this to work, I compared pghost with the raddr value, and printed
the IP address if it was not already printed. There is still a problem
about threading that I can fix.

Is this what we want?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Excerpts from Bruce Momjian's message of sáb nov 13 22:36:31 -0300 2010:

OK, I found out how to get the IP address with the attached patch. The
problem is that only pghost is set, never pghostaddr. I am not even
sure how that would get set for this code because my tests show it is
not:

This doesn't work for IPv6 addresses, though.

pghostaddr is specified by the user on the command line as an
optimization to avoid DNS lookups IIRC, which is why you don't see the
code setting it.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of s��b nov 13 22:36:31 -0300 2010:

OK, I found out how to get the IP address with the attached patch. The
problem is that only pghost is set, never pghostaddr. I am not even
sure how that would get set for this code because my tests show it is
not:

This doesn't work for IPv6 addresses, though.

pghostaddr is specified by the user on the command line as an
optimization to avoid DNS lookups IIRC, which is why you don't see the
code setting it.

OK, I doubt we want to add complexity to improve this, so I see our
options as:

o ignore the problem
o display IPv4/IPv6 labels
o display only an IPv6 label
o something else

Comments?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Excerpts from Bruce Momjian's message of mié nov 17 13:04:46 -0300 2010:

OK, I doubt we want to add complexity to improve this, so I see our
options as:

o ignore the problem
o display IPv4/IPv6 labels
o display only an IPv6 label
o something else

I think we should use inet_ntop where available to print the address.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of mi�� nov 17 13:04:46 -0300 2010:

OK, I doubt we want to add complexity to improve this, so I see our
options as:

o ignore the problem
o display IPv4/IPv6 labels
o display only an IPv6 label
o something else

I think we should use inet_ntop where available to print the address.

Good idea because inet_ntop() is thread-safe. Does that work on IPv6?
You indicated that inet_ntoa() does not.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of mi�� nov 17 13:04:46 -0300 2010:

OK, I doubt we want to add complexity to improve this, so I see our
options as:

o ignore the problem
o display IPv4/IPv6 labels
o display only an IPv6 label
o something else

I think we should use inet_ntop where available to print the address.

FYI, I see we use inet_ntoa() in getaddrinfo.c. That is not thread-safe
and I should replace it with inet_ntop().

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Excerpts from Bruce Momjian's message of vie nov 19 00:17:59 -0300 2010:

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of mi nov 17 13:04:46 -0300 2010:

OK, I doubt we want to add complexity to improve this, so I see our
options as:

o ignore the problem
o display IPv4/IPv6 labels
o display only an IPv6 label
o something else

I think we should use inet_ntop where available to print the address.

Good idea because inet_ntop() is thread-safe. Does that work on IPv6?
You indicated that inet_ntoa() does not.

According to opengroup.org, IPv6 should work if the underlying libraries
support it, whereas inet_ntoa explicitely does not.
http://www.opengroup.org/onlinepubs/009695399/functions/inet_ntop.html
http://www.opengroup.org/onlinepubs/009695399/functions/inet_addr.html

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Bruce Momjian's message of vie nov 19 00:17:59 -0300 2010:

Alvaro Herrera wrote:

I think we should use inet_ntop where available to print the address.

Good idea because inet_ntop() is thread-safe. Does that work on IPv6?
You indicated that inet_ntoa() does not.

According to opengroup.org, IPv6 should work if the underlying libraries
support it, whereas inet_ntoa explicitely does not.
http://www.opengroup.org/onlinepubs/009695399/functions/inet_ntop.html
http://www.opengroup.org/onlinepubs/009695399/functions/inet_addr.html

I get the impression that you guys have forgotten the existence of
src/backend/utils/adt/inet_net_ntop.c

regards, tom lane

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Bruce Momjian's message of vie nov 19 00:17:59 -0300 2010:

Alvaro Herrera wrote:

I think we should use inet_ntop where available to print the address.

Good idea because inet_ntop() is thread-safe. Does that work on IPv6?
You indicated that inet_ntoa() does not.

According to opengroup.org, IPv6 should work if the underlying libraries
support it, whereas inet_ntoa explicitely does not.
http://www.opengroup.org/onlinepubs/009695399/functions/inet_ntop.html
http://www.opengroup.org/onlinepubs/009695399/functions/inet_addr.html

I get the impression that you guys have forgotten the existence of
src/backend/utils/adt/inet_net_ntop.c

Yeah, that is nice, but we are calling this from libpq, not the backend.
Let me work up a patch.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Excerpts from Bruce Momjian's message of vie nov 19 16:43:33 -0300 2010:

Tom Lane wrote:

I get the impression that you guys have forgotten the existence of
src/backend/utils/adt/inet_net_ntop.c

Yeah, that is nice, but we are calling this from libpq, not the backend.
Let me work up a patch.

Actually the code seems agnostic (no ereport, palloc etc) so maybe it
could just be moved to src/port.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of vie nov 19 16:43:33 -0300 2010:

Tom Lane wrote:

I get the impression that you guys have forgotten the existence of
src/backend/utils/adt/inet_net_ntop.c

Yeah, that is nice, but we are calling this from libpq, not the backend.
Let me work up a patch.

Actually the code seems agnostic (no ereport, palloc etc) so maybe it
could just be moved to src/port.

I was wondering that. I am unclear if we need it though --- can we not
assume inet_ntop() exists on all systems? We assumed inet_ntoa() did.
Of course, the buildfarm will tell us.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Bruce Momjian <bruce@momjian.us> writes:

I was wondering that. I am unclear if we need it though --- can we not
assume inet_ntop() exists on all systems? We assumed inet_ntoa() did.

The Single Unix Spec includes inet_ntoa but not inet_ntop.

Of course, the buildfarm will tell us.

The buildfarm unfortunately contains only a subset of the platforms
we care about. I don't think this problem is large enough to justify
taking a portability risk by depending on non-SUS library functions.

If you want to do this, please do it as suggested previously, ie depend
on the copy of the code we have internally.

regards, tom lane