Re: Millisecond-precision connect_timeout for libpq

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: ivan babrou <ibobrik(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Millisecond-precision connect_timeout for libpq
Date: 2013-07-09 14:43:19
Message-ID: 20130709144319.GA27898@alap2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sawada Masahiko 2013-07-09 14:45:57 Re: Patch for fail-back without fresh backup
Previous Message Andrew Dunstan 2013-07-09 14:40:13 Re: Should we automatically run duplicate_oids?