Re: Millisecond-precision connect_timeout for libpq

From: ivan babrou <ibobrik(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Millisecond-precision connect_timeout for libpq
Date: 2013-07-15 12:34:07
Message-ID: CANWdNRAeAZ-d6+srqoBce=Eu0i+EcPh=kLadXFKh2EiywAYMFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)gmail(dot)com> wrote:
> On 9 July 2013 18:43, Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2013-07-15 12:43:02 Re: Millisecond-precision connect_timeout for libpq
Previous Message Rushabh Lathia 2013-07-15 11:55:14 Re: proposal: enable new error fields in plpgsql (9.4)