Re: connect_timeout parameter in libpq

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