Andres Freund wrote:
> On 2013-11-25 12:36:19 -0300, Alvaro Herrera wrote:
> > There is no way to close the window, but there is no need; if the
> > updater aborted, we don't need to mark the page prunable in the first
> > place. So we can just check the return value of
> > HeapTupleHeaderGetUpdateXid and if it's InvalidXid, don't set the
> > prunable bit. The second attachment below fixes the bug that way.
>
> I am not sure I like the fact that HeapTupleHeaderGetUpdateXid() checks
> for aborted transactions in the first place. Why is that a good idea?
> ISTM that wanders off a fair bit from the other HeapTupleHeaderGet*
> macros.
Originally it didn't, which caused various bugs. I recall it turned out
to be cleaner to do the check inside it than putting it out to its
callers.
I have thoughts that this design might break other things such as the
priorXmax checking while traversing HOT chains. Not seeing how: surely
if there's an aborted updater in a tuple, there can't be a followup HOT
chain elsewhere involving the same tuple. A HOT chain would require
another updater Xid in the MultiXact (and we ensure there can only be
one updater in a multi). I might be missing something.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services