Re: pgsql: psql: Add test for query canceling

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: psql: Add test for query canceling
Date: 2021-08-20 22:40:42
Message-ID: 1983764.1629499242@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> On 20 Aug 2021, at 20:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I think you should drop the overly-cute bit with a SIGALRM handler,
>> and instead have a loop-with-delay around an attempt to read the
>> psql.pid file, after launching the psql run without an immediate
>> wait for termination.

> This could perhaps be done with a PostgresNode::interactive_psql
> session?

Yeah, we do have that infrastructure available in the
010_tab_completion.pl test.

BTW, after looking more closely at the test, I think there's an
independent problem with the SIGALRM business. If I read the perl
script correctly, it's arming a SIGALRM timer in the Perl script's
process that is supposed to expire after the psql child process
has been launched. However, per the NetBSD man pages:

* exec() causes signal handlers to be reset to default actions in the
child process. The default action for SIGALRM is to terminate the
process.

* Active timer events are *inherited* by the child process. This seems
not very wise, but execve(2) is pretty definite about it [1].

This means that on NetBSD and similar systems, what actually happens
when the SIGALRM timer times out is that the psql process is killed,
likely before the Perl script is able to signal it, but in any case
not too darn much later. So I do not think that this test is testing
what it means to test at all: there is no reason to believe that any
query cancel request ever gets sent, even without the timing issue.
Since the Perl script is only testing that psql returns nonzero exit
status, and not bothering to inquire into just which status it was,
it cannot tell "query was canceled" apart from "psql died horribly".
And the latter is what's actually happening.

regards, tom lane

[1] https://man.netbsd.org/NetBSD-9.2-STABLE/execve.2

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Dilip Kumar 2021-08-21 12:38:17 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Previous Message Daniel Gustafsson 2021-08-20 21:21:09 Re: pgsql: psql: Add test for query canceling