From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org
Subject: Re: SKIP_LOCKED test causes random buildfarm failures
In-reply-to: <20191107013942.GA1768@paquier.xyz>
References: <25294.1573077278@sss.pgh.pa.us>
 <20191106230111.2kmd6hnriqttw6ys@alap3.anarazel.de>
 <20191107013942.GA1768@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Thu, 07 Nov 2019 10:39:42 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <12731.1573145425.1@sss.pgh.pa.us>
Date: Thu, 07 Nov 2019 11:50:25 -0500
Message-ID: <12732.1573145425@sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Nov 06, 2019 at 03:01:11PM -0800, Andres Freund wrote:
>> I don't know what lead us to doing so, but it doesn't seem reasonable to
>> allow the user to see whether the table has actually been vacuumed. I
>> would assume that one uses SKIP_LOCKED partially to avoid unnecessary
>> impacts in production due to other tasks starting to block on e.g. a
>> VACUUM FULL, even though without the "ordered queueing" everything could
>> just go on working fine. I'm not sure that indicates whether WARNING or
>> NOTICE is the best choice.

> Good question.  That's a historical choice, still I have seen cases
> where those warnings are helpful while not making the logs too
> verbose to see some congestion in the jobs.

I kind of feel that NOTICE is more semantically appropriate, but
perhaps there's an argument for keeping it at WARNING.

>> So I'd be inclined to go with the client_min_messages approach?

> The main purpose of the tests in regress/ is to check after the
> grammar, so using client_min_messages sounds like a plan.  We have
> a second set of tests in isolation/ where I would actually like to
> disable autovacuum by default on a subset of tables.  Thoughts about
> the attached?

I do not want to fix this in the main tests by disabling autovacuum,
because that'd actually reduce the tests' cross-section.  The fact
that this happens occasionally is a Good Thing for verifying that the
code paths actually work.  So it seems that there's a consensus on
adjusting client_min_messages to fix the test output instability ---
but we need to agree on whether to do s/WARNING/NOTICE/ first, so we
can know what to set client_min_messages to.

As for the case in the isolation test, shouldn't we also use
client_min_messages there, rather than prevent the conflict
from arising?  Or would that test fail in some larger way if
autovacuum gets in the way?

			regards, tom lane