Re: libpq should not look up all host addresses at once

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq should not look up all host addresses at once
Date: 2018-08-14 10:24:32
Message-ID: alpine.DEB.2.21.1808132356130.2727@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Tom,

>> As you noted in another message, a small doc update should be needed.
>
> Check. Proposed doc patch attached. (Only the last hunk is actually
> specific to this patch, the rest is cleanup that I noticed while looking
> around for possibly-relevant text.)

Doc build is ok.

Some comments that you may not find all useful, please accept my apology,
it just really shows that I read your prose in some detail:-)

The mention of possible reverse dns queries has been removed... but I do
not think there was any before? There could be if only hostaddr is
provided but a hostname is required by some auth, but it does not seem to
be the case according to the documentation.

I read the rational of the host/hostaddr artificial mapping. I cannot say
I'm thrilled with the result: I do not really see a setting where avoiding
a DNS query is required but which still needs a hostname for auth... If
you have GSSAPI or SSPI then you have an underlying network, in which a
dns query should be fine.

Moreover the feature implementation is misleading as hostaddr changes the
behavior of host when both are provided. Also, "hosts" accepts ips anyway
(although is not documented).

STM that we should have had only "host" for specifying the target (name,
ip, path), and if really necessary an ugly "authname" directive to provide
names on the side. I know, too late.

I think that in the string format host=foo:5433,bla:5432 could be
accepted as it is in the URL format.

Some of the changes are not directly related to the multi host feature,
and should/could be backpatched further?

>> I'd consider wrapping some of the logic. I'd check the port first, then
>> move the host resolution stuff into a function.
>
> Don't really see the value of either ...

What I see is that the host logic is rather lengthy and specific (it
depends on the connection type -- name, ip, socket including a so nice
#ifdef), distinct from the port stuff which is dealt with quite quickcly.

By separating the port & host and putting the lengthy stuff in a function
the scope of the for loop on potential connections would be easier to read
and understand, and would not require to see CHT_ cases to understand what
is being done for managing "host" variants, which is a mere detail.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Geoff Winkless 2018-08-14 10:32:56 Re: NetBSD vs libxml2
Previous Message Jarosław Torbicki 2018-08-14 09:58:43 Uncaught PHP Exception Doctrine\DBAL\Exception\UniqueConstraintViolationException: "An exception occurred while executing 'UPDATE