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

pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed

From: Maksim Milyutin <milyutinma@gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
To: tsunakawa.takay@fujitsu.com <tsunakawa.takay@fujitsu.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Zhihong Yu <zyu@yugabyte.com>
Subject: Re: Add client connection check during the execution of the query
Date: Mon, 11 Oct 2021 17:10:29 +0300
Message-ID: <f8547534-47ec-0f7c-fb6a-a7eb347f5a7e@gmail.com> (raw)
In-Reply-To: <CA+hUKGLYmBkROj=tUdXNeymxSdpBtfb=L5VcQKqUiZdvk2cBgg@mail.gmail.com>
References: <CA+hUKG+TGsHUZ4S6QOXr_ueKJ9-8d-TaoDpk9zdgjB_vHqotzQ@mail.gmail.com>
	<CA+hUKGJ3-N0F7T_tcRnGZv0_g_DJwcYM2=XTYCHrvcFG2YrrWg@mail.gmail.com>
	<20190718.121906.432767475415875714.t-ishii@sraoss.co.jp>
	<79f5f12b-184f-dcdb-b22f-4f95b6a89f2d@postgrespro.ru>
	<CA+hUKGL9LLNE8_+rBkRzBhmXA1XHEzJVqQr4T4fqzngqp66=-g@mail.gmail.com>
	<6bd6945e-f02e-8ee8-b8c4-e8d74e283efd@gmail.com>
	<CA+hUKGLyj5Aqt6ojYfSc+qSeB1x=3RbU61hnus5sL0BKqEBsLw@mail.gmail.com>
	<CA+hUKG+OuHdowPJ--cdN0+ZhPHiFqw0TDHp__N=Eu9yPsJtmyw@mail.gmail.com>
	<TYAPR01MB2990F4C319FC852CCED60394FE7B9@TYAPR01MB2990.jpnprd01.prod.outlook.com>
	<CA+hUKGLJ3PmhunHO4B+nFoxtJtodDo5H2GhP0zH2TVTfnmkJ7Q@mail.gmail.com>
	<TYAPR01MB2990E5F57212DBFD41F61983FE7A9@TYAPR01MB2990.jpnprd01.prod.outlook.com>
	<CA+hUKG++KitzNUOxW2-koB1pKWD2cyUqA9vLj5bf0g_i7L1M0w@mail.gmail.com>
	<CA+hUKG+tg5fWWvF5WbwEt4VO69Tj9o6F9mq-wd9kHHxDhUiBzw@mail.gmail.com>
	<CA+hUKGLYmBkROj=tUdXNeymxSdpBtfb=L5VcQKqUiZdvk2cBgg@mail.gmail.com>

On 12.06.2021 07:24, Thomas Munro wrote:

> On Fri, Apr 30, 2021 at 2:23 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> Here's something I wanted to park here to look into for the next
>> cycle:  it turns out that kqueue's EV_EOF flag also has the right
>> semantics for this.  That leads to the idea of exposing the event via
>> the WaitEventSet API, and would the bring
>> client_connection_check_interval feature to 6/10 of our OSes, up from
>> 2/10.  Maybe Windows' FD_CLOSE event could get us up to 7/10, not
>> sure.
> Rebased.  Added documentation tweak and a check to reject the GUC on
> unsupported OSes.


Good work. I have tested your patch on Linux and FreeBSD on three basic 
cases: client killing, cable breakdown (via manipulations with firewall) 
and silent closing client connection before completion of previously 
started query in asynchronous manner. And all works fine.

Some comments from me:

  bool
  pq_check_connection(void)
  {
-#if defined(POLLRDHUP)
+    WaitEvent events[3];


3 is looks like as magic constant. We might to specify a constant for 
all event groups in FeBeWaitSet.


+    ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, NULL);
+    ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos, 
WL_SOCKET_CLOSED, NULL);
+    rc = WaitEventSetWait(FeBeWaitSet, 0, events, lengthof(events), 0);
+    ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, 
MyLatch);

AFAICS, side effect to 
(FeBeWaitSet->events[FeBeWaitSetSocketPos]).events by setting 
WL_SOCKET_CLOSED value under calling of pq_check_connection() function 
doesn't have negative impact later, does it? That is, all 
WaitEventSetWait() calls have to setup socket events on its own from 
scratch.


-- 
Regards,
Maksim Milyutin

view thread (54+ messages)  latest in thread

Message-ID: <f8547534-47ec-0f7c-fb6a-a7eb347f5a7e@gmail.com>
Permalink:  ../f8547534-47ec-0f7c-fb6a-a7eb347f5a7e@gmail.com/
Also on:    postgresql.org/message-id/f8547534-47ec-0f7c-fb6a-a7eb347f5a7e@gmail.com

 ·

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: milyutinma@gmail.com, thomas.munro@gmail.com, tsunakawa.takay@fujitsu.com, ishii@sraoss.co.jp, tgl@sss.pgh.pa.us, andres@anarazel.de, zyu@yugabyte.com
  Subject: Re: Add client connection check during the execution of the query
  In-Reply-To: <f8547534-47ec-0f7c-fb6a-a7eb347f5a7e@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox