Re: Add client connection check during the execution of the query - Mailing list pgsql-hackers

On Sat, Feb 9, 2019 at 6:16 AM <s.cherkashin@postgrespro.ru> wrote:
> The purpose of this patch is to stop the execution of continuous
> requests in case of a disconnection from the client. In most cases, the
> client must wait for a response from the server before sending new data
> - which means there should not remain unread data on the socket and we
> will be able to determine a broken connection.
> Perhaps exceptions are possible, but I could not think of such a use
> case (except COPY). I would be grateful if someone could offer such
> cases or their solutions.
> I added a test for the GUC variable when the client connects via SSL,
> but I'm not sure that this test is really necessary.

Hello Sergey,

This seems like a reasonable idea to me.  There is no point in running
a monster 24 hour OLAP query if your client has gone away.  It's using
MSG_PEEK which is POSIX, and I can't immediately think of any reason
why it's not safe to try to peek at a byte in that socket at any time.
Could you add a comment to explain why you have !PqCommReadingMsg &&
!PqCommBusy?  The tests pass on a couple of different Unixoid OSes I
tried.  Is it really necessary to do a bunch of IO and busy CPU work
in $long_query?  pg_sleep(60) can do the job, because it includes a
standard CHECK_FOR_INTERRUPTS/latch loop that will loop around on
SIGALRM.  I just tested that your patch correctly interrupts
pg_sleep() if I kill -9 my psql process.  Why did you call the timeout
SKIP_CLIENT_CHECK_TIMEOUT (I don't understand the "SKIP" part)?  Why
not CLIENT_CONNECTION_CHECK_TIMEOUT or something?

I wonder if it's confusing to users that you get "connection to client
lost" if the connection goes away while running a query, but nothing
if the connection goes away without saying goodbye ("X") while idle.

The build fails on Windows.  I think it's because
src/tools/msvc/Mkvcbuild.pm is trying to find something to compile
under src/test/modules/connection, and I think the solution is to add
that to the variable @contrib_excludes.  (I wonder if that script
should assume there is nothing to build instead of dying with "Could
not determine contrib module type for connection", otherwise every
Unix hacker is bound to get this wrong every time.)

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.45820

Aside from that problem, my Windows CI building thing isn't smart
enough to actually run those extra tests yet, so I don't know if it
actually works on that platform yet (I really need to teach that thing
to use the full buildfarm scripts...)


--
Thomas Munro
https://enterprisedb.com