Re: connect_timeout parameter in libpq - Mailing list pgsql-patches

Do people think this is of general enough interest to be added to libpq?


---------------------------------------------------------------------------

Denis A Ustimenko wrote:
> New version of patch. Documentation updates, accurate remaining time calculation etc.
>
> On Mon, Aug 12, 2002 at 11:35:17PM -0400, Tom Lane wrote:
> > Shouldn't such a patch include documentation updates?  (And not
> > only user-level documentation; this patch adds not even a single
> > comment to explain what it's doing or why.)
> >
> > I'm also not thrilled with the way that the patch imposes the
> > overhead of calculating the timeout whether the user wants it or not.
> > The time() kernel calls should be skipped unless needed.
> >
> > A final comment is that the patch's timeout accuracy is quite poor, since
> > time()'s result is quantized to seconds.  gettimeofday() might be a
> > better choice.  Also it seems to assume that select() does not modify its
> > timeout argument, which is not a portable assumption.  On some platforms
> > the timeout struct is decremented by the elapsed time.
> >
> >             regards, tom lane
>
> *** postgresql-7.2.1/doc/src/sgml/libpq.sgml.orig       ?? ??? 13 12:34:51 2002
> --- postgresql-7.2.1/doc/src/sgml/libpq.sgml    ?? ??? 13 14:18:15 2002
> ***************
> *** 172,177 ****
> --- 172,186 ----
>       </varlistentry>
>
>       <varlistentry>
> +      <term><literal>connect_timeout</literal></term>
> +      <listitem>
> +      <para>
> +       Time space in seconds given to connect routine. Zero or not set means infinite.
> +      </para>
> +      </listitem>
> +     </varlistentry>
> +
> +     <varlistentry>
>        <term><literal>options</literal></term>
>        <listitem>
>         <para>
> *** postgresql-7.2.1/src/interfaces/libpq/fe-connect.c.orig     ?? ???  8 12:22:46 2002
> --- postgresql-7.2.1/src/interfaces/libpq/fe-connect.c  ?? ??? 13 15:50:43 2002
> ***************
> *** 114,119 ****
> --- 114,122 ----
>         {"password", "PGPASSWORD", DefaultPassword, NULL,
>         "Database-Password", "*", 20},
>
> +         {"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
> +         "Connect-timeout", "", 10}, /* strlen( INT32_MAX) == 10 */
> +
>         {"dbname", "PGDATABASE", NULL, NULL,
>         "Database-Name", "", 20},
>
> ***************
> *** 307,312 ****
> --- 310,317 ----
>         conn->pguser = tmp ? strdup(tmp) : NULL;
>         tmp = conninfo_getval(connOptions, "password");
>         conn->pgpass = tmp ? strdup(tmp) : NULL;
> +       tmp = conninfo_getval(connOptions, "connect_timeout");
> +       conn->connect_timeout = tmp ? strdup(tmp) : NULL;
>   #ifdef USE_SSL
>         tmp = conninfo_getval(connOptions, "requiressl");
>         conn->require_ssl = tmp ? (tmp[0] == '1' ? true : false) : false;
> ***************
> *** 1050,1061 ****
>   {
>         PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
>
>         if (conn == NULL || conn->status == CONNECTION_BAD)
>                 return 0;
>
> !       for (;;)
>         {
>                 /*
>                  * Wait, if necessary.  Note that the initial state (just after
>                  * PQconnectStart) is to wait for the socket to select for
>                  * writing.
> --- 1055,1093 ----
>   {
>         PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
>
> +       struct timeval remains, *rp = NULL, finish_time, start_time;
> +
>         if (conn == NULL || conn->status == CONNECTION_BAD)
>                 return 0;
>
> !       /*
> !        * Prepare to time calculations, if connect_timeout isn't zero.
> !        */
> !       if (conn->connect_timeout != NULL)
>         {
> +               remains.tv_sec = atoi(conn->connect_timeout);
> +               if (!remains.tv_sec)
> +               {
> +                       conn->status = CONNECTION_BAD;
> +                       return 0;
> +               }
> +               remains.tv_usec = 0;
> +               rp = &remains;
> +       }
> +
> +
> +       while (NULL == rp || remains.tv_sec > 0 || remains.tv_sec == 0 && remains.tv_usec > 0)
> +       {
>                 /*
> +                * If connecting timeout is set, get current time.
> +                */
> +               if ( NULL != rp && -1 == gettimeofday(&start_time, NULL))
> +               {
> +                       conn->status = CONNECTION_BAD;
> +                       return 0;
> +               }
> +
> +               /*
>                  * Wait, if necessary.  Note that the initial state (just after
>                  * PQconnectStart) is to wait for the socket to select for
>                  * writing.
> ***************
> *** 1069,1075 ****
>                                 return 1;               /* success! */
>
>                         case PGRES_POLLING_READING:
> !                               if (pqWait(1, 0, conn))
>                                 {
>                                         conn->status = CONNECTION_BAD;
>                                         return 0;
> --- 1101,1107 ----
>                                 return 1;               /* success! */
>
>                         case PGRES_POLLING_READING:
> !                               if (pqWaitTimed(1, 0, conn, rp))
>                                 {
>                                         conn->status = CONNECTION_BAD;
>                                         return 0;
> ***************
> *** 1077,1083 ****
>                                 break;
>
>                         case PGRES_POLLING_WRITING:
> !                               if (pqWait(0, 1, conn))
>                                 {
>                                         conn->status = CONNECTION_BAD;
>                                         return 0;
> --- 1109,1115 ----
>                                 break;
>
>                         case PGRES_POLLING_WRITING:
> !                               if (pqWaitTimed(0, 1, conn, rp))
>                                 {
>                                         conn->status = CONNECTION_BAD;
>                                         return 0;
> ***************
> *** 1094,1100 ****
> --- 1126,1156 ----
>                  * Now try to advance the state machine.
>                  */
>                 flag = PQconnectPoll(conn);
> +
> +               /*
> +                * If connecting timeout is set, calculate remain time.
> +                */
> +               if (NULL != rp) {
> +                       if (-1 == gettimeofday(&finish_time, NULL))
> +                       {
> +                               conn->status = CONNECTION_BAD;
> +                               return 0;
> +                       }
> +                       if (0 > (finish_time.tv_usec -= start_time.tv_usec))
> +                       {
> +                               remains.tv_sec++;
> +                               finish_time.tv_usec += 1000000;
> +                       }
> +                       if (0 > (remains.tv_usec -= finish_time.tv_usec))
> +                       {
> +                               remains.tv_sec--;
> +                               remains.tv_usec += 1000000;
> +                       }
> +                       remains.tv_sec -= finish_time.tv_sec - start_time.tv_sec;
> +               }
>         }
> +       conn->status = CONNECTION_BAD;
> +       return 0;
>   }
>
>   /* ----------------
> ***************
> *** 1929,1934 ****
> --- 1985,1992 ----
>                 free(conn->pguser);
>         if (conn->pgpass)
>                 free(conn->pgpass);
> +       if (conn->connect_timeout)
> +               free(conn->connect_timeout);
>         /* Note that conn->Pfdebug is not ours to close or free */
>         if (conn->notifyList)
>                 DLFreeList(conn->notifyList);
> *** postgresql-7.2.1/src/interfaces/libpq/fe-misc.c.orig        ?? ???  8 16:33:34 2002
> --- postgresql-7.2.1/src/interfaces/libpq/fe-misc.c     ?? ??? 13 16:16:57 2002
> ***************
> *** 748,757 ****
> --- 748,766 ----
>   int
>   pqWait(int forRead, int forWrite, PGconn *conn)
>   {
> +       return pqWaitTimed( forRead, forWrite, conn, (const struct timeval *) NULL);
> + }
> +
> + int
> + pqWaitTimed(int forRead, int forWrite, PGconn *conn, const struct timeval *timeout)
> + {
>         fd_set          input_mask;
>         fd_set          output_mask;
>         fd_set          except_mask;
>
> +       struct timeval  tmp_timeout;
> +       struct timeval  *ptmp_timeout = NULL;
> +
>         if (conn->sock < 0)
>         {
>                 printfPQExpBuffer(&conn->errorMessage,
> ***************
> *** 770,778 ****
>                 if (forWrite)
>                         FD_SET(conn->sock, &output_mask);
>                 FD_SET(conn->sock, &except_mask);
> !               if (select(conn->sock + 1, &input_mask, &output_mask, &except_mask,
> !                                  (struct timeval *) NULL) < 0)
>                 {
>                         if (SOCK_ERRNO == EINTR)
>                                 goto retry;
>                         printfPQExpBuffer(&conn->errorMessage,
> --- 779,796 ----
>                 if (forWrite)
>                         FD_SET(conn->sock, &output_mask);
>                 FD_SET(conn->sock, &except_mask);
> !
> !               if (NULL != timeout)
>                 {
> +               /*
> +                * select may modify timeout argument on some platforms use copy
> +                */
> +                       tmp_timeout = *timeout;
> +                       ptmp_timeout = &tmp_timeout;
> +               }
> +               if (select(conn->sock + 1, &input_mask, &output_mask,
> +                             &except_mask, ptmp_timeout) < 0)
> +               {
>                         if (SOCK_ERRNO == EINTR)
>                                 goto retry;
>                         printfPQExpBuffer(&conn->errorMessage,
> *** postgresql-7.2.1/src/interfaces/libpq/libpq-int.h.orig      ?? ???  8 16:37:56 2002
> --- postgresql-7.2.1/src/interfaces/libpq/libpq-int.h   ?? ??? 13 14:37:54 2002
> ***************
> *** 279,284 ****
> --- 279,286 ----
>         PQExpBufferData workBuffer; /* expansible string */
>
>         int                     client_encoding;        /* encoding id */
> +
> +       char *connect_timeout;
>   };
>
>   /* String descriptions of the ExecStatusTypes.
> ***************
> *** 324,329 ****
> --- 326,332 ----
>   extern int    pqReadData(PGconn *conn);
>   extern int    pqFlush(PGconn *conn);
>   extern int    pqWait(int forRead, int forWrite, PGconn *conn);
> + extern int    pqWaitTimed(int forRead, int forWrite, PGconn *conn, const struct timeval* timeout);
>   extern int    pqReadReady(PGconn *conn);
>   extern int    pqWriteReady(PGconn *conn);
>
>
>
> --
> Regards
> Denis
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073