Re: Millisecond-precision connect_timeout for libpq - Mailing list pgsql-hackers

From Markus Wanner
Subject Re: Millisecond-precision connect_timeout for libpq
Date
Msg-id 51DC174F.7000204@bluegap.ch
Whole thread Raw
In response to Millisecond-precision connect_timeout for libpq  (ivan babrou <ibobrik@gmail.com>)
Responses Re: Millisecond-precision connect_timeout for libpq
List 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@sss.pgh.pa.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



pgsql-hackers by date:

Previous
From: Merlin Moncure
Date:
Subject: Re: Millisecond-precision connect_timeout for libpq
Next
From: Andrew Dunstan
Date:
Subject: Re: Should we automatically run duplicate_oids?