Re: SKIP_LOCKED test causes random buildfarm failures

Hi,

On 2019-11-14 13:37:44 -0500, Tom Lane wrote:
> I 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.
>
> Well, I'm not hearing any groundswell of support for changing the
> message level, so let's leave that as-is and just see about
> stabilizing the tests.

Ok.

> >> 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?
>
> After looking more closely at the isolation test, I agree that adding
> the "ALTER TABLE ... SET (autovacuum_enabled = false)" bits to it is
> a good idea. The LOCK operations should make that irrelevant for
> part1, but there's at least a theoretical hazard for part2.
> (Actually, is "autovacuum_enabled = false" really sufficient to
> keep autovacuum away? It'd probably lock the table for long enough
> to examine its reloptions, so it seems like all we're doing here is
> reducing the window for trouble a little bit. Still, maybe that's
> worthwhile.)

+1

> As for the SKIP_LOCKED tests in vacuum.sql, what I now propose is that
> we just remove them. I do not see that they're offering any coverage
> that's not completely redundant with this isolation test. We don't
> need to spend cycles every day on that.

-0 on that, I'd rather just put a autovacuum_enabled = false for
them. They're quick enough, and it's nice to have decent coverage of
various options within the plain regression tests when possible.

Greetings,

Andres Freund