Re: Regression tests failures due to concurrent grants

Hi,

On 2026-09-18 11:08:43 -0700, Jeff Davis wrote:
> On Fri, 2026-09-18 at 12:56 -0400, Andres Freund wrote:
> > To do better, we would need to use a
> >  * self-exclusive lock, perhaps ShareUpdateExclusiveLock, here and
> > before
> >  * *every* CatalogTupleUpdate() of a row that GRANT/REVOKE can
> > affect.
>
> ...
>
> > I also am not convinced that cancelling autovacs in response to a
> > command like
> > this would be the end of the world.
>
> For GRANT/REVOKE, that matches my intuition.
> But if we need a stronger lock before every modification of a catalog row,
> that's a larger change that would affect lots of DDL.

ISTM it's just flat out broken to modify catalog rows without a corresponding
lock. I'm not saying that we can get to never doing that immediately, but it
seems a recipe for contiuing to have very complicated bugs if we continue with
the assumption that that's a sane thing to do.

The thread that Alexander Lakhin referenced [1] and the fact that we have
tests demonstrating that we know that vacuum gets confused by concurrent
GRANTs [2], shows how fragile all of this is. I see no reason why this would
only be a problem with GRANTs.

I suspect that we also need to make sure that we never do catalog accesses
while in PROC_IN_VACUUM mode, but that's another non-trivial change :(.

If there aren't a few, not-yet-known & hard to hit, bugs around PROC_IN_VACUUM
being set far too widely, I'd be extremely surprised. The fact that we set it
so far out (vacuum_rel(), before we even have opened relations etc), rather
than just down in lazy_scan_heap(), is insane.

I suspect we ought to entirely get rid of PROC_IN_VACUUM, it's basically an
usecured handgranade made with unstable explosives held on a rattling train.

I think we probably can replace it with releasing snapshots (and thux ->xmin)
during the long-running parts of vacuum (which have to guarantee that we don't
do catalog accesses).

I think Noah has done heroic work to plug holes around all this craziness, but
I don't think it's a sane path to continue down long-term. I don't think most
committers would accept such fragile & complex stuff if it were proposed
today.

A second aspect is that not having assertions against doing catalog
modifications with insufficient lock levels is that new bugs are way easier to
introduce. We had a number of such cases recently, were patch authors just
didn't think about that aspect sufficiently. So continuing in our old ways
also makes new development harder.

Greetings,

Andres Freund

[1] https://www.postgresql.org/message-id/9f7cc148-55d1-4062-8229-b6886d7aa380%40gmail.com
[2] https://git.postgresql.org/cgit/postgresql.git/tree/src/bin/pgbench/t/001_pgbench_with_server.pl#n71
https://git.postgresql.org/cgit/postgresql.git/tree/src/test/modules/injection_points/specs/inplace.spec