Re: Millisecond-precision connect_timeout for libpq

From: Markus Wanner <markus(at)bluegap(dot)ch>
To: ivan babrou <ibobrik(at)gmail(dot)com>
Cc: PostgreSQL-development Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Millisecond-precision connect_timeout for libpq
Date: 2013-07-09 13:59:43
Message-ID: 51DC174F.7000204@bluegap.ch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ian,

On 07/05/2013 07:28 PM, ivan babrou wrote:
> - /*
> - * Rounding could cause connection to fail; need at least 2 secs
> - */

You removed this above comment... please check why it's there. The
relevant revision seems to be:

###
commit 2908a838ac2cf8cdccaa115249f8399eef8a731e
Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Date: Thu Oct 24 23:35:55 2002 +0000

Code review for connection timeout patch. Avoid unportable assumption
that tv_sec is signed; return a useful error message on timeout failure;
honor PGCONNECT_TIMEOUT environment variable in PQsetdbLogin; make code
obey documentation statement that timeout=0 means no timeout.
###

> - 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;

I vaguely recall tv_usec only being required to hold values up to
1000000 by some standard. A signed 32 bit value would qualify, but only
hold up to a good half hour worth of microseconds. That doesn't quite
seem enough to calculate finish_time the way you are proposing to do it.

> + finish_time.tv_sec += finish_time.tv_usec / 1000000;
> + finish_time.tv_usec = finish_time.tv_usec % 1000000;
> }
> }
>
> @@ -1073,15 +1074,15 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
> input_fd.events |= POLLOUT;
>
> /* Compute appropriate timeout interval */
> - if (end_time == ((time_t) -1))
> + if (end_time == NULL)
> timeout_ms = -1;
> else
> {
> - time_t now = time(NULL);
> + struct timeval now;
> + gettimeofday(&now, NULL);
>
> - if (end_time > now)
> - timeout_ms = (end_time - now) * 1000;
> - else
> + timeout_ms = (end_time->tv_sec - now.tv_sec) * 1000 + (end_time->tv_usec - now.tv_usec) / 1000;

I think that's incorrect on a platform where tv_sec and/or tv_usec is
unsigned. (And the cited commit above indicates there are such platforms.)

On 07/09/2013 02:25 PM, ivan babrou wrote:
> There's no complexity here :)

Not so fast, cowboy... :-)

Regards

Markus Wanner

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message ivan babrou 2013-07-09 14:27:09 Re: Millisecond-precision connect_timeout for libpq
Previous Message Merlin Moncure 2013-07-09 13:53:12 Re: Millisecond-precision connect_timeout for libpq