Re: pgsql: Fix a couple of bugs in MultiXactId freezing

On Tue, Dec 03, 2013 at 04:08:23PM +0100, Andres Freund wrote:
> On 2013-12-03 09:16:18 -0500, Noah Misch wrote:
> > On Tue, Dec 03, 2013 at 11:56:07AM +0100, Andres Freund wrote:
> > > On 2013-12-03 00:47:07 -0500, Noah Misch wrote:
> > > > On Sat, Nov 30, 2013 at 01:06:09AM +0000, Alvaro Herrera wrote:
> > > Any idea how to cheat our way out of that one given the current way
> > > heap_freeze_tuple() works (running on both primary and standby)? My only
> > > idea was to MultiXactIdWait() if !InRecovery but that's extremly grotty.
> > > We can't even realistically create a new multixact with fewer members
> > > with the current format of xl_heap_freeze.
> > 
> > Perhaps set HEAP_XMAX_LOCK_ONLY on the tuple?  We'd then ensure all update XID
> > consumers check HEAP_XMAX_IS_LOCKED_ONLY() first, much like xmax consumers are
> > already expected to check HEAP_XMAX_INVALID first.  Seems doable, albeit yet
> > another injection of complexity.
> 
> I think its pretty much checked that way already, but the problem seems
> to be how to avoid checks on xid commit/abort in that case. I've
> complained in 20131121200517.GM7240@alap2.anarazel.de that the old
> pre-condition that multixacts aren't checked when they can't be relevant
> (via OldestVisibleM*) isn't observed anymore.
> So, if we re-introduce that condition again, we should be on the safe
> side with that, right?

What specific commit/abort checks do you have in mind?

> > > > The test spec additionally
> > > > covers a (probably-related) assertion failure, new in 9.3.2.
> > > 
> > > Too bad it's too late to do anthing about it for 9.3.2. :(. At least the
> > > last seems actually unrelated, I am not sure why it's 9.3.2
> > > only. Alvaro, are you looking?
> > 
> > (For clarity, the other problem demonstrated by the test spec is also a 9.3.2
> > regression.)
> 
> Yea, I just don't see why yet... Looking now.

Sorry, my original report was rather terse.  I speak of the scenario exercised
by the second permutation in that isolationtester spec.  The multixact is
later than VACUUM's cutoff_multi, so 9.3.1 does not freeze it at all.  9.3.2
does freeze it to InvalidTransactionId per the code I cited in my first
response on this thread, which wrongly removes a key lock.

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com