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
