Re: Millisecond-precision connect_timeout for libpq

Is there any hope to see it in libpq? If so, can anyone review latest
version of my patch?

On 10 July 2013 11:49, ivan babrou <ibobrik@gmail.com> wrote:
> On 9 July 2013 18:43, Andres Freund <andres@2ndquadrant.com> wrote:
>> On 2013-07-05 21:28:59 +0400, ivan babrou wrote:
>>> Hi, guys! I made a quick patch to support floating number in
>>> connect_timeout param for libpq. This will treat floating number as
>>> seconds so this is backwards-compatible. I don't usually write in C,
>>> so there may be mistakes. Could you review it and give me some
>>> feedback?
>>>
>>> --
>>> Regards, Ian Babrou
>>> http://bobrik.name http://twitter.com/ibobrik skype:i.babrou
>>
>>> diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
>>> index 18fcb0c..58c1a35 100644
>>> --- a/src/interfaces/libpq/fe-connect.c
>>> +++ b/src/interfaces/libpq/fe-connect.c
>>> @@ -1452,7 +1452,7 @@ static int
>>>  connectDBComplete(PGconn *conn)
>>>  {
>>>       PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
>>> -     time_t          finish_time = ((time_t) -1);
>>> +     struct timeval          finish_time;
>>>
>>>       if (conn == NULL || conn->status == CONNECTION_BAD)
>>>               return 0;
>>> @@ -1462,17 +1462,14 @@ connectDBComplete(PGconn *conn)
>>>        */
>>>       if (conn->connect_timeout != NULL)
>>>       {
>>> -             int                     timeout = atoi(conn->connect_timeout);
>>> +             int                     timeout_usec = (int) (atof(conn->connect_timeout) * 1000000);
>>
>> I'd rather not use a plain int for storing usecs. An overflow is rather
>> unlikely, but still. Also, I'd rather use something like USECS_PER_SEC
>> instead of a plain 1000000 in multiple places.
>>
>>>
>>> -             if (timeout > 0)
>>> +             if (timeout_usec > 0)
>>>               {
>>> -                     /*
>>> -                      * Rounding could cause connection to fail; need at least 2 secs
>>> -                      */
>>> -                     if (timeout < 2)
>>> -                             timeout = 2;
>>> -                     /* calculate the finish time based on start + timeout */
>>> -                     finish_time = time(NULL) + timeout;
>>> +                     gettimeofday(&finish_time, NULL);
>>> +                     finish_time.tv_usec += (int) timeout_usec;
>>
>> Accordingly adjust this.
>>
>> Looks like a sensible thing to me.
>>
>> *Independent* from this patch, you might want look into server-side
>> connection pooling using transaction mode. If that's applicable for
>> your application it might reduce latency noticeably.
>>
>> Greetings,
>>
>> Andres Freund
>>
>> --
>>  Andres Freund                     http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
>
> I tried to make it more safe. Still not sure about constants, I
> haven't found any good examples in libpq.
>
> --
> Regards, Ian Babrou
> http://bobrik.name http://twitter.com/ibobrik skype:i.babrou



-- 
Regards, Ian Babrou
http://bobrik.name http://twitter.com/ibobrik skype:i.babrou