Re: pgsql: Add hooks for session start and session end, take two

Michael Paquier <michael(at)paquier(dot)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(at)entry=0x5609ce25cdd0
> !IsParallelWorker()",
> errorType=errorType(at)entry=0x5609ce24501d "FailedAssertion",
> fileName=fileName(at)entry=0x5609ce259bef "xact.c",
> lineNumber=lineNumber(at)entry=757) at assert.c:54
> #3 0x00005609cde04410 in GetCurrentCommandId
> (used=used(at)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(at)entry=false,
> tcount=tcount(at)entry=0) at spi.c:514
> #8 0x00005609cdf7b3ea in SPI_exec (src=<optimized out>,
> tcount=tcount(at)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(at)entry=0) at ipc.c:239
> #11 0x00005609ce09a45d in proc_exit_prepare
> (code=code(at)entry=0) at ipc.c:194
> #12 0x00005609ce09a502 in proc_exit
> (code=code(at)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