Improve the granularity of PQsocketPoll's timeout parameter.
authorTom Lane <[email protected]>
Thu, 13 Jun 2024 19:14:32 +0000 (15:14 -0400)
committerTom Lane <[email protected]>
Thu, 13 Jun 2024 19:14:32 +0000 (15:14 -0400)
Commit f5e4dedfa exposed libpq's internal function PQsocketPoll
without a lot of thought about whether that was an API we really
wanted to chisel in stone.  The main problem with it is the use of
time_t to specify the timeout.  While we do want an absolute time
so that a loop around PQsocketPoll doesn't have problems with
timeout slippage, time_t has only 1-second resolution.  That's
already problematic for libpq's own internal usage --- for example,
pqConnectDBComplete has long had a kluge to treat "connect_timeout=1"
as 2 seconds so that it doesn't accidentally round to nearly zero.
And it's even less likely to be satisfactory for external callers.
Hence, let's change this while we still can.

The best idea seems to be to use an int64 count of microseconds since
the epoch --- basically the same thing as the backend's TimestampTz,
but let's use the standard Unix epoch (1970-01-01) since that's more
likely for clients to be easy to calculate.  Millisecond resolution
would be plenty for foreseeable uses, but maybe the day will come that
we're glad we used microseconds.

Also, since time(2) isn't especially helpful for computing timeouts
defined this way, introduce a new function PQgetCurrentTimeUSec
to get the current time in this form.

Remove the hack in pqConnectDBComplete, so that "connect_timeout=1"
now means what you'd expect.

We can also remove the "#include <time.h>" that f5e4dedfa added to
libpq-fe.h, since there's no longer a need for time_t in that header.
It seems better for v17 not to enlarge libpq-fe.h's include footprint
from what it's historically been, anyway.

I also failed to resist the temptation to do some wordsmithing
on PQsocketPoll's documentation.

Patch by me, per complaint from Dominique Devienne.

Discussion: https://postgr.es/m/913559.1718055575@sss.pgh.pa.us

doc/src/sgml/libpq.sgml
src/bin/psql/command.c
src/interfaces/libpq/exports.txt
src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/fe-misc.c
src/interfaces/libpq/libpq-fe.h
src/interfaces/libpq/libpq-int.h
src/tools/pgindent/typedefs.list

index 80179f997838415553fec973e10a1ab705556ac4..068ee60771cbf53852fd4468713d200d9261880d 100644 (file)
@@ -262,41 +262,6 @@ PGconn *PQsetdb(char *pghost,
      </listitem>
     </varlistentry>
 
-    <varlistentry id="libpq-PQsocketPoll">
-     <term><function>PQsocketPoll</function><indexterm><primary>PQsocketPoll</primary></indexterm></term>
-     <listitem>
-      <para>
-       <indexterm><primary>nonblocking connection</primary></indexterm>
-       Poll a connection&apos;s underlying socket descriptor retrieved with <xref linkend="libpq-PQsocket"/>.
-<synopsis>
-int PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time);
-</synopsis>
-      </para>
-
-      <para>
-       This function sets up polling of a file descriptor. The underlying function is either
-       <function>poll(2)</function> or <function>select(2)</function>, depending on platform
-       support. The primary use of this function is iterating through the connection sequence
-       described in the documentation of <xref linkend="libpq-PQconnectStartParams"/>. If
-       <parameter>forRead</parameter> is specified, the function waits for the socket to be ready
-       for reading. If <parameter>forWrite</parameter> is specified, the function waits for the
-       socket to be ready for write. See <literal>POLLIN</literal> and <literal>POLLOUT</literal>
-       from <function>poll(2)</function>, or <parameter>readfds</parameter> and
-       <parameter>writefds</parameter> from <function>select(2)</function> for more information. If
-       <parameter>end_time</parameter> is not <literal>-1</literal>, it specifies the time at which
-       this function should stop waiting for the condition to be met.
-      </para>
-
-      <para>
-       The function returns a value greater than <literal>0</literal> if the specified condition
-       is met, <literal>0</literal> if a timeout occurred, or <literal>-1</literal> if an error
-       occurred. The error can be retrieved by checking the <literal>errno(3)</literal> value. In
-       the event <literal>forRead</literal> and <literal>forWrite</literal> are not set, the
-       function immediately returns a timeout condition.
-      </para>
-     </listitem>
-    </varlistentry>
-
     <varlistentry id="libpq-PQconnectStartParams">
      <term><function>PQconnectStartParams</function><indexterm><primary>PQconnectStartParams</primary></indexterm></term>
      <term><function>PQconnectStart</function><indexterm><primary>PQconnectStart</primary></indexterm></term>
@@ -546,6 +511,70 @@ switch(PQstatus(conn))
      </listitem>
     </varlistentry>
 
+    <varlistentry id="libpq-PQsocketPoll">
+     <term><function>PQsocketPoll</function><indexterm><primary>PQsocketPoll</primary></indexterm></term>
+     <listitem>
+      <para>
+       <indexterm><primary>nonblocking connection</primary></indexterm>
+       Poll a connection's underlying socket descriptor retrieved with
+       <xref linkend="libpq-PQsocket"/>.
+       The primary use of this function is iterating through the connection
+       sequence described in the documentation of
+       <xref linkend="libpq-PQconnectStartParams"/>.
+<synopsis>
+typedef pg_int64 pg_usec_time_t;
+
+int PQsocketPoll(int sock, int forRead, int forWrite,
+                 pg_usec_time_t end_time);
+</synopsis>
+      </para>
+
+      <para>
+       This function performs polling of a file descriptor, optionally with
+       a timeout.
+       If <parameter>forRead</parameter> is nonzero, the
+       function will terminate when the socket is ready for
+       reading. If <parameter>forWrite</parameter> is nonzero,
+       the function will terminate when the
+       socket is ready for writing.
+      </para>
+
+      <para>
+       The timeout is specified by <parameter>end_time</parameter>, which
+       is the time to stop waiting expressed as a number of microseconds since
+       the Unix epoch (that is, <type>time_t</type> times 1 million).
+       Timeout is infinite if <parameter>end_time</parameter>
+       is <literal>-1</literal>.  Timeout is immediate (no blocking) if
+       end_time is <literal>0</literal> (or indeed, any time before now).
+       Timeout values can be calculated conveniently by adding the desired
+       number of microseconds to the result of
+       <xref linkend="libpq-PQgetCurrentTimeUSec"/>.
+       Note that the underlying system calls may have less than microsecond
+       precision, so that the actual delay may be imprecise.
+      </para>
+
+      <para>
+       The function returns a value greater than <literal>0</literal> if the
+       specified condition is met, <literal>0</literal> if a timeout occurred,
+       or <literal>-1</literal> if an error occurred. The error can be
+       retrieved by checking the <literal>errno(3)</literal> value. In the
+       event both <parameter>forRead</parameter>
+       and <parameter>forWrite</parameter> are zero, the function immediately
+       returns a timeout indication.
+      </para>
+
+      <para>
+       <function>PQsocketPoll</function> is implemented using either
+       <function>poll(2)</function> or <function>select(2)</function>,
+       depending on platform.  See <literal>POLLIN</literal>
+       and <literal>POLLOUT</literal> from <function>poll(2)</function>,
+       or <parameter>readfds</parameter> and
+       <parameter>writefds</parameter> from <function>select(2)</function>,
+       for more information.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry id="libpq-PQconndefaults">
      <term><function>PQconndefaults</function><indexterm><primary>PQconndefaults</primary></indexterm></term>
      <listitem>
@@ -1390,8 +1419,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <para>
        Maximum time to wait while connecting, in seconds (write as a decimal integer,
        e.g., <literal>10</literal>).  Zero, negative, or not specified means
-       wait indefinitely.  The minimum allowed timeout is 2 seconds, therefore
-       a value of <literal>1</literal> is interpreted as <literal>2</literal>.
+       wait indefinitely.
        This timeout applies separately to each host name or IP address.
        For example, if you specify two hosts and <literal>connect_timeout</literal>
        is 5, each host will time out if no connection is made within 5
@@ -8039,6 +8067,25 @@ int PQlibVersion(void);
     </listitem>
    </varlistentry>
 
+   <varlistentry id="libpq-PQgetCurrentTimeUSec">
+    <term><function>PQgetCurrentTimeUSec</function><indexterm><primary>PQgetCurrentTimeUSec</primary></indexterm></term>
+
+    <listitem>
+     <para>
+      Retrieves the current time, expressed as the number of microseconds
+      since the Unix epoch (that is, <type>time_t</type> times 1 million).
+<synopsis>
+pg_usec_time_t PQgetCurrentTimeUSec(void);
+</synopsis>
+     </para>
+
+     <para>
+      This is primarily useful for calculating timeout values to use with
+      <xref linkend="libpq-PQsocketPoll"/>.
+     </para>
+    </listitem>
+   </varlistentry>
+
   </variablelist>
 
  </sect1>
index fae5940b54e972f5f174cbf38429b3ac055be9dc..180781ecd05250c4aaf405884b76ccb63ebee9ae 100644 (file)
@@ -3764,7 +3764,7 @@ wait_until_connected(PGconn *conn)
    {
        int         rc;
        int         sock;
-       time_t      end_time;
+       pg_usec_time_t end_time;
 
        /*
         * On every iteration of the connection sequence, let's check if the
@@ -3795,7 +3795,7 @@ wait_until_connected(PGconn *conn)
         * solution happens to just be adding a timeout, so let's wait for 1
         * second and check cancel_pressed again.
         */
-       end_time = time(NULL) + 1;
+       end_time = PQgetCurrentTimeUSec() + 1000000;
        rc = PQsocketPoll(sock, forRead, !forRead, end_time);
        if (rc == -1)
            return;
index 8ee0811510079b8d76a74e27c93f56f91c5acfe0..5d8213e0b571a95e5163b9d42590206c988d043d 100644 (file)
@@ -204,3 +204,4 @@ PQcancelReset             201
 PQcancelFinish            202
 PQsocketPoll              203
 PQsetChunkedRowsMode      204
+PQgetCurrentTimeUSec      205
index 90f259fc70684e98ddd61f402e8a3ce68385bcb3..071b1b34aa1184d692b4ddd38b04683b8bd4a503 100644 (file)
@@ -2470,7 +2470,7 @@ int
 pqConnectDBComplete(PGconn *conn)
 {
    PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
-   time_t      finish_time = ((time_t) -1);
+   pg_usec_time_t end_time = -1;
    int         timeout = 0;
    int         last_whichhost = -2;    /* certainly different from whichhost */
    int         last_whichaddr = -2;    /* certainly different from whichaddr */
@@ -2479,7 +2479,7 @@ pqConnectDBComplete(PGconn *conn)
        return 0;
 
    /*
-    * Set up a time limit, if connect_timeout isn't zero.
+    * Set up a time limit, if connect_timeout is greater than zero.
     */
    if (conn->connect_timeout != NULL)
    {
@@ -2490,19 +2490,6 @@ pqConnectDBComplete(PGconn *conn)
            conn->status = CONNECTION_BAD;
            return 0;
        }
-
-       if (timeout > 0)
-       {
-           /*
-            * Rounding could cause connection to fail unexpectedly quickly;
-            * to prevent possibly waiting hardly-at-all, insist on at least
-            * two seconds.
-            */
-           if (timeout < 2)
-               timeout = 2;
-       }
-       else                    /* negative means 0 */
-           timeout = 0;
    }
 
    for (;;)
@@ -2519,7 +2506,7 @@ pqConnectDBComplete(PGconn *conn)
            (conn->whichhost != last_whichhost ||
             conn->whichaddr != last_whichaddr))
        {
-           finish_time = time(NULL) + timeout;
+           end_time = PQgetCurrentTimeUSec() + (pg_usec_time_t) timeout * 1000000;
            last_whichhost = conn->whichhost;
            last_whichaddr = conn->whichaddr;
        }
@@ -2534,7 +2521,7 @@ pqConnectDBComplete(PGconn *conn)
                return 1;       /* success! */
 
            case PGRES_POLLING_READING:
-               ret = pqWaitTimed(1, 0, conn, finish_time);
+               ret = pqWaitTimed(1, 0, conn, end_time);
                if (ret == -1)
                {
                    /* hard failure, eg select() problem, aborts everything */
@@ -2544,7 +2531,7 @@ pqConnectDBComplete(PGconn *conn)
                break;
 
            case PGRES_POLLING_WRITING:
-               ret = pqWaitTimed(0, 1, conn, finish_time);
+               ret = pqWaitTimed(0, 1, conn, end_time);
                if (ret == -1)
                {
                    /* hard failure, eg select() problem, aborts everything */
index f562cd8d34441465fa5d0ab7e4e985c2e468999e..f235bfbb41fb558b19bd2f32ecca96b88a7d75a8 100644 (file)
@@ -54,7 +54,7 @@
 static int pqPutMsgBytes(const void *buf, size_t len, PGconn *conn);
 static int pqSendSome(PGconn *conn, int len);
 static int pqSocketCheck(PGconn *conn, int forRead, int forWrite,
-                         time_t end_time);
+                         pg_usec_time_t end_time);
 
 /*
  * PQlibVersion: return the libpq version number
@@ -977,22 +977,25 @@ pqFlush(PGconn *conn)
 int
 pqWait(int forRead, int forWrite, PGconn *conn)
 {
-   return pqWaitTimed(forRead, forWrite, conn, (time_t) -1);
+   return pqWaitTimed(forRead, forWrite, conn, -1);
 }
 
 /*
- * pqWaitTimed: wait, but not past finish_time.
- *
- * finish_time = ((time_t) -1) disables the wait limit.
+ * pqWaitTimed: wait, but not past end_time.
  *
  * Returns -1 on failure, 0 if the socket is readable/writable, 1 if it timed out.
+ *
+ * The timeout is specified by end_time, which is the int64 number of
+ * microseconds since the Unix epoch (that is, time_t times 1 million).
+ * Timeout is infinite if end_time is -1.  Timeout is immediate (no blocking)
+ * if end_time is 0 (or indeed, any time before now).
  */
 int
-pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
+pqWaitTimed(int forRead, int forWrite, PGconn *conn, pg_usec_time_t end_time)
 {
    int         result;
 
-   result = pqSocketCheck(conn, forRead, forWrite, finish_time);
+   result = pqSocketCheck(conn, forRead, forWrite, end_time);
 
    if (result < 0)
        return -1;              /* errorMessage is already set */
@@ -1013,7 +1016,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
 int
 pqReadReady(PGconn *conn)
 {
-   return pqSocketCheck(conn, 1, 0, (time_t) 0);
+   return pqSocketCheck(conn, 1, 0, 0);
 }
 
 /*
@@ -1023,7 +1026,7 @@ pqReadReady(PGconn *conn)
 int
 pqWriteReady(PGconn *conn)
 {
-   return pqSocketCheck(conn, 0, 1, (time_t) 0);
+   return pqSocketCheck(conn, 0, 1, 0);
 }
 
 /*
@@ -1035,7 +1038,7 @@ pqWriteReady(PGconn *conn)
  * for read data directly.
  */
 static int
-pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
+pqSocketCheck(PGconn *conn, int forRead, int forWrite, pg_usec_time_t end_time)
 {
    int         result;
 
@@ -1079,11 +1082,13 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
  * condition (without waiting).  Return >0 if condition is met, 0
  * if a timeout occurred, -1 if an error or interrupt occurred.
  *
+ * The timeout is specified by end_time, which is the int64 number of
+ * microseconds since the Unix epoch (that is, time_t times 1 million).
  * Timeout is infinite if end_time is -1.  Timeout is immediate (no blocking)
  * if end_time is 0 (or indeed, any time before now).
  */
 int
-PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
+PQsocketPoll(int sock, int forRead, int forWrite, pg_usec_time_t end_time)
 {
    /* We use poll(2) if available, otherwise select(2) */
 #ifdef HAVE_POLL
@@ -1103,14 +1108,16 @@ 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 == -1)
        timeout_ms = -1;
+   else if (end_time == 0)
+       timeout_ms = 0;
    else
    {
-       time_t      now = time(NULL);
+       pg_usec_time_t now = PQgetCurrentTimeUSec();
 
        if (end_time > now)
-           timeout_ms = (end_time - now) * 1000;
+           timeout_ms = (end_time - now) / 1000;
        else
            timeout_ms = 0;
    }
@@ -1138,17 +1145,28 @@ PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
    FD_SET(sock, &except_mask);
 
    /* Compute appropriate timeout interval */
-   if (end_time == ((time_t) -1))
+   if (end_time == -1)
        ptr_timeout = NULL;
+   else if (end_time == 0)
+   {
+       timeout.tv_sec = 0;
+       timeout.tv_usec = 0;
+       ptr_timeout = &timeout;
+   }
    else
    {
-       time_t      now = time(NULL);
+       pg_usec_time_t now = PQgetCurrentTimeUSec();
 
        if (end_time > now)
-           timeout.tv_sec = end_time - now;
+       {
+           timeout.tv_sec = (end_time - now) / 1000000;
+           timeout.tv_usec = (end_time - now) % 1000000;
+       }
        else
+       {
            timeout.tv_sec = 0;
-       timeout.tv_usec = 0;
+           timeout.tv_usec = 0;
+       }
        ptr_timeout = &timeout;
    }
 
@@ -1157,6 +1175,21 @@ PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
 #endif                         /* HAVE_POLL */
 }
 
+/*
+ * PQgetCurrentTimeUSec: get current time with microsecond precision
+ *
+ * This provides a platform-independent way of producing a reference
+ * value for PQsocketPoll's timeout parameter.
+ */
+pg_usec_time_t
+PQgetCurrentTimeUSec(void)
+{
+   struct timeval tval;
+
+   gettimeofday(&tval, NULL);
+   return (pg_usec_time_t) tval.tv_sec * 1000000 + tval.tv_usec;
+}
+
 
 /*
  * A couple of "miscellaneous" multibyte related functions. They used
index 73f6e65ae55c8e548e169caba702b5f5857813b3..87a6f3df0744009102a0400aaef6b504b64788db 100644 (file)
@@ -21,7 +21,6 @@ extern "C"
 #endif
 
 #include <stdio.h>
-#include <time.h>
 
 /*
  * postgres_ext.h defines the backend's externally visible types,
@@ -202,6 +201,9 @@ typedef struct pgNotify
    struct pgNotify *next;      /* list link */
 } PGnotify;
 
+/* pg_usec_time_t is like time_t, but with microsecond resolution */
+typedef pg_int64 pg_usec_time_t;
+
 /* Function types for notice-handling callbacks */
 typedef void (*PQnoticeReceiver) (void *arg, const PGresult *res);
 typedef void (*PQnoticeProcessor) (void *arg, const char *message);
@@ -673,7 +675,11 @@ extern int lo_export(PGconn *conn, Oid lobjId, const char *filename);
 extern int PQlibVersion(void);
 
 /* Poll a socket for reading and/or writing with an optional timeout */
-extern int PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time);
+extern int PQsocketPoll(int sock, int forRead, int forWrite,
+                        pg_usec_time_t end_time);
+
+/* Get current time in the form PQsocketPoll wants */
+extern pg_usec_time_t PQgetCurrentTimeUSec(void);
 
 /* Determine length of multibyte encoded char at *s */
 extern int PQmblen(const char *s, int encoding);
index 3886204c70820de14db90e291f93abfc1c26b482..f36d76bf3fedfdf8bdd387ec480e420d7dd37d1a 100644 (file)
@@ -755,7 +755,7 @@ 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,
-                       time_t finish_time);
+                       pg_usec_time_t end_time);
 extern int pqReadReady(PGconn *conn);
 extern int pqWriteReady(PGconn *conn);
 
index 4f57078d133cb83e4e8ef6f0a9d481b70040c602..61ad417cde664a59e52bbde8ba4708894c070f05 100644 (file)
@@ -3717,6 +3717,7 @@ pg_unicode_normprops
 pg_unicode_properties
 pg_unicode_range
 pg_unicode_recompinfo
+pg_usec_time_t
 pg_utf_to_local_combined
 pg_uuid_t
 pg_wc_probefunc