Re: pg_dump possible fix, need testers. (was: Re: [HACKERS] pg_dump disaster)

Alfred Perlstein <bright@wintelcom.net> writes:
> There is no non-block mode, there's the old mode, and the _real_
> non-blocking mode that I'm trying to get working.

Ah --- well, I guess we can agree that it doesn't work yet ;-)

>>>> user sends data almost filling the output buffer...
>>>> user sends another line of text overflowing the buffer...
>>>> pqFlush is invoked blocking the user until the output pipe clears...
>>>> and repeat.
>> 
>> Right.

> You agree that it's somewhat broken to do that right?

It's only broken for a client that doesn't want to block --- but,
clearly, for such a client we can't do it that way.  I like the way
that fe-connect.c has been rewritten over the past couple of months:
provide a "start" and a "poll" function for each operation, and then
implement the old blocking-style function as "start" followed by a
loop around the "poll" function.  Note (just to beat the drum one
more time) that neither the start nor the poll function has the
same API as the old-style function.

>> You can't simply refuse to support the case nbytes > bufsize at all,
>> because that will cause application failures as well (too long query
>> sends it into an infinite loop trying to queue data, most likely).

> I don't have to think about this too much (nbytes > conn->outBufSize),
> see:  http://www.postgresql.org/docs/programmer/libpq-chapter4142.htm
> the Caveats section for libpq:

>   The query buffer is 8192 bytes long, and queries over that length
>   will be rejected.

Is that still there?  Well, I'm sorry if you relied on that statement,
but it's obsolete.  Postgres no longer has any fixed limit on the
length of queries, and it won't do for libpq to re-introduce one.

(Quick grep: yipes, that statement is indeed still in the docs, in
more than one place even.  Thanks for pointing this out.)

> If I can enforce this limit then i'm fine, also there's nothing
> stopping me from temporarily realloc()'ing the send buffer, or
> chaining another sendbuffer if the first fills mid query, it would
> be easy to restrict the application to only a single overcommit of
> the send buffer, or a single circular buffer to avoid memory
> copies.

I think reallocing the send buffer is perfectly acceptable.  I'm not
quite following what you mean by "only one overcommit", though.

>> PQendcopy has the same interface design problem.  It used to be that
>> (unless you passed a null pointer) PQendcopy would *guarantee* that
>> the connection was no longer in COPY state on return --- by resetting
>> it, if necessary.  So the return code was mainly informative; the
>> application didn't have to do anything different if PQendcopy reported
>> failure.  But now, a nonblocking application does need to pay attention
>> to whether PQendcopy completed or not --- and you haven't provided a way
>> for it to tell.  If 1 is returned, the connection might still be in
>> COPY state, or it might not (PQendcopy might have reset it).  If the
>> application doesn't distinguish these cases then it will fail.

> PQstatus will allow you to determine if the connection has gone to
> CONNECTION_BAD.

Neither of the states that we need to distinguish will be CONNECTION_BAD.

I suppose the application could look at the asyncStatus to see if it is
COPY_IN/COPY_OUT, but I think that would be very poor software design:
asyncStatus is intended to be private to libpq and readily redefinable.
(It's not presently visible in libpq-fe.h at all.)  If we start
exporting it as public data we will regret it, IMHO.  Much better to
provide a new exported function whose API is appropriate for this purpose.

>> I don't think that the nonblock code is anywhere near production quality
>> at this point.  It may work for you, if you don't stress it too hard and
>> never have a communications failure; but I don't want to see us ship it
>> as part of Postgres unless these issues get addressed.

> I'd really appreciate if it was instead left as undocumented until we
> have it completed.

I'd be willing to consider that if I were sure that it couldn't break
any ordinary blocking-mode cases.  Right now I don't have much
confidence about that...
        regards, tom lane