Received: from malur.postgresql.org ([2a02:16a8:dc51::56])
	by arkaria.postgresql.org with esmtps
 (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1fpIne-0001jN-K6
	for pgsql-hackers@arkaria.postgresql.org; Mon, 13 Aug 2018 19:46:46 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1fpInc-0007kV-RO
	for pgsql-hackers@arkaria.postgresql.org; Mon, 13 Aug 2018 19:46:44 +0000
Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29])
	by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1fpInc-0007kO-JT
	for pgsql-hackers@lists.postgresql.org; Mon, 13 Aug 2018 19:46:44 +0000
Received: from sss.pgh.pa.us ([66.207.139.130])
	by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1fpInY-0006i8-Ko
	for pgsql-hackers@lists.postgresql.org; Mon, 13 Aug 2018 19:46:44 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id w7DJkY2O027729;
	Mon, 13 Aug 2018 15:46:35 -0400
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Fabien COELHO <coelho@cri.ensmp.fr>
cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: libpq should not look up all host addresses at once
In-reply-to: <alpine.DEB.2.21.1808111122220.1705@lancre>
References: <4913.1533827102@sss.pgh.pa.us>
 <alpine.DEB.2.21.1808111122220.1705@lancre>
Comments: In-reply-to Fabien COELHO <coelho@cri.ensmp.fr>
	message dated "Sat, 11 Aug 2018 12:18:48 +0200"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <27657.1534189498.0@sss.pgh.pa.us>
Date: Mon, 13 Aug 2018 15:46:34 -0400
Message-ID: <27728.1534189594@sss.pgh.pa.us>
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27657.1534189498.1@sss.pgh.pa.us>

Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Patch compiles, global "make check" ok, although I'm unsure whether the
> feature is actually tested somewhere. I think not:-(

Yeah, it's hard to test this stuff without either opening up security
hazards or making unwarranted assumptions about the local network setup.
I think that the standard regression tests only use Unix-socket
communication (except on Windows) for exactly that reason, and that makes
it hard to do anything much about regression-testing this feature.

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

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

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="libpq-delay-getaddrinfo-docs.patch";
	charset="us-ascii"
Content-ID: <27657.1534189498.2@sss.pgh.pa.us>
Content-Description: libpq-delay-getaddrinfo-docs.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 80e55f5..7c150f3 100644
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** PostgresPollingStatusType PQconnectPoll(
*** 303,311 ****
         <itemizedlist>
          <listitem>
           <para>
!           The <literal>hostaddr</literal> and <literal>host</literal> pa=
rameters are used appropriately to ensure that
!           name and reverse name queries are not made. See the documentat=
ion of
!           these parameters in <xref linkend=3D"libpq-paramkeywords"/> fo=
r details.
           </para>
          </listitem>
  =

--- 303,311 ----
         <itemizedlist>
          <listitem>
           <para>
!           The <literal>hostaddr</literal> parameter must be used appropr=
iately
!           to prevent DNS queries from being made.  See the documentation=
 of
!           this parameter in <xref linkend=3D"libpq-paramkeywords"/> for =
details.
           </para>
          </listitem>
  =

*************** PostgresPollingStatusType PQconnectPoll(
*** 318,324 ****
  =

          <listitem>
           <para>
!           You ensure that the socket is in the appropriate state
            before calling <function>PQconnectPoll</function>, as describe=
d below.
           </para>
          </listitem>
--- 318,324 ----
  =

          <listitem>
           <para>
!           You must ensure that the socket is in the appropriate state
            before calling <function>PQconnectPoll</function>, as describe=
d below.
           </para>
          </listitem>
*************** PostgresPollingStatusType PQconnectPoll(
*** 326,349 ****
        </para>
  =

        <para>
!        Note: use of <function>PQconnectStartParams</function> is analogo=
us to
!        <function>PQconnectStart</function> shown below.
!       </para>
! =

!       <para>
!        To begin a nonblocking connection request, call <literal>conn =3D=
 PQconnectStart("<replaceable>connection_info_string</replaceable>")</lite=
ral>.
!        If <varname>conn</varname> is null, then <application>libpq</appl=
ication> has been unable to allocate a new <structname>PGconn</structname>
!        structure. Otherwise, a valid <structname>PGconn</structname> poi=
nter is returned (though not yet
!        representing a valid connection to the database). On return from
!        <function>PQconnectStart</function>, call <literal>status =3D PQs=
tatus(conn)</literal>. If <varname>status</varname> equals
!        <symbol>CONNECTION_BAD</symbol>, <function>PQconnectStart</functi=
on> has failed.
        </para>
  =

        <para>
!        If <function>PQconnectStart</function> succeeds, the next stage i=
s to poll
!        <application>libpq</application> so that it can proceed with the =
connection sequence.
         Use <function>PQsocket(conn)</function> to obtain the descriptor =
of the
         socket underlying the database connection.
         Loop thus: If <function>PQconnectPoll(conn)</function> last retur=
ned
         <symbol>PGRES_POLLING_READING</symbol>, wait until the socket is =
ready to
         read (as indicated by <function>select()</function>, <function>po=
ll()</function>, or
--- 326,352 ----
        </para>
  =

        <para>
!        To begin a nonblocking connection request,
!        call <function>PQconnectStart</function>
!        or <function>PQconnectStartParams</function>.  If the result is n=
ull,
!        then <application>libpq</application> has been unable to allocate=
 a
!        new <structname>PGconn</structname> structure.  Otherwise, a
!        valid <structname>PGconn</structname> pointer is returned (though=
 not
!        yet representing a valid connection to the database).  Next
!        call <literal>PQstatus(conn)</literal>.  If the result
!        is <symbol>CONNECTION_BAD</symbol>, the connection attempt has al=
ready
!        failed, typically because of invalid connection parameters.
        </para>
  =

        <para>
!        If <function>PQconnectStart</function>
!        or <function>PQconnectStartParams</function> succeeds, the next s=
tage
!        is to poll <application>libpq</application> so that it can procee=
d with
!        the connection sequence.
         Use <function>PQsocket(conn)</function> to obtain the descriptor =
of the
         socket underlying the database connection.
+        (Caution: do not assume that the socket remains the same
+        across <function>PQconnectPoll</function> calls.)
         Loop thus: If <function>PQconnectPoll(conn)</function> last retur=
ned
         <symbol>PGRES_POLLING_READING</symbol>, wait until the socket is =
ready to
         read (as indicated by <function>select()</function>, <function>po=
ll()</function>, or
*************** PostgresPollingStatusType PQconnectPoll(
*** 352,360 ****
         Conversely, if <function>PQconnectPoll(conn)</function> last retu=
rned
         <symbol>PGRES_POLLING_WRITING</symbol>, wait until the socket is =
ready
         to write, then call <function>PQconnectPoll(conn)</function> agai=
n.
!        If you have yet to call
!        <function>PQconnectPoll</function>, i.e., just after the call to
!        <function>PQconnectStart</function>, behave as if it last returne=
d
         <symbol>PGRES_POLLING_WRITING</symbol>.  Continue this loop until
         <function>PQconnectPoll(conn)</function> returns
         <symbol>PGRES_POLLING_FAILED</symbol>, indicating the connection =
procedure
--- 355,362 ----
         Conversely, if <function>PQconnectPoll(conn)</function> last retu=
rned
         <symbol>PGRES_POLLING_WRITING</symbol>, wait until the socket is =
ready
         to write, then call <function>PQconnectPoll(conn)</function> agai=
n.
!        On the first iteration, i.e. if you have yet to call
!        <function>PQconnectPoll</function>, behave as if it last returned
         <symbol>PGRES_POLLING_WRITING</symbol>.  Continue this loop until
         <function>PQconnectPoll(conn)</function> returns
         <symbol>PGRES_POLLING_FAILED</symbol>, indicating the connection =
procedure
*************** switch(PQstatus(conn))
*** 479,488 ****
        </para>
  =

        <para>
!        Note that if <function>PQconnectStart</function> returns a non-nu=
ll pointer, you must call
!        <function>PQfinish</function> when you are finished with it, in o=
rder to dispose of
!        the structure and any associated memory blocks. This must be done=
 even if
!        the connection attempt fails or is abandoned.
        </para>
       </listitem>
      </varlistentry>
--- 481,492 ----
        </para>
  =

        <para>
!        Note that when <function>PQconnectStart</function>
!        or <function>PQconnectStartParams</function> returns a non-null
!        pointer, you must call <function>PQfinish</function> when you are
!        finished with it, in order to dispose of the structure and any
!        associated memory blocks.  This must be done even if the connecti=
on
!        attempt fails or is abandoned.
        </para>
       </listitem>
      </varlistentry>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 913,919 ****
         It is possible to specify multiple hosts to connect to, so that t=
hey are
         tried in the given order. In the Keyword/Value format, the <liter=
al>host</literal>,
         <literal>hostaddr</literal>, and <literal>port</literal> options =
accept a comma-separated
!        list of values. The same number of elements must be given in each=
 option, such
         that e.g. the first <literal>hostaddr</literal> corresponds to th=
e first host name,
         the second <literal>hostaddr</literal> corresponds to the second =
host name, and so
         forth. As an exception, if only one <literal>port</literal> is sp=
ecified, it
--- 917,924 ----
         It is possible to specify multiple hosts to connect to, so that t=
hey are
         tried in the given order. In the Keyword/Value format, the <liter=
al>host</literal>,
         <literal>hostaddr</literal>, and <literal>port</literal> options =
accept a comma-separated
!        list of values. The same number of elements must be given in each
!        option that is specified, such
         that e.g. the first <literal>hostaddr</literal> corresponds to th=
e first host name,
         the second <literal>hostaddr</literal> corresponds to the second =
host name, and so
         forth. As an exception, if only one <literal>port</literal> is sp=
ecified, it
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 922,930 ****
  =

       <para>
         In the connection URI format, you can list multiple <literal>host=
:port</literal> pairs
!        separated by commas, in the <literal>host</literal> component of =
the URI. In either
!        format, a single host name can also translate to multiple network=
 addresses. A
!        common example of this is a host that has both an IPv4 and an IPv=
6 address.
       </para>
  =

       <para>
--- 927,939 ----
  =

       <para>
         In the connection URI format, you can list multiple <literal>host=
:port</literal> pairs
!        separated by commas, in the <literal>host</literal> component of =
the URI.
!      </para>
! =

!      <para>
!        In either format, a single host name can translate to multiple ne=
twork
!        addresses. A common example of this is a host that has both an IP=
v4 and
!        an IPv6 address.
       </para>
  =

       <para>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 958,966 ****
          Name of host to connect to.<indexterm><primary>host name</primar=
y></indexterm>
          If a host name begins with a slash, it specifies Unix-domain
          communication rather than TCP/IP communication; the value is the
!         name of the directory in which the socket file is stored.  If
!         multiple host names are specified, each will be tried in turn in
!         the order given.  The default behavior when <literal>host</liter=
al> is
          not specified, or is empty, is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexter=
m> in
          <filename>/tmp</filename> (or whatever socket directory was spec=
ified
--- 967,974 ----
          Name of host to connect to.<indexterm><primary>host name</primar=
y></indexterm>
          If a host name begins with a slash, it specifies Unix-domain
          communication rather than TCP/IP communication; the value is the
!         name of the directory in which the socket file is stored.
!         The default behavior when <literal>host</literal> is
          not specified, or is empty, is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexter=
m> in
          <filename>/tmp</filename> (or whatever socket directory was spec=
ified
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 997,1004 ****
          <itemizedlist>
           <listitem>
            <para>
!            If <literal>host</literal> is specified without <literal>host=
addr</literal>,
!            a host name lookup occurs.
            </para>
           </listitem>
           <listitem>
--- 1005,1016 ----
          <itemizedlist>
           <listitem>
            <para>
!            If <literal>host</literal> is specified
!            without <literal>hostaddr</literal>, a host name lookup occur=
s.
!            (When using <function>PQconnectPoll</function>, the lookup oc=
curs
!            when <function>PQconnectPoll</function> first considers this =
host
!            name, and it may cause <function>PQconnectPoll</function> to =
block
!            for a significant amount of time.)
            </para>
           </listitem>
           <listitem>

------- =_aaaaaaaaaa0--