Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Nov 12, 2015 at 9:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Just for the record, this patch changes no behavior whatsoever.
> Actually, yes it does. I tested. Without the patch, the message sent
> by ac1d7945f866b1928c2554c0f80fd52d7f977772 isn't received by psql.
> With the patch, it is.
Hmm ....
>> It could only make a difference if the backend were still actively
>> sending data, which certainly isn't so in the case claimed by the
>> commit message. Moreover, removing the comment that explains why
>> only NOTICE and NOTIFY messages will be eaten doesn't make it not so.
> I did post an analysis of why I believe these changes are correct on
> pgsql-hackers. Did you read it? If it's wrong, perhaps you could
> respond to that thread. In short, the old comment is appears to me to
> be factually wrong: the claim that PGASYNC_IDLE ignores everything but
> NOTICE and NOTIFY is at odds with the actual behavior of
> pqParseInput3.
[ pokes around... ] Yeah, that comment was out of date: it was obsoleted
by 58f481c4, but I missed updating this comment.
The point I was trying to make, though, was that whether parseInput will
process an 'E' message depends on the asyncStatus, and neither pqReadData
nor parseInput should change the asyncStatus if it's initially IDLE.
So on first glance, pumping pqReadData till it's dry and then doing
parseInput once should have the same effect as the new coding. It's
not wrong to include a parseInput in the loop, and if there are multiple
messages available it might avoid buffer bloat, but this should not fix
any problem.
After looking around, I suspect what actually happened in your test
was that we kept pumping pqReadData until it realized it was seeing EOF,
whereupon it did pqDropConnection(), and guess what that does:
/* Discard any unread/unsent data */
conn->inStart = conn->inCursor = conn->inEnd = 0;
conn->outCount = 0;
So the reason your patch appears to fix it is that you invoked parseInput
before the input buffer got wiped. But I think a better fix would be to
remove the input-buffer flushing here. Flushing the output buffer is
fine, since the data can never get sent, but it's a mistake to throw away
unprocessed data like that. And there's surely no API guarantee by
pqReadData that the timing of its noticing EOF always works like this.
regards, tom lane
