Re: SKIP_LOCKED test causes random buildfarm failures

On Thu, Nov 07, 2019 at 11:50:25AM -0500, Tom Lane wrote:
> Michael Paquier <michael(at)paquier(dot)xyz> writes:
>> 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.

Perhaps. Well, that's the same level as the one used after the
permission checks on the relation vacuumed.

> 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.

Makes sense.

> 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?

I think that there is no risk regarding the stability of the output
because we use LOCK before from a first session on the relation to
vacuum in a second session. So if autovacuum runs in parallel, the
consequence would be a small slow down while waiting on the lock to be
taken. And per the way the test is ordered, it seems to me that it
makes the most sense to disable autovacuum as it would just get in the
way. In this case I think that it is actually better to show the
messages as that makes the tests more verbose and we make sure to test
their format, even if we could just rely on the fact that VACUUM
should just be blocking or non-blocking.
--
Michael