Re: SKIP_LOCKED test causes random buildfarm failures

Michael Paquier <michael(at)paquier(dot)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