On Wed, Sep 8, 2021 at 12:27 PM Peter Geoghegan <pg@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
