Re: HeapTupleHeaderAdvanceLatestRemovedXid doing the wrong thing with multixacts - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: HeapTupleHeaderAdvanceLatestRemovedXid doing the wrong thing with multixacts
Date
Msg-id 1345132934-sup-1120@alvh.no-ip.org
Whole thread Raw
In response to Re: HeapTupleHeaderAdvanceLatestRemovedXid doing the wrong thing with multixacts  (Bruce Momjian <bruce@momjian.us>)
List pgsql-hackers
Excerpts from Bruce Momjian's message of jue ago 16 11:44:48 -0400 2012:
> On Thu, Aug 16, 2012 at 11:38:14AM -0400, Alvaro Herrera wrote:
> > Excerpts from Bruce Momjian's message of jue ago 16 11:24:55 -0400 2012:
> > > On Mon, Oct 17, 2011 at 08:38:18PM +0100, Simon Riggs wrote:
> > > > On Mon, Oct 17, 2011 at 8:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > > > Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > > > >> I just noticed that HeapTupleHeaderAdvanceLatestRemovedXid is comparing Xmax as a TransactionId without
verifyingwhether it is a multixact or not.  Since they advance separately, this could lead to bogus answers.  This
probablyneeds to be fixed.  I didn't look into past releases to see if there's a live released bug here or not. 
> > > > >
> > > > >> I think the fix is simply to ignore the Xmax if the HEAP_XMAX_IS_MULTI bit is set.
> > > > >
> > > > >> Additionally I think it should check HEAP_XMAX_INVALID before reading the Xmax at all.
> > > > >
> > > > > If it's failing to even check XMAX_INVALID, surely it's completely
> > > > > broken?  Perhaps it assumes its caller has checked all this?
> > > >
> > > > HeapTupleHeaderAdvanceLatestRemovedXid() is only ever called when
> > > > HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, which only happens
> > > > when HEAP_XMAX_IS_MULTI is not set.
> > > >
> > > > I'll add an assert to check this and a comment to explain.
> > >
> > > Was this completed?
> >
> > As far as I recall, there are changes related to this in my fklocks
> > patch.  I am hoping to have some review happen on it during the upcoming
> > commitfest (which presumably means I need to do a merge to newer
> > sources.)
>
> I was asking about adding the assert check --- does that need to wait
> too?

I don't think it's worth fussing about.  Also I don't need any more
merge conflicts than I already have.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: The pgrminclude problem
Next
From: "Kevin Grittner"
Date:
Subject: Re: cataloguing NOT NULL constraints