Re: [HACKERS] pg_dump disaster

* Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> [000120 22:13] wrote:
> > Patrick Welche <prlw1(at)newn(dot)cam(dot)ac(dot)uk> writes:
> > > Trying to load a 6 January pg_dumpall file with today's postgresql gives
> > > many
> > > invalid command \N
> > > probably because
> > > PQexec: you gotta get out of a COPY state yourself.
> > > db.out4:11621: invalid command \.
> > > Somehow psql is out of sync and thinks the \N isn't within a COPY block.
> >
> > The answer appears to be that Perlstein's "nonblocking mode" patches
> > have broken psql copy, and doubtless a lot of other applications as
> > well, because pqPutBytes no longer feels any particular compulsion
> > to actually send the data it's been handed. (Moreover, if it does
> > do only a partial send, there is no way to discover how much it sent;
> > while its callers might be blamed for not having checked for an error
> > return, they'd have no way to recover anyhow.)
> >
> > I thought these patches should not have been applied without more
> > peer review, and now I'm sure of it. I recommend reverting 'em.
>
> Can we give the submitter a few days to address the issue?

Tom is wrong in his assessment, this is exactly the reason I brought
the patches in.

pqPutBytes _never_ felt any compulsion to flush the buffer to the backend,
or at least not since I started using it. The only change I made was
for send reservations to be made for non-blocking connections, nothing
inside of postgresql uses non-blocking connections.

pqPutBytes from revision 1.33: (plus my commentary)

static int
pqPutBytes(const char *s, size_t nbytes, PGconn *conn)
{
size_t avail = Max(conn->outBufSize - conn->outCount, 0);

/* while the amount to send is greater than the output buffer */
while (nbytes > avail)
{
/* copy as much as we can into the buffer */
memcpy(conn->outBuffer + conn->outCount, s, avail);
conn->outCount += avail;
s += avail;
nbytes -= avail;
/* try to flush it.... */
if (pqFlush(conn))
return EOF;
avail = conn->outBufSize;
}

/* XXX: ok, we have enough room... where is the flush? */
memcpy(conn->outBuffer + conn->outCount, s, nbytes);
conn->outCount += nbytes;

return 0;
}

I may have introduced bugs elsewhere, but i doubt it was in pqPutBytes.

This is the exact problem I was describing and why I felt that routines
that checked for data needed to flush beforehand, there may have been
data that still needed to be sent.

I'll be reviewing my own changes again to make sure I haven't broken
something else that could be causing this.

The implications of this is trully annoying, exporting the socket to
the backend to the client application causes all sorts of problems because
the person select()'ing on the socket sees that it's 'clear' but yet
all thier data has not been sent...

-Alfred

>
> --
> Bruce Momjian | http://www.op.net/~candle
> pgman(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

--
-Alfred Perlstein - [bright(at)wintelcom(dot)net|alfred(at)freebsd(dot)org]