Re: Add client connection check during the execution of the query

On Mon, Mar 22, 2021 at 3:29 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> 2.  The tests need tightening up.  The thing with the "sleep 3" will
> not survive contact with the build farm, and I'm not sure if the SSL
> test is as short as it could be.

I don't think the TAP test can be done in the way Sergey had it,
because of multiple races that would probably fail on
slow/overloaded/valgrind machines.  I'll try to think of a better way,
but for now I've removed those tests.

I realised that this should really be testing DoingCommandRead to
decide when it's time to stop checking and re-arming (originally it
was checking PqReadingMessage, which isn't quite right), so I moved a
tiny bit more of the logic into postgres.c, keeping only the actual
connection-check in pqcomm.c.

That leaves the thorny problem Tom mentioned at the top of this
thread[1]: this socket-level approach can be fooled by an 'X' sitting
in the socket buffer, if a client that did PQsendQuery() and then
PQfinish().  Or perhaps even by SSL messages invisible to our protocol
level.  That can surely only be addressed by moving the 'peeking' one
level up the protocol stack.  I've attached a WIP attemp to do that,
on top of the other patch.  Lookahead happens in our receive buffer,
not the kernel's socket buffer.  It detects the simple 'X' case, but
not deeper pipelines of queries (which would seem to require an
unbounded receive buffer and lookahead that actually decodes message
instead of just looking at the first byte, which seems way over the
top considering the price of infinite RAM these days).  I think it's
probably safe in terms of protocol synchronisation because it consults
PqCommReadingMsg to avoid look at non-message-initial bytes, but I
could be wrong, it's a first swing at it...  Maybe it's a little
unprincipled to bother with detecting 'X' at all if you can't handle
pipelining in general... I don't know.

Today I learned that there have been other threads[2][3] with people
wanting some variant of this feature over the years.

[1] https://www.postgresql.org/message-id/19003.1547420739%40sss.pgh.pa.us
[2] https://www.postgresql.org/message-id/flat/e09785e00907271728k4bf4d17kac0e7f5ec9316069%40mail.gmail.com
[3] https://www.postgresql.org/message-id/flat/20130810.113901.1014453099921841746.t-ishii%40sraoss.co.jp