Re: [HACKERS] Re: [doc fix] PG10: wroing description onconnect_timeout when multiple hosts are specified

On Wed, May 17, 2017 at 11:49 AM, Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
> From: pgsql-hackers-owner@postgresql.org
>> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Michael Paquier
>> pqWait is internal to libpq, so we are free to set up what we want here.
>> Still I think that we should be consistent with what pqSocketCheck returns:
>
> Please let this what it is now for the same reason Robert mentioned.
>
>> +    int            ret = 0;
>> +    int            timeout = 0;
>> The declaration of "ret" should be internal in the for(;;) loop.
>
> Done.
>
>> +           /* Attempt connection to the next host, starting the
>> connect_timeout timer */
>> +           pqDropConnection(conn, true);
>> +           conn->addr_cur = conn->connhost[conn->whichhost].addrlist;
>> +           conn->status = CONNECTION_NEEDED;
>> +           finish_time = time(NULL) + timeout;
>> +       }
>> I think that it would be safer to not set finish_time if
>> conn->connect_timeout is NULL. I agree that your code works because
>> pqWaitTimed() will never complain on timeout reached if finish_time is -1.
>> That's for robustness sake.
>
> Done, but I'm not sure how this contributes to the robustness.  I guess you were concerned just in case pqWaitTimed()
returned0 (timeout) even when it should not.
 

Thanks for the updated patch. This looks good to me.
-- 
Michael