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 connection timeout mismanagement
In-reply-to: <alpine.DEB.2.21.1808111611040.30788@lancre>
References: <5735.1533828184@sss.pgh.pa.us>
 <alpine.DEB.2.21.1808111611040.30788@lancre>
Comments: In-reply-to Fabien COELHO <coelho@cri.ensmp.fr>
	message dated "Sat, 11 Aug 2018 16:42:16 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20524.1534180589.1@sss.pgh.pa.us>
Date: Mon, 13 Aug 2018 13:16:29 -0400
Message-ID: <20525.1534180589@sss.pgh.pa.us>
Precedence: bulk

Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Patch does not "git apply", but is ok with "patch -p1". Compiles.

Yeah, as far as I can tell, "git apply" is very intolerant.

> The code suggests that timeout is always 2 or more
>      if (timeout < 2) timeout = 2;
> but doc says "It is not recommended to use a timeout of less than 2 
> seconds", which is inconsistent. It should read "Timeout is always set to 
> at least 2 whatever the user asks.".

Agreed, changed the docs to clarify this.

> connect_timeout=2.9 is accepted and considered as meaning 2. 
> connect_timeout=-10 or connect_timeout=two are also accepted and mean 
> forever. Probably thanks to "atoi".

Right.  As before, I'm not excited about rejecting trailing junk,
considering we never did before.  It is kind of bogus that the
resolution of the timeout is only 1 second though --- maybe in
2002 that was fine, but today it seems strange.  I'm tempted to
change the parameter to be floating point (parse with atof, say)
and allow millisecond precision.  Then we'd not really need to
have the restriction about minimum delay.

That's a task for a different patch though.  In the meantime,
pushed this one --- thanks for reviewing!

			regards, tom lane