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 18:47:58
Message-ID: 1957503.1629485278@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:
> If we want the test to run but not fail the entire test suite if it fails then
> it should use a TODO block instead, but that’s intended for tests known to fail
> and this doesn’t seem to fall in that category.

That seems pretty useless. If we did break things in this area,
such a test would not help us notice.

The problem with the test seems blindingly obvious from here: it
is assuming first that psql will start fast enough to print its
PID within one second, and next that we'll be able to issue
the cancel (and have the backend react) in less than 2 seconds
more. This seems about guaranteed to fail on cache-clobber
animals, for example, but animals that are merely slow or overloaded
would have issues too.

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. That gets rid of the first problem (though
you still want the loop to timeout eventually, it could wait up
to say 180 seconds, as we do elsewhere). Then the second problem
is easy to solve by making the pg_sleep delay twice as much.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-08-20 21:21:09 Re: pgsql: psql: Add test for query canceling
Previous Message Daniel Gustafsson 2021-08-20 18:27:32 Re: pgsql: psql: Add test for query canceling