Hi,
On 2022-02-10 19:44:57 -0500, Tom Lane wrote:
> BTW, this also changes what is looking to me like a thinko in
> my_sock_write(): if we get a zero result from pqsecure_raw_write(),
> we should not be consulting errno, because it wouldn't have
> been set. Am I missing something there?
It does look like an oversight. I think it's effectively harmless, because
pqsecure_raw_write() will always set up result_errno = 0, and it doesn't make
the same mistake of checking errno with a 0 result.
> + * If a socket-level write failure occurs, conn->write_failed is set and the
> + * error message is saved in conn->write_err_msg, but we clear the output
> + * buffer and return zero anyway; this is because callers should soldier on
> + * until it's possible to read from the server and check for an error message.
> + * write_err_msg should be reported only when we are unable to obtain a server
> + * error first. Much of that behavior is implemented at lower levels, but
> + * this function deals with some edge cases.
The "soldier on until" bit seems to imply something stronger than how I
understand this to work. Makes it sound like we're going to busy loop or such
if necessary. Maybe something like "should soldier on until they have tried to
read from the server, so that server error messages can be processed"?
It's not in this change, but related. I don't really understand the
/*
* PQgetResult will return immediately in all states except BUSY, or if we
* had a write failure.
*/
return conn->asyncStatus == PGASYNC_BUSY || conn->write_failed;
business in PQisBusy(). Won't that potentially lead to clients waiting for
more network IO indefinitely, never getting around to calling PQgetResult()?
Greetings,
Andres Freund
