Date: Wed, 11 Mar 2020 17:52:54 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>,
	Julien Rouhaud <rjuju123@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add an optional timeout clause to isolationtester step.
Message-ID: <20200311205254.GA2648@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <24078.1583958800@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Mar-11, Tom Lane wrote:

> We could re-use Julien's ideas about the isolation spec syntax by
> making it be, roughly,
> 
> step "<name>" { <SQL> } [ blocked if "<wait_event_type>" "<wait_event>" ]
> 
> and then those items would need to be passed as parameters of the prepared
> query.

I think for test readability's sake, it'd be better to put the BLOCKED
IF clause ahead of the SQL, so you can write it in the same line and let
the SQL flow to the next one:

STEP "long_select" BLOCKED IF "lwlock" "ClogControlLock"
  { select foo from pg_class where ... some more long clauses ... }

otherwise I think a step would require more lines to write.

> I'd like to see an attempt to rewrite some of the existing
> timeout-dependent test cases to use this facility instead of
> long timeouts.  If we could get rid of the timeouts in the
> deadlock tests, that'd go a long way towards showing that this
> idea is actually any good.

+1.  Those long timeouts are annoying enough that infrastructure to make
a run shorter in normal circumstances might be sufficient justification
for this patch ...

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services