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 <pgsql-committers-owner+archive@lists.postgresql.org>)
	id 1iFAQv-0006vq-2r
	for pgsql-committers@arkaria.postgresql.org; Tue, 01 Oct 2019 05:10:45 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.89)
	(envelope-from <pgsql-committers-owner+archive@lists.postgresql.org>)
	id 1iFAQt-0002h8-OM
	for pgsql-committers@arkaria.postgresql.org; Tue, 01 Oct 2019 05:10:43 +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 <tgl@sss.pgh.pa.us>)
	id 1iFAQt-0002h1-HW
	for pgsql-committers@lists.postgresql.org; Tue, 01 Oct 2019 05:10:43 +0000
Received: from sss.pgh.pa.us ([66.207.139.130])
	by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1iFAQr-00020j-0M
	for pgsql-committers@lists.postgresql.org; Tue, 01 Oct 2019 05:10:43 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id x915AaxV018723;
	Tue, 1 Oct 2019 01:10:36 -0400
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Add hooks for session start and session end, take two
In-reply-to: <20191001045246.GF2781@paquier.xyz>
References: <E1iF8dy-0000fc-9S@gemulon.postgresql.org>
 <20191001045246.GF2781@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Tue, 01 Oct 2019 13:52:46 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <18721.1569906636.1@sss.pgh.pa.us>
Date: Tue, 01 Oct 2019 01:10:36 -0400
Message-ID: <18722.1569906636@sss.pgh.pa.us>
List-Id: <pgsql-committers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-committers@lists.postgresql.org>
List-Owner: <mailto:pgsql-committers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-committers>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Oct 01, 2019 at 03:16:06AM +0000, Michael Paquier wrote:
>> Add hooks for session start and session end, take two

> So the buildfarm got again angry on that with crake and thorntail:

> Both machines are using force_parallel_mode = regress, and I can
> reproduce the module crash with that backtrace once I enforce the
> parameter:
> #2  0x00005609ce1f1454 in ExceptionalCondition
> (conditionName=conditionName@entry=0x5609ce25cdd0
> !IsParallelWorker()",
> errorType=errorType@entry=0x5609ce24501d "FailedAssertion",
> fileName=fileName@entry=0x5609ce259bef "xact.c",
> lineNumber=lineNumber@entry=757) at assert.c:54
> #3  0x00005609cde04410 in GetCurrentCommandId
> (used=used@entry=true) at xact.c:757
> #4  0x00005609cdf3e620 in standard_ExecutorStart
> (queryDesc=0x5609cf072330, eflags=0) at execMain.c:238
> #5  0x00005609cdf7ad71 in _SPI_pquery (tcount=0,
> fire_triggers=true, queryDesc=<optimized out>) at spi.c:2519
> #6  _SPI_execute_plan (plan=<optimized out>, paramLI=<optimized
> out>, snapshot=<optimized out>, crosscheck_snapshot=<optimized
> out>, read_only=<optimized out>,
> fire_triggers=<optimized out>, tcount=<optimized out>) at spi.c:2297
> #7  0x00005609cdf7b3ac in SPI_execute (src=0x5609cf06c050 "INSERT INTO
> session_hook_log (dbname, username, hook_at) VALUES
> ('contrib_regression', 'regress_sess_hook_usr2', 'END');",
>     read_only=read_only@entry=false,
> tcount=tcount@entry=0) at spi.c:514
> #8  0x00005609cdf7b3ea in SPI_exec (src=<optimized out>,
> tcount=tcount@entry=0) at spi.c:526
> #9  0x00007feefe55b2ef in register_session_hook
> (hook_at=<optimized out>) at test_session_hooks.c:67
> #10 0x00005609ce09a351 in shmem_exit
> (code=code@entry=0) at ipc.c:239
> #11 0x00005609ce09a45d in proc_exit_prepare
> (code=code@entry=0) at ipc.c:194
> #12 0x00005609ce09a502 in proc_exit
> (code=code@entry=0) at ipc.c:107
> #13 0x00005609ce02c42f in StartBackgroundWorker () at
> bgworker.c:837

Uh, WHAT?

The idea that you can launch a query after proc_exit has started is
complete insanity.  I hope this is just a poorly-thought-out test
case, and not something that's inherent to this module.  If there
are not reasonable use-cases that don't need that, we should just
revert the feature altogether, because it's nothing but a large
caliber foot-gun.

			regards, tom lane