Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1htacJ-00034D-Lr for pgsql-hackers@arkaria.postgresql.org; Fri, 02 Aug 2019 16:41:20 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1htabI-0006uR-S8 for pgsql-hackers@arkaria.postgresql.org; Fri, 02 Aug 2019 16:40:16 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1htabI-0006qq-CV for pgsql-hackers@lists.postgresql.org; Fri, 02 Aug 2019 16:40:16 +0000 Received: from cyclops.postgrespro.ru ([93.174.131.138] helo=mail.postgrespro.ru) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1htabB-0000KU-1P for pgsql-hackers@postgresql.org; Fri, 02 Aug 2019 16:40:15 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id EBB1421C4303; Fri, 2 Aug 2019 19:40:00 +0300 (MSK) X-Virus-Scanned: Debian amavisd-new at postgrespro.ru X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=x tagged_above=-99 required=4 WHITELISTED tests=[] autolearn=unavailable Received: from [192.168.27.200] (gw.postgrespro.ru [93.174.131.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id 3679F21C42F9; Fri, 2 Aug 2019 19:40:00 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1564764000; bh=jRNjjXx1yoSFmNS57wTJnoUc2a6ULGhtMts1GoL7Uqw=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=YPlxEXNX6I9QFR+xKcd64m/1zKuRWBmMG1r6m/AWwKyjEgQiLzSgXs4QLXRM+ujrz 8sS4uRnnAmsyNYKXWZkdBR5Q+GxoCBq//V6cWiEbKOMOHE/vZGmcror4RGOSD5Msg9 niXDSSaBmkygTpXE5z1/upT2ARGIq1gHx+WsVLag= Subject: Re: Add client connection check during the execution of the query To: Tatsuo Ishii , thomas.munro@gmail.com Cc: s.kelvich@postgrespro.ru, tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org References: <37D9769E-2991-46A9-A65B-498E3CF55D8C@postgrespro.ru> <20190718.121906.432767475415875714.t-ishii@sraoss.co.jp> From: Konstantin Knizhnik Message-ID: <79f5f12b-184f-dcdb-b22f-4f95b6a89f2d@postgrespro.ru> Date: Fri, 2 Aug 2019 19:39:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190718.121906.432767475415875714.t-ishii@sraoss.co.jp> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk On 18.07.2019 6:19, Tatsuo Ishii wrote: > > I noticed that there are some confusions in the doc and code regarding what the new > configuration parameter means. According to the doc: > > + Default value is zero - it disables connection > + checks, so the backend will detect client disconnection only when trying > + to send a response to the query. > > But guc.c comment says: > > + gettext_noop("Sets the time interval for checking connection with the client."), > + gettext_noop("A value of -1 disables this feature. Zero selects a suitable default value."), > > Probably the doc is correct since the actual code does so. Yes, value -1 is not even accepted due to the specified range. > tps = 67715.993428 (including connections establishing) > tps = 67717.251843 (excluding connections establishing) > > So the performance is about 5% down with the feature enabled in this > case. For me, 5% down is not subtle. Probably we should warn this in > the doc. I also see some performance degradation, although it is not so large in my case (I used pgbench with scale factor 10 and run the same command as you). In my case difference is 103k vs. 105k TPS is smaller than 2%. It seems to me that it is not necessary to enable timeout at each command: @@ -4208,6 +4210,9 @@ PostgresMain(int argc, char *argv[], */ CHECK_FOR_INTERRUPTS(); DoingCommandRead = false; + if (client_connection_check_interval) + enable_timeout_after(SKIP_CLIENT_CHECK_TIMEOUT, + client_connection_check_interval); /* * (5) turn off the idle-in-transaction timeout Unlike statement timeout or idle in transaction timeout price start of measuring time is not important. So it is possible to do once before main backend loop: @@ -3981,6 +3983,10 @@ PostgresMain(int argc, char *argv[], if (!IsUnderPostmaster) PgStartTime = GetCurrentTimestamp(); + if (client_connection_check_interval) + enable_timeout_after(SKIP_CLIENT_CHECK_TIMEOUT, + client_connection_check_interval); + /* * POSTGRES main processing loop begins here * But actually I do not see much difference from moving enabling timeout code. Moreover the difference in performance almost not depend on the value of timeout. I set it to 100 seconds with pgbench loop 30 seconds (so timeout never fired and recv is never called) and still there is small difference in performance. After some experiments I found out that just presence of active timer results some small performance penalty. You can easily check it: set for example statement_timeout to the same large value (100 seconds) and you will get the same small slowdown. So recv() itself is not source of the problem. Actually any system call (may be except fsync) performed with frequency less than one second can not have some noticeable impact on performance. So I do not think that recv(MSG_PEEK) can cause any performance problem at Windows or any other platform. But I wonder why we can not perform just pool with POLLOUT flag and zero timeout. If OS detected closed connection, it should return POLLHUP, should not it? I am not sure if it is more portable or more efficient way - just seems to be a little bit more natural way (from my point of view) to check if connection is still alive. -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company