Re: [HACKERS] LIBPQ patches ...

Looks fine. I have talked to someone about doing no-blocking
connections in the past. Maybe this the same person.

I will let someone else comment on whether the protocol changes are
correct.

>
> Does anyone have anything against me applying this to the current source
> tree?
>
>
> Marc G. Fournier ICQ#7615664 IRC Nick: Scrappy
> Systems Administrator @ hub.org
> primary: scrappy(at)hub(dot)org secondary: scrappy(at){freebsd|postgresql}.org
>
> ---------- Forwarded message ----------
> Date: Fri, 17 Dec 1999 13:51:50 -0800 (PST)
> From: Alfred Perlstein <bright(at)wintelcom(dot)net>
> To: The Hermit Hacker <scrappy(at)hub(dot)org>
> Subject: Re: pctrackd updates and such
>
> On Fri, 17 Dec 1999, The Hermit Hacker wrote:
>
> >
> > Okay, first thing...can you redo these as context diffs? We generally
> > refuse *any* patches that aren't context...
>
> sure.
>
> >
> > Second, are these against a reasonably current snapshot of PostgreSQL
> > (aka. the upcoming v7), or v6.5.3 release? If v6.5.3, we're gonna need to
> > get these v7.x ready before we can commit them...
>
> they are against a checked out cvs copy as of a couple days ago,
> and should apply cleanly to what's in the current repo.
>
> > Once both of the above conditions are in place, and after I get back from
> > BC, I'll work on getting these into the v7.0 release...or, at least,
> > talked/commented about if there are any objections...
> >
> > I'm outta here for 10 days...Happy Holidays and talk with ya when I get
> > back...
>
> ok, cool see you soon. :)
>
> -Alfred
>
> don't forget the problem with sending queries that may occur:
>
> i'm not sure if handlesendfailure() can cope with only sending
> a 'Q' to the backend, we may have to work out reservations or
> something for space, another idea would be to implement a
> pqWritev() of some sort that would take an array of pointers
> and lengths to send to the backend and only allow any data to
> go into the backend if the entire string can fit.
>
> then again, handlesendfailure may work, but doing reservations
> for the send buffer seems cleaner...
>
> diff's contexted against pgsql-'current':
>
>
> Index: fe-connect.c
> ===================================================================
> RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.108
> diff -u -c -r1.108 fe-connect.c
> cvs diff: conflicting specifications of output style
> *** fe-connect.c 1999/12/02 00:26:15 1.108
> --- fe-connect.c 1999/12/14 09:42:24
> ***************
> *** 595,625 ****
> 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.
> --- 595,600 ----
> ***************
> *** 792,798 ****
> * Ewan Mellor <eem21(at)cam(dot)ac(dot)uk>.
> * ---------- */
> #if (!defined(WIN32) || defined(WIN32_NON_BLOCKING_CONNECTIONS)) && !defined(USE_SSL)
> ! if (!connectMakeNonblocking(conn))
> goto connect_errReturn;
> #endif
>
> --- 767,773 ----
> * Ewan Mellor <eem21(at)cam(dot)ac(dot)uk>.
> * ---------- */
> #if (!defined(WIN32) || defined(WIN32_NON_BLOCKING_CONNECTIONS)) && !defined(USE_SSL)
> ! if (PQsetnonblocking(conn, TRUE) != 0)
> goto connect_errReturn;
> #endif
>
> ***************
> *** 904,910 ****
> /* 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
>
> --- 879,885 ----
> /* 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
>
> ***************
> *** 1702,1707 ****
> --- 1677,1683 ----
> 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 ||
> ***************
> *** 1811,1816 ****
> --- 1787,1793 ----
> conn->lobjfuncs = NULL;
> conn->inStart = conn->inCursor = conn->inEnd = 0;
> conn->outCount = 0;
> + conn->nonblocking = FALSE;
>
> }
>
> Index: 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
> cvs diff: conflicting specifications of output style
> *** fe-exec.c 1999/11/11 00:10:14 1.86
> --- fe-exec.c 1999/12/14 05:55:11
> ***************
> *** 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 ----
> ***************
> *** 574,580 ****
> --- 574,588 ----
> * we will NOT block waiting for more input.
> */
> if (pqReadData(conn) < 0)
> + {
> + /*
> + * try to flush the send-queue otherwise we may never get a
> + * resonce for something that may not have already been sent
> + * because it's in our write buffer!
> + */
> + pqFlush(conn);
> return 0;
> + }
> /* Parsing of the data waits till later. */
> return 1;
> }
> ***************
> *** 1088,1095 ****
> --- 1096,1112 ----
> {
> 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);
> + PQsetnonblocking(conn, FALSE);
> +
> + /*
> * Silently discard any prior query result that application didn't
> * eat. This is probably poor design, but it's here for backward
> * compatibility.
> ***************
> *** 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
> --- 1119,1133 ----
> 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;
>
> /*
> * For backwards compatibility, return the last result if there are
> ***************
> *** 1142,1148 ****
> --- 1160,1172 ----
> result->resultStatus == PGRES_COPY_OUT)
> break;
> }
> +
> + PQsetnonblocking(conn, savedblocking);
> return lastResult;
> +
> + errout:
> + PQsetnonblocking(conn, savedblocking);
> + 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;
> --- 1455,1468 ----
> "PQendcopy() -- I don't think there's a copy in progress.\n");
> return 1;
> }
> +
> + /* make sure no data is waiting to be sent */
> + if (pqFlush(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 ****
> --- 2055,2126 ----
> 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;
> + if (arg == conn->nonblocking)
> + return (0);
> +
> + #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;
> + return (0);
> + }
> +
> + /* return the blocking status of the database connection, TRUE == nonblocking,
> + FALSE == blocking
> + */
> + int
> + PQisnonblocking(PGconn *conn)
> + {
> +
> + return (conn->nonblocking);
> + }
> +
> + /* try to force data out, really only useful for non-blocking users */
> + int
> + PQflush(PGconn *conn)
> + {
> +
> + return (pqFlush(conn));
> }
> Index: 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
> cvs diff: conflicting specifications of output style
> *** fe-misc.c 1999/11/30 03:08:19 1.33
> --- fe-misc.c 1999/12/14 08:21:09
> ***************
> *** 86,91 ****
> --- 86,119 ----
> {
> 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;
> + }
> + }
> +
> + /*
> + * the non-blocking code above makes sure that this isn't true,
> + * essentially this is no-op
> + */
> while (nbytes > avail)
> {
> memcpy(conn->outBuffer + conn->outCount, s, avail);
> ***************
> *** 548,553 ****
> --- 576,589 ----
> 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 closed the connection. */
> ***************
> *** 556,561 ****
> --- 592,598 ----
> #endif
>
> int sent;
> +
> #ifdef USE_SSL
> if (conn->ssl)
> sent = SSL_write(conn->ssl, ptr, len);
> ***************
> *** 585,590 ****
> --- 622,629 ----
> 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;
> }
> --- 655,685 ----
> 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: libpq-fe.h
> ===================================================================
> RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/libpq-fe.h,v
> retrieving revision 1.53
> diff -u -c -r1.53 libpq-fe.h
> cvs diff: conflicting specifications of output style
> *** libpq-fe.h 1999/11/30 03:08:19 1.53
> --- libpq-fe.h 1999/12/14 01:30:01
> ***************
> *** 269,274 ****
> --- 269,281 ----
> 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(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: libpq-int.h
> ===================================================================
> RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/libpq-int.h,v
> retrieving revision 1.14
> diff -u -c -r1.14 libpq-int.h
> cvs diff: conflicting specifications of output style
> *** libpq-int.h 1999/11/30 03:08:19 1.14
> --- libpq-int.h 1999/12/14 01:30:01
> ***************
> *** 215,220 ****
> --- 215,223 ----
> 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 */
>
>
>
>
>
> ************
>

--
Bruce Momjian | http://www.op.net/~candle
maillist(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026