Обсуждение: non-blocking connections in libpq, fix proposal
Hi,
after finding out that libpq apparently doesn't work properly when
sending long queries ('long' meaning somewhere larger than 8KB), I had a
look at the sources and also found some mails in the archives where this
issue had been discussed. The problem appears to be still present in the
current CVS version.
I've worked on a fix today that works for me. It's perhaps not the best
solution but it's simple :-)
The basic strategy is to fix pqFlush and pqPutBytes.
The problem with pqFlush as it stands now is that it returns EOF when an
error occurs or when not all data could be sent. The latter case is
clearly not an error for a non-blocking connection but the caller can't
distringuish it from an error very well.
The first part of the fix is therefore to fix pqFlush. This is done by
to renaming it to pqFlushSome which only differs from pqFlush in its
return values to allow the caller to make the above distinction and a
new pqFlush which is implemented in terms of pqFlushSome and behaves
exactly like the old pqFlush.
The second part of the fix modifies pqPutBytes to use pqFlushSome
instead of pqFlush and to either send all the data or if not all data
can be sent on a non-blocking connection to at least put all data into
the output buffer, enlarging it if necessary.
I've also added a new API function PQflushSome which analogously to
PQflush just calls pqFlushSome. Programs using PQsendQuery should use
this new function. The main difference is that this function will have
to be called repeatedly (calling select() properly in between) until all
data has been written.
Being new to postgresql development I'm not completely sure how to
proceed from here. Is it OK if I post the patch here?
Bernhard
--
Intevation GmbH http://intevation.de/
Sketch http://sketch.sourceforge.net/
MapIt! http://mapit.de/
Bernhard Herzog <bh@intevation.de> writes:
> after finding out that libpq apparently doesn't work properly when
> sending long queries ('long' meaning somewhere larger than 8KB), I had a
> look at the sources and also found some mails in the archives where this
> issue had been discussed. The problem appears to be still present in the
> current CVS version.
Indeed, the "non blocking" support in libpq is completely broken IMHO,
and has been since it was put in; but I've not had time to work on it
myself. And the original developer of the feature apparently never
stressed it hard enough to have a problem, so he insisted there was
no problem :-( I think you're the first person to come along and
actually want to do something about it.
> The problem with pqFlush as it stands now is that it returns EOF when an
> error occurs or when not all data could be sent. The latter case is
> clearly not an error for a non-blocking connection but the caller can't
> distringuish it from an error very well.
Right. The real problem with the design is that the return codes of a
lot of routines aren't adequate to distinguish these cases. However,
I'm dubious that pqFlush is the only place where there's a problem;
AFAIR the problem propagates out to quite a lot of places.
> The second part of the fix modifies pqPutBytes to use pqFlushSome
> instead of pqFlush and to either send all the data or if not all data
> can be sent on a non-blocking connection to at least put all data into
> the output buffer, enlarging it if necessary.
I think that was the same approach I had in mind awhile back --- if
pqPutBytes only fails when it can neither send nor queue data, then
that reduces the number of places that need to cope with "can't send
yet" conditions.
> I've also added a new API function PQflushSome which analogously to
> PQflush just calls pqFlushSome. Programs using PQsendQuery should use
> this new function.
You mean "programs using nonblocking send should use this function", no?
I don't like the name "pqFlushSome" --- to me "flush" means "get rid of
all the data or else". Maybe "pqSendSome"? Any other ideas out there?
> Being new to postgresql development I'm not completely sure how to
> proceed from here. Is it OK if I post the patch here?
Patches should go to pgsql-patches, especially if they're more than
a few lines long.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Bernhard Herzog <bh@intevation.de> writes: > Right. The real problem with the design is that the return codes of a > lot of routines aren't adequate to distinguish these cases. However, > I'm dubious that pqFlush is the only place where there's a problem; > AFAIR the problem propagates out to quite a lot of places. pqFlush is called in a lot of places, that's true. However, it seems to me that in many of those places, it doesn't have to be called at all. For one, for a blocking connection, pqFlush only needs to be called after data has been put into the output buffer and most public functions (and many internal functions) already do that before they return (notable exceptions: PQputline and PQputnbytes). Calling pqFlush at the beginning of a function shouldn't be necessary. For a non-blocking connection, the program using the library can be expected to call PQsendSome until all pending data has been sent, I think, so it wouldn't be necessary to call pqFlush/pqSendSome at the beginning of e.g. PQsendQuery. PQsendQuery should perhaps check whether the buffer is empty instead. > I think that was the same approach I had in mind awhile back --- if > pqPutBytes only fails when it can neither send nor queue data, then > that reduces the number of places that need to cope with "can't send > yet" conditions. Exactly. that's why I did it that way. I tried to make as few changes as possible. > > I've also added a new API function PQflushSome which analogously to > > PQflush just calls pqFlushSome. Programs using PQsendQuery should use > > this new function. > > You mean "programs using nonblocking send should use this function", no? Yes. > I don't like the name "pqFlushSome" --- to me "flush" means "get rid of > all the data or else". Maybe "pqSendSome"? Any other ideas out there? I'll call it pqSendSome for now. > Patches should go to pgsql-patches, especially if they're more than > a few lines long. I've sent it there. Regards, Bernhard -- Intevation GmbH http://intevation.de/ Sketch http://sketch.sourceforge.net/ MapIt! http://mapit.de/
Thread has been saved for the 7.3 release: http://candle.pha.pa.us/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Bernhard Herzog wrote: > > Hi, > > after finding out that libpq apparently doesn't work properly when > sending long queries ('long' meaning somewhere larger than 8KB), I had a > look at the sources and also found some mails in the archives where this > issue had been discussed. The problem appears to be still present in the > current CVS version. > > I've worked on a fix today that works for me. It's perhaps not the best > solution but it's simple :-) > > The basic strategy is to fix pqFlush and pqPutBytes. > > The problem with pqFlush as it stands now is that it returns EOF when an > error occurs or when not all data could be sent. The latter case is > clearly not an error for a non-blocking connection but the caller can't > distringuish it from an error very well. > > The first part of the fix is therefore to fix pqFlush. This is done by > to renaming it to pqFlushSome which only differs from pqFlush in its > return values to allow the caller to make the above distinction and a > new pqFlush which is implemented in terms of pqFlushSome and behaves > exactly like the old pqFlush. > > The second part of the fix modifies pqPutBytes to use pqFlushSome > instead of pqFlush and to either send all the data or if not all data > can be sent on a non-blocking connection to at least put all data into > the output buffer, enlarging it if necessary. > > I've also added a new API function PQflushSome which analogously to > PQflush just calls pqFlushSome. Programs using PQsendQuery should use > this new function. The main difference is that this function will have > to be called repeatedly (calling select() properly in between) until all > data has been written. > > > Being new to postgresql development I'm not completely sure how to > proceed from here. Is it OK if I post the patch here? > > > Bernhard > > > -- > Intevation GmbH http://intevation.de/ > Sketch http://sketch.sourceforge.net/ > MapIt! http://mapit.de/ > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026