On 2013-11-25 13:45:53 -0300, Alvaro Herrera wrote: > 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.
This seems strange to me because we do *not* do those checks for !IS_MULTI xmax's. So there surely shouldn't be too many callers caring about that? > I have thoughts that this design might break other things such as the > priorXmax checking while traversing HOT chains. Not seeing how: Hm. Are you arguing with yourself about this? > 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. I don't see dangers that way either. I think there might be some strange behaviour because callers need to check IsRunning() first though, which MultiXactIdGetUpdateXid() doesn't. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers