From: Tom Lane <tgl@sss.pgh.pa.us>
To: Julien Rouhaud <rjuju123@gmail.com>
cc: Michael Paquier <michael@paquier.xyz>,
        Michael Paquier <michael.paquier@gmail.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add an optional timeout clause to isolationtester step.
In-reply-to: <20200307061615.c47sju2bytuiaazv@nol>
References: <20200222150657.GA54846@nol> <20200227073211.GA403330@paquier.xyz>
 <20200227080735.l32fqcauy73lon7o@nol> <20200303080642.GA1890@paquier.xyz>
 <20200303092551.GB1890@paquier.xyz> <20200304051510.GE2593@paquier.xyz>
 <CAOBaU_Z=qkE42VQ25mps+zyAz+WUn3wZoev4uhGW+gAXBBS2YA@mail.gmail.com>
 <20200305035354.GQ2593@paquier.xyz> <20200306131547.GA2904@nol>
 <20200307014142.GC1531@paquier.xyz> <20200307061615.c47sju2bytuiaazv@nol>
Comments: In-reply-to Julien Rouhaud <rjuju123@gmail.com>
	message dated "Sat, 07 Mar 2020 07:16:15 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8440.1583595994.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sat, 07 Mar 2020 10:46:34 -0500
Message-ID: <8441.1583595994@sss.pgh.pa.us>
Precedence: bulk

Julien Rouhaud <rjuju123@gmail.com> writes:
> On Sat, Mar 07, 2020 at 10:41:42AM +0900, Michael Paquier wrote:
>> On Fri, Mar 06, 2020 at 02:15:47PM +0100, Julien Rouhaud wrote:
>>> Here's a patch to add an optional "timeout val" clause to isolationtes=
ter's
>>> step definition.  When used, isolationtester will actively wait on the=
 query
>>> rather than continuing with the permutation next step, and will issue =
a cancel
>>> once the defined timeout is reached.

>> One problem with this approach is that it does address the stability
>> of the test on very slow machines, and there are some of them in the
>> buildfarm.

> Looking at timeouts.spec and e.g. a7921f71a3c, it seems that we already =
chose
> to fix this problem by having a timeout long enough to statisfy the slow=
er
> buildfarm members, even when running on fast machines, so I assumed that=
 the
> same approach could be used here.

The arbitrarily-set timeouts that exist in some of the isolation tests
are horrid kluges that have caused us lots of headaches in the past
and no doubt will again in the future.  Aside from occasionally failing
when a machine is particularly overloaded, they cause the tests to
take far longer than necessary on decently-fast machines.  So ideally
we'd get rid of those entirely in favor of some more-dynamic approach.
Admittedly, I have no proposal for what that would be.  But adding yet
more ways to set a (guaranteed-to-be-wrong) timeout seems like the
wrong direction to be going in.  What's the actual need that you're
trying to deal with?

			regards, tom lane