Date: Tue, 10 Sep 2019 04:37:25 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
	pgsql-committers@lists.postgresql.org,
	Thomas Munro <thomas.munro@gmail.com>
Subject: Re: pgsql: Reorder EPQ work, to fix rowmark related bugs and improve
 effici
Message-ID: <20190910113725.54d4z3arjgwdf6j6@alap3.anarazel.de>
References: <20190910013835.GA5167@alvherre.pgsql>
 <30239.1568081602@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <30239.1568081602@sss.pgh.pa.us>
Precedence: bulk

Hi,

On 2019-09-09 22:13:22 -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2019-Sep-09, Tom Lane wrote:
> >> It appears to me that this is indeed a case of notice-reporting
> >> timing problems in isolationtester, since these WARNING messages
> >> are handled the same way as notices.  I want to try to reproduce
> >> manually on prairiedog to confirm that, but it seems like a pretty
> >> likely explanation.
> 
> Yeah, seems confirmed.  I ran forty cycles of eval-plan-qual
> without a failure with current HEAD.  I then reverted 30717637c,
> and eval-plan-qual fell over six times out of six, with the same
> diffs shown in the buildfarm report.  So that patch is definitely
> a prerequisite to making this version of eval-plan-qual stable.

Phew. Thanks for testing that.


> >> On balance I'm inclined to back-patch both changes.  Thoughts?
> 
> > As well as a28e10e82e54, I suppose.  I agree with keeping the tool
> > similar across branches, if we're going to do this.
> 
> Hm, good point.  My first thought was that a28e10e82e54 is just
> cosmetic, but it isn't entirely, because it suppresses notice
> reports on the control connection.  That means it might actually
> be a prerequisite to having stable output with ebd499282 (the
> change of client_min_messages).
> 
> After reviewing the git log a little more, I'm inclined to think
> we should only back-patch this stuff to 9.6, which is where 38f8bdcac
> ("Modify the isolation tester so that multiple sessions can wait")
> and a number of follow-up patches came in.  That was enough of a
> quantum jump in flexibility that it'd likely limit our ability to
> back-patch tests further than that anyway.  Also I don't think the
> patches mentioned here would apply without that ...

That seems like a good plan to me.

Greetings,

Andres Freund