On Wed, Sep 8, 2021 at 12:27 PM Peter Geoghegan <p...@bowt.ie> wrote: > But these things are *highly* related. > > The RelationGetBufferForTuple() prune mechanism I described (that > targets aborted xact tuples and sets hint bits) is fundamentally built > on top of the idea of ownership of heap pages by backends/transactions > -- that was what I meant before. We *need* to have context. This isn't an > ordinary heap prune -- it doesn't have any of the prechecks to avoid > useless pruning that you see at the top of heap_page_prune_opt(). It's > possible that we won't be able to get a super-exclusive lock in the > specialized prune code path, but that's considered a rare corner case. > There is no question of concurrent inserters senselessly blocking the > prune, which is not at all true with the current approach to free > space management. So there is no way I could extract a minimal "prune > inside RelationGetBufferForTuple()" patch that would actually work.
I'm not trying to argue for slimming down your patches to a size that is so small that they no longer work. However, I *am* arguing that, like bottom-up index deletion and the emergency vacuum mechanism, this change sounds like something that could *easily* have unforeseen consequences. And therefore a lot of caution is needed. And part of that caution is not changing more things at the same time than is really necessary. And that it's worth thinking *hard* about how much change is *really* necessary. It's very easy to convince oneself that everything is connected to everything else and therefore we have to change a lot of things all at once, but that's often not true. -- Robert Haas EDB: http://www.enterprisedb.com