docs done Re: [HACKERS] LIBPQ patches ...

* Tom Lane <tgl@sss.pgh.pa.us> [000109 08:18] wrote:
> Don Baccus <dhogaza@pacifier.com> writes:
> > At 05:27 PM 1/8/00 -0500, Tom Lane wrote:
> >> I also object strongly to the lack of documentation.
> 
> > ... I know there are some folks who aren't native-english speakers, so
> > perhaps you don't want to require that the implementor of such patches
> > provide the final documentation wording.  But the information should
> > be there and spelled out in a form that can be very easily moved to
> > the docs.
> 
> Oh, absolutely.  Thomas, our master of the docs, has always had the
> policy of "give me some words, I'll take care of formatting and
> editing..."
> 
> I was probably too harsh on Alfred last night, since in fact his code
> was fairly well commented, and some minimal doco could have been
> extracted from the routine headers.  But on a change like this, I think
> some paragraphs of coherent high-level explanation are needed: what it
> does, when and why you'd use it.  I didn't see that anywhere...

Here's the revised patch, it includes sgml docs and changes to
ensure that old style connections behave the way they are expected
to:

Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /home/pgcvs/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.25
diff -u -c -r1.25 libpq.sgml
*** doc/src/sgml/libpq.sgml    2000/01/14 05:33:13    1.25
--- doc/src/sgml/libpq.sgml    2000/01/17 03:40:30
***************
*** 377,382 ****
--- 377,386 ----    changed in the future.   </para>   <para>
+    These functions leave the socket in a non-blocking state as if 
+    <function>PQsetnonblocking</function> had been called.
+   </para>
+   <para>    These functions are not thread-safe.   </para>  </listitem>
***************
*** 1168,1175 ****
--- 1172,1229 ---- Applications that do not like these limitations can instead use the underlying functions that
<function>PQexec</function>is built from: <function>PQsendQuery</function> and <function>PQgetResult</function>.
 
+ </para>
+ <para>
+ Older programs that used this functionality as well as 
+ <function>PQputline</function> and <function>PQputnbytes</function>
+ could block waiting to send data to the backend, to
+ address that issue, the function <function>PQsetnonblocking</function>
+ was added.
+ </para>
+ <para>
+ Old applications can neglect to use <function>PQsetnonblocking</function>
+ and get the older potentially blocking behavior.  Newer programs can use 
+ <function>PQsetnonblocking</function> to achieve a completely non-blocking
+ connection to the backend.  <itemizedlist>
+  <listitem>
+    <para>
+     <function>PQsetnonblocking</function> Sets the state of the connection
+     to non-blocking.
+ <synopsis>
+ int PQsetnonblocking(PGconn *conn)
+ </synopsis>
+     this function will ensure that calls to 
+     <function>PQputline</function>, <function>PQputnbytes</function>,
+     <function>PQsendQuery</function> and <function>PQendcopy</function>
+     will not block but instead return an error if they need to be called
+     again.
+    </para>
+    <para>
+     When a database connection has been set to non-blocking mode and
+     <function>PQexec</function> is called, it will temporarily set the state
+     of the connection to blocking until the <function>PQexec</function> 
+     completes. 
+    </para>
+    <para>
+     More of libpq is expected to be made safe for 
+     <function>PQsetnonblocking</function> functionality in the near future.
+   </para>
+  </listitem>
+ 
+ <listitem>
+ <para>
+ <function>PQisnonblocking</function>
+        Returns the blocking status of the database connection.
+ <synopsis>
+ int PQisnonblocking(const PGconn *conn)
+ </synopsis>
+        Returns TRUE if the connection is set to non-blocking mode,
+        FALSE if blocking.
+ </para>
+ </listitem>
+  <listitem> <para> <function>PQsendQuery</function>
***************
*** 1267,1286 ****  <listitem> <para> <function>PQsocket</function>       Obtain the file descriptor number for the
backendconnection socket.
 
!       A valid descriptor will be >= 0; a result of -1 indicates that       no backend connection is currently open.
<synopsis>int PQsocket(const PGconn *conn); </synopsis> <function>PQsocket</function> should be used to obtain the
backendsocket descriptor in preparation for executing <function>select</function>(2).  This allows an
 
! application to wait for either backend responses or other conditions. If the result of <function>select</function>(2)
indicatesthat data can be read from the backend socket, then <function>PQconsumeInput</function> should be called to
readthe data; after which, <function>PQisBusy</function>, <function>PQgetResult</function>, and/or
<function>PQnotifies</function>can be used to process the response. </para> </listitem> 
 
--- 1321,1363 ----  <listitem> <para>
+ <function>PQflush</function> Attempt to flush any data queued to the backend,
+ returns 0 if successful (or if the send queue is empty) or EOF if it failed for
+ some reason.
+ <synopsis>
+ int PQflush(PGconn *conn);
+ </synopsis>
+ <function>PQflush</function> needs to be called on a non-blocking connection 
+ before calling <function>select</function> to determine if a responce has
+ arrived.  If 0 is returned it ensures that there is no data queued to the 
+ backend that has not actually been sent.  Only applications that have used
+ <function>PQsetnonblocking</function> have a need for this.
+ </para>
+ </listitem>
+ 
+ <listitem>
+ <para> <function>PQsocket</function>       Obtain the file descriptor number for the backend connection socket.
!       A valid descriptor will be >= 0; a result of -1 indicates that       no backend connection is currently
open.<synopsis> int PQsocket(const PGconn *conn); </synopsis> <function>PQsocket</function> should be used to obtain
thebackend socket descriptor in preparation for executing <function>select</function>(2).  This allows an
 
! application using a blocking connection to wait for either backend responses or
! other conditions. If the result of <function>select</function>(2) indicates that data can be read from the backend
socket,then <function>PQconsumeInput</function> should be called to read the data; after which,
<function>PQisBusy</function>,<function>PQgetResult</function>, and/or <function>PQnotifies</function> can be used to
processthe response.
 
+ </para>
+ <para>
+ Non-blocking connections (that have used <function>PQsetnonblocking</function>)
+ should not use <function>select</function> until <function>PQflush</function>
+ has returned 0 indicating that there is no buffered data waiting to be sent
+ to the backend. </para> </listitem> 
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.111
diff -u -c -r1.111 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    2000/01/16 21:18:52    1.111
--- src/interfaces/libpq/fe-connect.c    2000/01/17 02:35:56
***************
*** 594,624 ****     return 0; } 
- 
- /* ----------
-  * connectMakeNonblocking -
-  * Make a connection non-blocking.
-  * Returns 1 if successful, 0 if not.
-  * ----------
-  */
- static int
- connectMakeNonblocking(PGconn *conn)
- {
- #ifndef WIN32
-     if (fcntl(conn->sock, F_SETFL, O_NONBLOCK) < 0)
- #else
-     if (ioctlsocket(conn->sock, FIONBIO, &on) != 0)
- #endif
-     {
-         printfPQExpBuffer(&conn->errorMessage,
-                           "connectMakeNonblocking -- fcntl() failed: errno=%d\n%s\n",
-                           errno, strerror(errno));
-         return 0;
-     }
- 
-     return 1;
- }
-  /* ----------  * connectNoDelay -  * Sets the TCP_NODELAY socket option.
--- 594,599 ----
***************
*** 789,795 ****      *   Ewan Mellor <eem21@cam.ac.uk>.      * ---------- */ #if (!defined(WIN32) ||
defined(WIN32_NON_BLOCKING_CONNECTIONS))&& !defined(USE_SSL) 
!     if (!connectMakeNonblocking(conn))         goto connect_errReturn; #endif     
--- 764,770 ----      *   Ewan Mellor <eem21@cam.ac.uk>.      * ---------- */ #if (!defined(WIN32) ||
defined(WIN32_NON_BLOCKING_CONNECTIONS))&& !defined(USE_SSL)
 
!     if (PQsetnonblocking(conn, TRUE) != 0)         goto connect_errReturn; #endif     
***************
*** 898,904 ****     /* This makes the connection non-blocking, for all those cases which forced us        not to do it
above.*/ #if (defined(WIN32) && !defined(WIN32_NON_BLOCKING_CONNECTIONS)) || defined(USE_SSL)
 
!     if (!connectMakeNonblocking(conn))         goto connect_errReturn; #endif     
--- 873,879 ----     /* This makes the connection non-blocking, for all those cases which forced us        not to do it
above.*/ #if (defined(WIN32) && !defined(WIN32_NON_BLOCKING_CONNECTIONS)) || defined(USE_SSL)
 
!     if (PQsetnonblocking(conn, TRUE) != 0)         goto connect_errReturn; #endif     
***************
*** 1720,1725 ****
--- 1695,1701 ----     conn->inBuffer = (char *) malloc(conn->inBufSize);     conn->outBufSize = 8 * 1024;
conn->outBuffer= (char *) malloc(conn->outBufSize);
 
+     conn->nonblocking = FALSE;     initPQExpBuffer(&conn->errorMessage);     initPQExpBuffer(&conn->workBuffer);
if(conn->inBuffer == NULL ||
 
***************
*** 1830,1835 ****
--- 1806,1812 ----     conn->lobjfuncs = NULL;     conn->inStart = conn->inCursor = conn->inEnd = 0;     conn->outCount
=0;
 
+     conn->nonblocking = FALSE;  } 
Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.86
diff -u -c -r1.86 fe-exec.c
*** src/interfaces/libpq/fe-exec.c    1999/11/11 00:10:14    1.86
--- src/interfaces/libpq/fe-exec.c    2000/01/14 22:47:07
***************
*** 13,18 ****
--- 13,19 ----  */ #include <errno.h> #include <ctype.h>
+ #include <fcntl.h>  #include "postgres.h" #include "libpq-fe.h"
***************
*** 24,30 **** #include <unistd.h> #endif 
-  /* keep this in same order as ExecStatusType in libpq-fe.h */ const char *const pgresStatus[] = {
"PGRES_EMPTY_QUERY",
--- 25,30 ----
***************
*** 514,526 ****     conn->curTuple = NULL;      /* send the query to the backend; */
!     /* the frontend-backend protocol uses 'Q' to designate queries */
!     if (pqPutnchar("Q", 1, conn) ||
!         pqPuts(query, conn) ||
!         pqFlush(conn))     {
!         handleSendFailure(conn);
!         return 0;     }      /* OK, it's launched! */
--- 514,566 ----     conn->curTuple = NULL;      /* send the query to the backend; */
! 
!     /*
!      * in order to guarantee that we don't send a partial query 
!      * where we would become out of sync with the backend and/or
!      * block during a non-blocking connection we must first flush
!      * the send buffer before sending more data
!      *
!      * an alternative is to implement 'queue reservations' where
!      * we are able to roll up a transaction 
!      * (the 'Q' along with our query) and make sure we have
!      * enough space for it all in the send buffer.
!      */
!     if (pqIsnonblocking(conn))     {
!         /*
!          * the buffer must have emptied completely before we allow
!          * a new query to be buffered
!          */
!         if (pqFlush(conn))
!             return 0;
!         /* 'Q' == queries */
!         /* XXX: if we fail here we really ought to not block */
!         if (pqPutnchar("Q", 1, conn) ||
!             pqPuts(query, conn))
!         {
!             handleSendFailure(conn);    
!             return 0;
!         }
!         /*
!          * give the data a push, ignore the return value as
!          * ConsumeInput() will do any aditional flushing if needed
!          */
!         (void) pqFlush(conn);    
!     }
!     else
!     {
!         /* 
!          * the frontend-backend protocol uses 'Q' to 
!          * designate queries 
!          */
!         if (pqPutnchar("Q", 1, conn) ||
!             pqPuts(query, conn) ||
!             pqFlush(conn))
!         {
!             handleSendFailure(conn);
!             return 0;
!         }     }      /* OK, it's launched! */
***************
*** 574,580 ****
--- 614,630 ----      * we will NOT block waiting for more input.      */     if (pqReadData(conn) < 0)
+     {
+         /*
+          * for non-blocking connections
+          * try to flush the send-queue otherwise we may never get a 
+          * responce for something that may not have already been sent
+          * because it's in our write buffer!
+          */
+         if (pqIsnonblocking(conn))
+             (void) pqFlush(conn);         return 0;
+     }     /* Parsing of the data waits till later. */     return 1; }
***************
*** 1088,1093 ****
--- 1138,1153 ---- {     PGresult   *result;     PGresult   *lastResult;
+     bool    savedblocking;
+ 
+     /*
+      * we assume anyone calling PQexec wants blocking behaviour,
+      * we force the blocking status of the connection to blocking
+      * for the duration of this function and restore it on return
+      */
+     savedblocking = pqIsnonblocking(conn);
+     if (PQsetnonblocking(conn, FALSE) == -1)
+         return NULL;      /*      * Silently discard any prior query result that application didn't
***************
*** 1102,1115 ****             PQclear(result);             printfPQExpBuffer(&conn->errorMessage,
"PQexec:you gotta get out of a COPY state yourself.\n");
 
!             return NULL;         }         PQclear(result);     }      /* OK to send the message */     if
(!PQsendQuery(conn,query))
 
!         return NULL;      /*      * For backwards compatibility, return the last result if there are
--- 1162,1176 ----             PQclear(result);             printfPQExpBuffer(&conn->errorMessage,
"PQexec:you gotta get out of a COPY state yourself.\n");
 
!             /* restore blocking status */
!             goto errout;         }         PQclear(result);     }      /* OK to send the message */     if
(!PQsendQuery(conn,query))
 
!         goto errout;    /* restore blocking status */      /*      * For backwards compatibility, return the last
resultif there are
 
***************
*** 1142,1148 ****
--- 1203,1217 ----             result->resultStatus == PGRES_COPY_OUT)             break;     }
+ 
+     if (PQsetnonblocking(conn, savedblocking) == -1)
+         return NULL;     return lastResult;
+ 
+ errout:
+     if (PQsetnonblocking(conn, savedblocking) == -1)
+         return NULL;
+     return NULL; }  
***************
*** 1431,1438 ****              "PQendcopy() -- I don't think there's a copy in progress.\n");         return 1;     }

!     (void) pqFlush(conn);        /* make sure no data is waiting to be sent */      /* Return to active duty */
conn->asyncStatus= PGASYNC_BUSY;
 
--- 1500,1516 ----              "PQendcopy() -- I don't think there's a copy in progress.\n");         return 1;     }
+ 
+     /*
+      * make sure no data is waiting to be sent, 
+      * abort if we are non-blocking and the flush fails
+      */
+     if (pqFlush(conn) && pqIsnonblocking(conn))
+         return (1); 
!     /* non blocking connections may have to abort at this point. */
!     if (pqIsnonblocking(conn) && PQisBusy(conn))
!         return (1);      /* Return to active duty */     conn->asyncStatus = PGASYNC_BUSY;
***************
*** 2025,2028 ****
--- 2103,2192 ----         return 1;     else         return 0;
+ }
+ 
+ /* PQsetnonblocking:
+      sets the PGconn's database connection non-blocking if the arg is TRUE
+      or makes it non-blocking if the arg is FALSE, this will not protect
+      you from PQexec(), you'll only be safe when using the non-blocking
+      API
+      Needs to be called only on a connected database connection.
+ */
+ 
+ int
+ PQsetnonblocking(PGconn *conn, int arg)
+ {
+     int    fcntlarg;
+ 
+     arg = (arg == TRUE) ? 1 : 0;
+     /* early out if the socket is already in the state requested */
+     if (arg == conn->nonblocking)
+         return (0);
+ 
+     /*
+      * to guarantee constancy for flushing/query/result-polling behavior
+      * we need to flush the send queue at this point in order to guarantee
+      * proper behavior.
+      * this is ok because either they are making a transition
+      *  _from_ or _to_ blocking mode, either way we can block them.
+      */
+     /* if we are going from blocking to non-blocking flush here */
+     if (!pqIsnonblocking(conn) && pqFlush(conn))
+         return (-1);
+ 
+ 
+ #ifdef USE_SSL
+     if (conn->ssl)
+     {
+         printfPQExpBuffer(&conn->errorMessage,
+             "PQsetnonblocking() -- not supported when using SSL\n");
+         return (-1);
+     }
+ #endif /* USE_SSL */
+ 
+ #ifndef WIN32
+     fcntlarg = fcntl(conn->sock, F_GETFL, 0);
+     if (fcntlarg == -1)
+         return (-1);
+ 
+     if ((arg == TRUE && 
+         fcntl(conn->sock, F_SETFL, fcntlarg | O_NONBLOCK) == -1) ||
+         (arg == FALSE &&
+         fcntl(conn->sock, F_SETFL, fcntlarg & ~O_NONBLOCK) == -1)) 
+ #else
+     fcntlarg = arg;
+     if (ioctlsocket(conn->sock, FIONBIO, &fcntlarg) != 0)
+ #endif
+     {
+         printfPQExpBuffer(&conn->errorMessage,
+             "PQsetblocking() -- unable to set nonblocking status to %s\n",
+             arg == TRUE ? "TRUE" : "FALSE");
+         return (-1);
+     }
+ 
+     conn->nonblocking = arg;
+ 
+     /* if we are going from non-blocking to blocking flush here */
+     if (pqIsnonblocking(conn) && pqFlush(conn))
+         return (-1);
+ 
+     return (0);
+ }
+ 
+ /* return the blocking status of the database connection, TRUE == nonblocking,
+      FALSE == blocking
+ */
+ int
+ PQisnonblocking(const PGconn *conn)
+ {
+ 
+     return (pqIsnonblocking(conn));
+ }
+ 
+ /* try to force data out, really only useful for non-blocking users */
+ int
+ PQflush(PGconn *conn)
+ {
+ 
+     return (pqFlush(conn)); }
Index: src/interfaces/libpq/fe-misc.c
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.33
diff -u -c -r1.33 fe-misc.c
*** src/interfaces/libpq/fe-misc.c    1999/11/30 03:08:19    1.33
--- src/interfaces/libpq/fe-misc.c    2000/01/12 03:12:14
***************
*** 86,91 ****
--- 86,122 ---- {     size_t avail = Max(conn->outBufSize - conn->outCount, 0); 
+     /*
+      * if we are non-blocking and the send queue is too full to buffer this
+      * request then try to flush some and return an error 
+      */
+     if (pqIsnonblocking(conn) && nbytes > avail && pqFlush(conn))
+     {
+         /* 
+          * even if the flush failed we may still have written some
+          * data, recalculate the size of the send-queue relative
+          * to the amount we have to send, we may be able to queue it
+          * afterall even though it's not sent to the database it's
+          * ok, any routines that check the data coming from the
+          * database better call pqFlush() anyway.
+          */
+         if (nbytes > Max(conn->outBufSize - conn->outCount, 0))
+         {
+             printfPQExpBuffer(&conn->errorMessage,
+                 "pqPutBytes --  pqFlush couldn't flush enough"
+                 " data: space available: %d, space needed %d\n",
+                 Max(conn->outBufSize - conn->outCount, 0), nbytes);
+             return EOF;
+         }
+     }
+ 
+     /* 
+      * is the amount of data to be sent is larger than the size of the
+      * output buffer then we must flush it to make more room.
+      *
+      * the code above will make sure the loop conditional is never 
+      * true for non-blocking connections
+      */     while (nbytes > avail)     {         memcpy(conn->outBuffer + conn->outCount, s, avail);
***************
*** 548,553 ****
--- 579,592 ----         return EOF;     } 
+     /* 
+      * don't try to send zero data, allows us to use this function
+      * without too much worry about overhead
+      */
+     if (len == 0)
+         return (0);
+ 
+     /* while there's still data to send */     while (len > 0)     {         /* Prevent being SIGPIPEd if backend has
closedthe connection. */
 
***************
*** 556,561 ****
--- 595,601 ---- #endif          int sent;
+  #ifdef USE_SSL         if (conn->ssl)            sent = SSL_write(conn->ssl, ptr, len);
***************
*** 585,590 ****
--- 625,632 ----                 case EWOULDBLOCK:                     break; #endif
+                 case EINTR:
+                     continue;                  case EPIPE: #ifdef ECONNRESET
***************
*** 616,628 ****             ptr += sent;             len -= sent;         }         if (len > 0)         {
/*We didn't send it all, wait till we can send more */ 
 
-             /* At first glance this looks as though it should block.  I think
-              * that it will be OK though, as long as the socket is
-              * non-blocking. */             if (pqWait(FALSE, TRUE, conn))                 return EOF;         }
--- 658,688 ----             ptr += sent;             len -= sent;         }
+          if (len > 0)         {             /* We didn't send it all, wait till we can send more */
+ 
+             /* 
+              * if the socket is in non-blocking mode we may need
+              * to abort here 
+              */
+ #ifdef USE_SSL
+             /* can't do anything for our SSL users yet */
+             if (conn->ssl == NULL)
+             {
+ #endif
+                 if (pqIsnonblocking(conn))
+                 {
+                     /* shift the contents of the buffer */
+                     memmove(conn->outBuffer, ptr, len);
+                     conn->outCount = len;
+                     return EOF;
+                 }
+ #ifdef USE_SSL
+             }
+ #endif              if (pqWait(FALSE, TRUE, conn))                 return EOF;         }
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.55
diff -u -c -r1.55 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    2000/01/15 05:37:21    1.55
--- src/interfaces/libpq/libpq-fe.h    2000/01/17 02:35:56
***************
*** 263,268 ****
--- 263,275 ----     extern int    PQputnbytes(PGconn *conn, const char *buffer, int nbytes);     extern int
PQendcopy(PGconn*conn); 
 
+     /* Set blocking/nonblocking connection to the backend */
+     extern int    PQsetnonblocking(PGconn *conn, int arg);
+     extern int    PQisnonblocking(const PGconn *conn);
+ 
+     /* Force the write buffer to be written (or at least try) */
+     extern int    PQflush(PGconn *conn);
+      /*      * "Fast path" interface --- not really recommended for application      * use
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.16
diff -u -c -r1.16 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    2000/01/15 05:37:21    1.16
--- src/interfaces/libpq/libpq-int.h    2000/01/17 02:35:56
***************
*** 214,219 ****
--- 214,222 ----     int            inEnd;            /* offset to first position after avail
      * data */ 
 
+     int            nonblocking;    /* whether this connection is using a blocking
+                                  * socket to the backend or not */
+      /* Buffer for data not yet sent to backend */     char       *outBuffer;        /* currently allocated buffer */
   int            outBufSize;        /* allocated size of buffer */
 
***************
*** 299,303 ****
--- 302,312 ---- #define strerror(A) (sys_errlist[(A)]) #endif     /* sunos4 */ #endif     /* !strerror */
+ 
+ /* 
+  * this is so that we can check is a connection is non-blocking internally
+  * without the overhead of a function call
+  */
+ #define pqIsnonblocking(conn)    (conn->nonblocking)  #endif     /* LIBPQ_INT_H */